GenericForeignKey crashes if content_type_id is changed and object_id is type incompatible with old object

[Richard Laager]

Steps to reproduce:

1. Create a model ("A") with a GenericForeignKey.
2. Create an instance of A ("a") referencing an instance of model "B" with a PK that is an integer type.
3. 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 and object_id properties, not content_object, i.e.:

       a.content_type_id = ContentType.objects.get_for_model(B).pk
       a.object_id = "foo"
   

4. 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

@@ -242 +242 @@
             ct_match = (
                 ct_id == self.get_content_type(obj=rel_obj, using=instance._state.db).id
             )
-            pk_match = rel_obj._meta.pk.to_python(pk_val) == rel_obj.pk
-            if ct_match and pk_match:
-                return rel_obj
-            else:
-                rel_obj = None
+            if ct_match:
+                pk_match = rel_obj._meta.pk.to_python(pk_val) == rel_obj.pk
+                if pk_match:
+                    return rel_obj
+            rel_obj = None
         if ct_id is not None:
             ct = self.get_content_type(id=ct_id, using=instance._state.db)
             try:

[Mariusz Felisiak]

Thanks for this report, however, as far as I'm aware it's an issue in your code:

    a.content_type_id = ContentType.objects.get_for_model(B)
    a.object_id = "foo"

You should assign id to the content_type_id, not a content type instance. Using:

    a.content_type_id = ContentType.objects.get_for_model(B).id
    a.object_id = "foo"

or

    a.content_type = ContentType.objects.get_for_model(B)
    a.object_id = "foo"

works for me.

[Richard Laager]

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.

[Mariusz Felisiak]

I was able to reproduce the issue with the following test:

tests/generic_relations_regress/tests.py

@@ -314 +314 @@
         result = Link.objects.filter(places=place)
         self.assertCountEqual(result, [link])
 
+    def test_1(self):
+        from django.contrib.contenttypes.models import ContentType
+
+        board = Board.objects.create(name="some test")
+        oddrel = OddRelation1.objects.create(name="clink")
+        charlink = CharLink.objects.create(content_object=oddrel)
+        charlink = CharLink.objects.get(pk=charlink.pk)
+        charlink.content_object
+        charlink.object_id = board.pk
+        charlink.content_type_id = ContentType.objects.get_for_model(Board).id
+        charlink.content_object
+
     def test_generic_reverse_relation_with_abc(self):
         """
         The reverse generic relation accessor (targets) is created if the

[GitHub <noreply@…>]

In e41f9f94:

Fixed #34816 -- Fixed GenericForeignKey crash when checking cache for primary keys with different types.