Opened 12 months ago
Closed 12 months ago
#34816 closed Bug (fixed)
GenericForeignKey crashes if content_type_id is changed and object_id is type incompatible with old object
Reported by: | Richard Laager | Owned by: | Oguzhan Akan |
---|---|---|---|
Component: | contrib.contenttypes | Version: | 4.2 |
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 (last modified by )
Steps to reproduce:
- Create a model ("A") with a GenericForeignKey.
- Create an instance of A ("a") referencing an instance of model "B" with a PK that is an integer type.
- Change the instance of "A" to reference an instance of model "C" with a PK that is incompatible with int(). But make this change using
content_type_id
andobject_id
properties, notcontent_object
, i.e.:a.content_type_id = ContentType.objects.get_for_model(B).pk a.object_id = "foo"
- Try to access
a.content_object
.
Expected result:
This looks up and returns an instance of "b" (the B with a PK of "foo").
Actual result:
This crashes (I'm using 3.2, but the code is unchanged in master):
File "django/db/models/fields/__init__.py", line 1836, in to_python return int(value) ValueError: invalid literal for int() with base 10: 'foo'
This happens because it tries to to_python()
the new key on the old model's PK field.
One possible fix would be to make the logic short-circuit, like this (also attached):
-
django/contrib/contenttypes/fields.py
diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index 35fcd0d908..e984fb5375 100644
a b class GenericForeignKey(FieldCacheMixin): 242 242 ct_match = ( 243 243 ct_id == self.get_content_type(obj=rel_obj, using=instance._state.db).id 244 244 ) 245 pk_match = rel_obj._meta.pk.to_python(pk_val) == rel_obj.pk246 if ct_match and pk_match:247 return rel_obj248 else:249 245 if ct_match: 246 pk_match = rel_obj._meta.pk.to_python(pk_val) == rel_obj.pk 247 if pk_match: 248 return rel_obj 249 rel_obj = None 250 250 if ct_id is not None: 251 251 ct = self.get_content_type(id=ct_id, using=instance._state.db) 252 252 try:
Attachments (1)
Change History (8)
by , 12 months ago
Attachment: | gfk-crash.patch added |
---|
comment:1 by , 12 months ago
Has patch: | unset |
---|---|
Resolution: | → invalid |
Status: | new → closed |
comment:2 by , 12 months ago
Description: | modified (diff) |
---|---|
Resolution: | invalid |
Status: | closed → new |
Sorry, that's just a red herring from writing up the bug report. Here is a real example from right now:
In [1]: from case.models import Case In [2]: case = Case.objects.get(id=REDACTED) In [3]: case.content_object Out[3]: <REDACTED: REDACTED> In [4]: case.content_type_id = 175 In [5]: case.object_id = '218-555-1212' In [6]: case.content_object --------------------------------------------------------------------------- ValueError Traceback (most recent call last) File /srv/www/testing/lib/.venv/lib/python3.10/site-packages/django/db/models/fields/__init__.py:1836, in IntegerField.to_python(self, value) 1835 try: -> 1836 return int(value) 1837 except (TypeError, ValueError): ValueError: invalid literal for int() with base 10: '218-555-1212' During handling of the above exception, another exception occurred: ValidationError Traceback (most recent call last) Cell In[6], line 1 ----> 1 case.content_object File /srv/www/testing/lib/.venv/lib/python3.10/site-packages/django/contrib/contenttypes/fields.py:233, in GenericForeignKey.__get__(self, instance, cls) 231 if rel_obj is not None: 232 ct_match = ct_id == self.get_content_type(obj=rel_obj, using=instance._state.db).id --> 233 pk_match = rel_obj._meta.pk.to_python(pk_val) == rel_obj.pk 234 if ct_match and pk_match: 235 return rel_obj File /srv/www/testing/lib/.venv/lib/python3.10/site-packages/django/db/models/fields/__init__.py:1838, in IntegerField.to_python(self, value) 1836 return int(value) 1837 except (TypeError, ValueError): -> 1838 raise exceptions.ValidationError( 1839 self.error_messages['invalid'], 1840 code='invalid', 1841 params={'value': value}, 1842 ) ValidationError: ['“218-555-1212” value must be an integer.']
If step 3 (accessing case.content_object
) is omitted, the problem does not occur because there is no cached content_object. Alternatively, if between 3 and 4 one does case.content_object = None
to clear the cache, the problem does not occur.
comment:3 by , 12 months ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Bug |
I was able to reproduce the issue with the following test:
-
tests/generic_relations_regress/tests.py
diff --git a/tests/generic_relations_regress/tests.py b/tests/generic_relations_regress/tests.py index b7ecb499eb..7b7883ee88 100644
a b class GenericRelationTests(TestCase): 314 314 result = Link.objects.filter(places=place) 315 315 self.assertCountEqual(result, [link]) 316 316 317 def test_1(self): 318 from django.contrib.contenttypes.models import ContentType 319 320 board = Board.objects.create(name="some test") 321 oddrel = OddRelation1.objects.create(name="clink") 322 charlink = CharLink.objects.create(content_object=oddrel) 323 charlink = CharLink.objects.get(pk=charlink.pk) 324 charlink.content_object 325 charlink.object_id = board.pk 326 charlink.content_type_id = ContentType.objects.get_for_model(Board).id 327 charlink.content_object 328 317 329 def test_generic_reverse_relation_with_abc(self): 318 330 """ 319 331 The reverse generic relation accessor (targets) is created if the
comment:4 by , 12 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 12 months ago
Has patch: | set |
---|
comment:6 by , 12 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
Thanks for this report, however, as far as I'm aware it's an issue in your code:
You should assign
id
to thecontent_type_id
, not a content type instance. Using:or
works for me.