Opened 5 years ago

Closed 2 years ago

#13085 closed Bug (fixed)

GenericForeignKey.get_content_type fails if `object` or `id` evaluate to False. Should use 'is not None' test.

Reported by: benreynwar Owned by: Ramiro Morales <cramm0@…>
Component: contrib.contenttypes Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

contenttypes.GenericForeignKey.get_content_type is used to find the content type of an object. If the object evaluates to False, then an error is raised. This is because 'if obj:' is used, rather than 'if obj is not None:'. The same problem exists for the 'id' argument, and I believe also in the __get__ method.

Attachments (2)

generic.py.diff (1.2 KB) - added by benreynwar 5 years ago.
13085.diff (3.4 KB) - added by benreynwar 5 years ago.
Patch with tests included

Download all attachments as: .zip

Change History (13)

Changed 5 years ago by benreynwar

comment:1 Changed 5 years ago by russellm

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Two quick points:

1) Please generate patches relative to the root of the source tree
2) Please provide a test case, not just a patch. Under what conditions will the object be false? Ideally, this should be in the form of a regression test integrated into Django's test suite, but if you can't work out how to do that, a code sample will do.

Accepting because I think I can spot the problem case (getting the content type for an unsaved object)

comment:2 Changed 5 years ago by benreynwar

  • Owner changed from nobody to benreynwar

Changed 5 years ago by benreynwar

Patch with tests included

comment:3 Changed 5 years ago by benreynwar

  • Needs tests unset

I've included a new patch with a regression test included. The object will be false if it has nonzero is defined and returns False, or if len is defined and returns 0. Either of these should be possible without generic objects failing. The bug would also cause problems if a generic object was used with content_type_id=0.

comment:4 Changed 4 years ago by gabrielhurley

  • Component changed from Contrib apps to contrib.contenttypes

comment:5 Changed 4 years ago by lukeplant

  • Type set to Bug

comment:6 Changed 4 years ago by lukeplant

  • Severity set to Normal

comment:7 Changed 4 years ago by lrekucki

  • Easy pickings unset
  • Patch needs improvement set
  • UI/UX unset

Patch no longer cleanly applies to trunk.

comment:8 Changed 4 years ago by benreynwar

  • Owner benreynwar deleted

comment:9 Changed 3 years ago by ramiro

#14325 and #19494 were duplicates.

comment:10 Changed 2 years ago by ramiro

See also #13137.

comment:11 Changed 2 years ago by Ramiro Morales <cramm0@…>

  • Owner set to Ramiro Morales <cramm0@…>
  • Resolution set to fixed
  • Status changed from new to closed

In 04d9730b127c689b8eda01cbc913efa6e2eb230b:

Fixed #13085 -- Don't fail on creation of model with GFK to a model with len() returning zero.

Also, according to the comments on the ticket and its duplicates, added
tests execising saving an instance of a model with a GFK to:

  • An unsaved object -- This actually doesn't generate the same failure but another ORM-level exception. The test verifies it's the case.
  • An instance of a model with a nonzero() method thant returns False for it. This doesn't fail because that code path isn't executed.
  • An instance of a model with a CharField PK and an empty value for it. This doesn't fail.
Note: See TracTickets for help on using tickets.
Back to Top