[Django] #31357: Stale content types can cause exception in ContentType.get_for_models
Django
------------------------------------------------+------------------------
Reporter: Andy Chosak | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 3.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
`ContentType.get_for_models`
([[https://github.com/django/django/blob/e3e48b00127c09eafe6439d980a82fc5c591b673/django/contrib/contenttypes/models.py#L62|source]])
retrieves content types from the database for a given list of models. Due
to the way this method works, it's possible to trigger an unexpected
`AttributeError` by passing in a list of models whose `(app_label,
model_name)` pairs can be permuted to refer to a model with a stale
content type in the database.
Consider this example:
1. A project has an app named `foo` with models named `Author` and `Book`.
Content types for these exist in the database.
2. A new app named `bar` is created providing an alternate `Book` model.
The content type for this also exists.
3. The model `foo.Book` is deleted. The database is migrated, but the
stale content type is left behind.
The following code will raise an exception:
{{{
>> from foo.models import Author
>>> from bar.models import Book
>>> from django.contrib.contenttypes.models import ContentType
>>> ContentType.objects.get_for_models(Author, Book)
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/path/to/myproject/lib/python3.6/site-
packages/django/contrib/contenttypes/models.py", line 89, in
get_for_models
opts_models = needed_opts.pop(ct.model_class()._meta, [])
AttributeError: 'NoneType' object has no attribute '_meta'
}}}
This occurs because the `ct.model_class()` call returns `None` for stale
content types.
The problem is in
[[https://github.com/django/django/blob/e3e48b00127c09eafe6439d980a82fc5c591b673/django/contrib/contenttypes/models.py#L83-L87|this
database lookup]] that tries to pull back content types for the models
that were passed in. Instead of pulling back only the `ContentType`
instances for those models, it filters by
`app_label__in=needed_app_labels, model__in=needed_models`. For the above
case, this pulls back not just `(foo, Author)` and `(bar, Book)` but also
`(foo, Book)`, which in this case is stale.
A fix here could either alter the query to be more specific or check the
returned content types to make sure they correspond to one of the desired
models.
--
Ticket URL: <https://code.djangoproject.com/ticket/31357>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Andy Chosak | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 3.0
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* stage: Unreviewed => Accepted
Comment:
The proper way of performing the query is likely to group models by
`app_label` and filter by unions of `Q(app_label=app_label,
model__in=app_models)` for each of them.
That will likely allow the database to keep using the unique index on
`(app_label, model)`.
--
Ticket URL: <https://code.djangoproject.com/ticket/31357#comment:1>
Django
Reporter: Andy Chosak | Owner: amartya-
| dev
Type: Bug | Status: assigned
Component: | Version: 3.0
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by amartya-dev):
* owner: nobody => amartya-dev
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31357#comment:2>
Django
Reporter: Andy Chosak | Owner: Biel
| Frontera
Type: Bug | Status: assigned
Component: | Version: 3.0
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Biel Frontera):
* owner: Amartya Gaur => Biel Frontera
--
Ticket URL: <https://code.djangoproject.com/ticket/31357#comment:3>
Django
Reporter: Andy Chosak | Owner: Biel
| Frontera
Type: Bug | Status: assigned
Component: | Version: 3.0
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Biel Frontera):
* has_patch: 0 => 1
Comment:
PR sent to fix this issue: [https://github.com/django/django/pull/15508]
I think there is no need to change the query as long unwanted content
types are discarded.
--
Ticket URL: <https://code.djangoproject.com/ticket/31357#comment:4>
Django
Reporter: Andy Chosak | Owner: Biel
| Frontera
Type: Bug | Status: assigned
Component: | Version: 3.0
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31357#comment:5>
Django
Reporter: Andy Chosak | Owner: Biel
| Frontera
Type: Bug | Status: assigned
Component: | Version: 3.0
contrib.contenttypes |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31357#comment:6>
Django
Reporter: Andy Chosak | Owner: Biel
| Frontera
Component: | Version: 3.0
contrib.contenttypes |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"859a87d873ce7152af73ab851653b4e1c3ffea4c" 859a87d8]:
{{{
#!CommitTicketReference repository=""
revision="859a87d873ce7152af73ab851653b4e1c3ffea4c"
Fixed #31357 -- Fixed get_for_models() crash for stale content types when
model with the same name exists in another app.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31357#comment:7>