Opened 6 years ago

Closed 6 years ago

#27198 closed Bug (fixed)

QueryDict getlist allows data to be mutated

Reported by: Fraser Nevett Owned by: Jani Tiainen
Component: HTTP handling Version: 1.10
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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))

Attachments (1)

27198-test.diff (686 bytes) - added by Tim Graham 6 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 6 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

Agreed that doesn't seem correct. Attached is a regression test for Django's test suite that currently fails.

Changed 6 years ago by Tim Graham

Attachment: 27198-test.diff added

comment:2 Changed 6 years ago by Jani Tiainen

Owner: changed from nobody to Jani Tiainen
Status: newassigned

comment:3 Changed 6 years ago by Jani Tiainen

Has patch: set

comment:4 Changed 6 years ago by Jani Tiainen

Actual implementation is done in MultiValueDict instead of QueryDict.

comment:5 Changed 6 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 727d7ce6:

Fixed #27198 -- Made MultiValueDict.getlist() return a new list to prevent mutation.

Note: See TracTickets for help on using tickets.
Back to Top