[Django] #27198: QueryDict getlist allows data to be mutated
Django
-------------------------------+--------------------
Reporter: frasern | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
Calling `getlist()` on a `QueryDict` simply returns the underlying `list`,
which means it can be mutated:
{{{
>>> q = QueryDict('a=1&a=2&a=3', mutable=False)
>>> a = q.getlist('a')
>>> a
[u'1', u'2', u'3']
>>> a.append(u'4')
>>> q.getlist('a')
[u'1', u'2', u'3', u'4']
>>> q
<QueryDict: {u'a': [u'1', u'2', u'3', u'4']}>
}}}
I encountered this unexpected behaviour in code using the following
pattern:
{{{
values = request.POST.getlist('a')
values += request.POST.getlist('b')
values += request.POST.getlist('c')
}}}
This results in `request.POST` being updated so that ''a'' now also
contains the values for ''b'' and ''c''.
Given `request.GET` and `request.POST` are created as '''immutable'''
`QueryDict` objects, I would not expect this behaviour.
At present, `getlist` is inherited from `MultiValueDict`. I think the fix
would be to add a `getlist` method to `QueryDict` that forces a new `list`
to be created:
{{{
def setlist(self, key, default=None):
return list(super(QueryDict, self).getlist(key, default))
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27198>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: frasern | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: 1.10
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 timgraham):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
Agreed that doesn't seem correct. Attached is a regression test for
Django's test suite that currently fails.
--
Ticket URL: <https://code.djangoproject.com/ticket/27198#comment:1>
Django
Reporter: frasern | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* Attachment "27198-test.diff" added.
Django
Reporter: frasern | Owner: jtiai
Type: Bug | Status: assigned
Component: HTTP handling | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jtiai):
* owner: nobody => jtiai
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/27198#comment:2>
Django
Reporter: frasern | Owner: jtiai
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jtiai):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27198#comment:3>
Django
Reporter: frasern | Owner: jtiai
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by jtiai):
Actual implementation is done in MultiValueDict instead of QueryDict.
--
Ticket URL: <https://code.djangoproject.com/ticket/27198#comment:4>
Django
Reporter: frasern | Owner: jtiai
Component: HTTP handling | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"727d7ce6cba21363470aaefb2dc5353017531be3" 727d7ce6]:
{{{
#!CommitTicketReference repository=""
revision="727d7ce6cba21363470aaefb2dc5353017531be3"
Fixed #27198 -- Made MultiValueDict.getlist() return a new list to prevent
mutation.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27198#comment:5>