Opened 5 years ago
Closed 3 years ago
#31357 closed Bug (fixed)
Stale content types can cause exception in ContentType.get_for_models
Reported by: | Andy Chosak | Owned by: | Biel Frontera |
---|---|---|---|
Component: | contrib.contenttypes | Version: | 3.0 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
ContentType.get_for_models
(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:
- A project has an app named
foo
with models namedAuthor
andBook
. Content types for these exist in the database. - A new app named
bar
is created providing an alternateBook
model. The content type for this also exists. - 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 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.
Change History (7)
comment:1 by , 5 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 3 years ago
Owner: | changed from | to
---|
comment:4 by , 3 years ago
Has patch: | set |
---|
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.
comment:5 by , 3 years ago
Patch needs improvement: | set |
---|
comment:6 by , 3 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
The proper way of performing the query is likely to group models by
app_label
and filter by unions ofQ(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)
.