Opened 15 months ago

Last modified 15 months ago

#34816 closed Bug

GenericForeignKey crashes if content_type_id is changed and object_id is type incompatible with old object — at Version 2

Reported by: Richard Laager Owned by: nobody
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 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

    diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py
    index 35fcd0d908..e984fb5375 100644
    a b class GenericForeignKey(FieldCacheMixin):  
    242242            ct_match = (
    243243                ct_id == self.get_content_type(obj=rel_obj, using=instance._state.db).id
    244244            )
    245             pk_match = rel_obj._meta.pk.to_python(pk_val) == rel_obj.pk
    246             if ct_match and pk_match:
    247                 return rel_obj
    248             else:
    249                 rel_obj = None
     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
    250250        if ct_id is not None:
    251251            ct = self.get_content_type(id=ct_id, using=instance._state.db)
    252252            try:

Change History (3)

by Richard Laager, 15 months ago

Attachment: gfk-crash.patch added

comment:1 by Mariusz Felisiak, 15 months ago

Has patch: unset
Resolution: invalid
Status: newclosed

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.

comment:2 by Richard Laager, 15 months ago

Description: modified (diff)
Resolution: invalid
Status: closednew

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.

Note: See TracTickets for help on using tickets.
Back to Top