[Django] #22808: ModelMultipleChoiceField does not properly check if value is valid
Django
------------------------------------+-----------------------
Reporter: mattias.lindvall@… | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.5
Severity: Normal | Keywords: modelform
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
------------------------------------+-----------------------
ModelMultipleChoiceField does try to check if value is valid by doing a
query here:
https://github.com/django/django/blob/stable/1.6.x/django/forms/models.py#L1185
However, it only cathes ValueError, assuming that filter() is capable of
even using the value.
If the value is a weird data type like list or dict, the call to filter
throws TypeError.
ModelMultipleChoiceField.clean should catch TypeError, in addition to
ValueError, on line 1187.
Here is gist with a complete minimal app that demonstrates how this can
happen: https://gist.github.com/thnee/8e7c6b22f350582efe57/
Specifically the file: 4. views.py.
It is tested with 1.6.5, using 100% default settings in a clean test
project.
--
Ticket URL: <https://code.djangoproject.com/ticket/22808>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: mattias.lindvall@… | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.5
Keywords: modelform | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by maxocub):
* cc: maxime.turcotte@… (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/22808#comment:1>
Django
Reporter: mattias.lindvall@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: modelform | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
------------------------------------+--------------------------------------
Changes (by maxocub):
* version: 1.5 => 1.6
* easy: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/22808#comment:2>
Django
Reporter: mattias.lindvall@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
to-commit | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by EvilDMP):
* keywords: modelform => modelform, afraid-to-commit
--
Ticket URL: <https://code.djangoproject.com/ticket/22808#comment:3>
Django
Reporter: mattias.lindvall@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
to-commit | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by bmispelon):
* stage: Unreviewed => Accepted
Comment:
Hi,
Looking at `django/forms/fields.py`, it seems that there is precedent for
catching `(ValueError, TypeError)` so I think it makes sense.
The same error seems to be present in `ModelChoiceField` so both should be
fixed as part of this ticket.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/22808#comment:4>
Django
Reporter: mattias.lindvall@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: modelform, afraid- | Triage Stage: Accepted
to-commit | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by anonymous):
Created pull request: https://github.com/django/django/pull/2813
--
Ticket URL: <https://code.djangoproject.com/ticket/22808#comment:5>
Django
Reporter: mattias.lindvall@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: modelform, afraid- | Triage Stage: Accepted
to-commit | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by timo):
* needs_tests: 0 => 1
Comment:
It needs a test (or tests). Please uncheck "Needs tests" if you can update
it, thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/22808#comment:6>
Django
Reporter: mattias.lindvall@… | Owner: nip3o
Type: Bug | Status: assigned
Component: Forms | Version: 1.6
Severity: Normal | Resolution:
Keywords: modelform, afraid- | Triage Stage: Accepted
to-commit | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
* owner: nobody => nip3o
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/22808#comment:7>
Django
Reporter: mattias.lindvall@… | Owner: nip3o
Type: Bug | Status: assigned
Component: Forms | Version: 1.6
Severity: Normal | Resolution:
Keywords: modelform, afraid- | Triage Stage: Accepted
to-commit | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by nip3o):
* has_patch: 0 => 1
* needs_tests: 1 => 0
Comment:
New PR with the existing patch applied and including test cases.
https://github.com/django/django/pull/2964
--
Ticket URL: <https://code.djangoproject.com/ticket/22808#comment:8>
Django
Reporter: mattias.lindvall@… | Owner: nip3o
Component: Forms | Version: 1.6
Severity: Normal | Resolution: fixed
Keywords: modelform, afraid- | Triage Stage: Accepted
to-commit | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"cdc25ac4747bf5a6cdc2e70461c2d43c54529d35"]:
{{{
#!CommitTicketReference repository=""
revision="cdc25ac4747bf5a6cdc2e70461c2d43c54529d35"
Fixed #22808 -- Made ModelMultipleChoiceField validation more robust to
invalid data types..
Thanks Mattias Lindvall for the report and inital patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22808#comment:9>