[Django] #34816: GenericForeignKey crashes if content_type_id is changed and object_id is type incompatible with old object
Django
type incompatible with old object
------------------------------------------------+------------------------
Reporter: Richard Laager | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.contenttypes | Version: 4.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
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.:
{{{#!python
a.content_type_id = ContentType.objects.get_for_model(B)
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):
{{{#!python
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):
{{{#!diff
diff --git a/django/contrib/contenttypes/fields.py
b/django/contrib/contenttypes/fields.py
index 35fcd0d908..e984fb5375 100644
--- a/django/contrib/contenttypes/fields.py
+++ b/django/contrib/contenttypes/fields.py
@@ -242,11 +242,11 @@ class GenericForeignKey(FieldCacheMixin):
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:
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34816>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
type incompatible with old object
Reporter: Richard Laager | Owner: nobody
Type: Uncategorized | Status: new
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Richard Laager):
* Attachment "gfk-crash.patch" added.
Django
type incompatible with old object
-------------------------------------+-------------------------------------
Reporter: Richard Laager | Owner: nobody
Component: | Version: 4.2
contrib.contenttypes |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* has_patch: 1 => 0
* resolution: => invalid
Comment:
Thanks for this report, however, as far as I'm aware it's an issue in your
code:
{{{#!python
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:
{{{#!python
a.content_type_id = ContentType.objects.get_for_model(B).id
a.object_id = "foo"
}}}
or
{{{#!python
a.content_type = ContentType.objects.get_for_model(B)
a.object_id = "foo"
}}}
works for me.
--
Ticket URL: <https://code.djangoproject.com/ticket/34816#comment:1>
Django
type incompatible with old object
-------------------------------------+-------------------------------------
Reporter: Richard Laager | Owner: nobody
Component: | Version: 4.2
contrib.contenttypes |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: invalid =>
Old description:
> 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.:
> {{{#!python
> a.content_type_id = ContentType.objects.get_for_model(B)
> a.object_id = "foo"
> }}}
New description:
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.:
{{{#!python
a.content_type_id = ContentType.objects.get_for_model(B).pk
Expected result:
Actual result:
--
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/34816#comment:2>
Django
type incompatible with old object
Reporter: Richard Laager | Owner: nobody
Component: contrib.contenttypes | Version: 4.2
Severity: Normal | Resolution:
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 Mariusz Felisiak):
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted
Comment:
I was able to reproduce the issue with the following test:
{{{#!diff
diff --git a/tests/generic_relations_regress/tests.py
b/tests/generic_relations_regress/tests.py
index b7ecb499eb..7b7883ee88 100644
--- a/tests/generic_relations_regress/tests.py
+++ b/tests/generic_relations_regress/tests.py
@@ -314,6 +314,18 @@ class GenericRelationTests(TestCase):
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34816#comment:3>
Django
type incompatible with old object
Reporter: Richard Laager | Owner: Oguzhan
| Akan
Type: Bug | Status: assigned
Component: | Version: 4.2
contrib.contenttypes |
Severity: Normal | Resolution:
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 Oguzhan Akan):
* owner: nobody => Oguzhan Akan
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34816#comment:4>
Django
type incompatible with old object
-------------------------------------+-------------------------------------
Reporter: Richard Laager | Owner: Oguzhan
| Akan
Type: Bug | Status: assigned
Component: | Version: 4.2
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34816#comment:5>
Django
type incompatible with old object
-------------------------------------+-------------------------------------
Reporter: Richard Laager | Owner: Oguzhan
| Akan
Type: Bug | Status: assigned
Component: | Version: 4.2
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
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34816#comment:6>
Django
type incompatible with old object
-------------------------------------+-------------------------------------
Reporter: Richard Laager | Owner: Oguzhan
| Akan
Component: | Version: 4.2
contrib.contenttypes |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e41f9f94505f690525f16af8bd10e62e22bc0207" e41f9f94]:
{{{
#!CommitTicketReference repository=""
revision="e41f9f94505f690525f16af8bd10e62e22bc0207"
Fixed #34816 -- Fixed GenericForeignKey crash when checking cache for
primary keys with different types.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34816#comment:7>