Opened 7 years ago

Closed 4 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: Ben 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 Ben 7 years ago.
13085.diff (3.4 KB) - added by Ben 7 years ago.
Patch with tests included

Download all attachments as: .zip

Change History (13)

Changed 7 years ago by Ben

Attachment: generic.py.diff added

comment:1 Changed 7 years ago by Russell Keith-Magee

Needs tests: set
Triage Stage: UnreviewedAccepted

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 7 years ago by Ben

Owner: changed from nobody to Ben

Changed 7 years ago by Ben

Attachment: 13085.diff added

Patch with tests included

comment:3 Changed 7 years ago by Ben

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 6 years ago by Gabriel Hurley

Component: Contrib appscontrib.contenttypes

comment:5 Changed 6 years ago by Luke Plant

Type: Bug

comment:6 Changed 6 years ago by Luke Plant

Severity: Normal

comment:7 Changed 5 years ago by Łukasz Rekucki

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

Patch no longer cleanly applies to trunk.

comment:8 Changed 5 years ago by Ben

Owner: Ben deleted

comment:9 Changed 4 years ago by Ramiro Morales

#14325 and #19494 were duplicates.

comment:10 Changed 4 years ago by Ramiro Morales

See also #13137.

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

Owner: set to Ramiro Morales <cramm0@…>
Resolution: fixed
Status: newclosed

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