Code

Opened 2 years ago

Last modified 6 months ago

#17235 assigned Bug

Multipartparser shouldn't leave the QueryDict mutable

Reported by: apollo13 Owned by: hirokiky
Component: HTTP handling Version: master
Severity: Normal Keywords:
Cc: hirokiky@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Currently the multipart parser constructs QueryDicts for POST and FILES as mutable. Since we discourage users to change QueryDicts (and don't allow it for GET and normal POST) the parser should change the flag to False before returning it. This way multipart POSTs would be more consistent with normal POSTs which aren't mutable.

Attachments (1)

t17235.patch (3.7 KB) - added by hirokiky 15 months ago.
Improved requests POST and FILES to be handled as immutable.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 2 years ago by lukeplant

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

comment:2 Changed 15 months ago by anonymous

FILES is constructed by MultiValueDict, not QueryDict. MultiValueDict cannot be handle as mutable.
So, It seems some new class (for example, ImmutableMultiValueDict) will be needed.

comment:3 Changed 15 months ago by hirokiky

  • Cc hirokiky@… added

Changed 15 months ago by hirokiky

Improved requests POST and FILES to be handled as immutable.

comment:4 Changed 15 months ago by hirokiky

I added a patch.

But the tests raised 4 errors.
For example...

ERROR: testEditSaveAs (regressiontests.admin_views.tests.AdminViewBasicTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/hirokiky/django/kydjango/tests/regressiontests/admin_views/tests.py", line 225, in testEditSaveAs
    response = self.client.post('/test_admin/%s/admin_views/section/1/' % self.urlbit, post_data)
  File "/home/hirokiky/django/kydjango/tests/django/test/client.py", line 463, in post
    response = super(Client, self).post(path, data=data, content_type=content_type, **extra)
  File "/home/hirokiky/django/kydjango/tests/django/test/client.py", line 297, in post
    return self.request(**r)
  File "/home/hirokiky/django/kydjango/tests/django/test/client.py", line 424, in request
    six.reraise(*exc_info)
  File "/home/hirokiky/django/kydjango/tests/django/utils/six.py", line 313, in reraise
    raise value
  File "/home/hirokiky/django/kydjango/tests/django/core/handlers/base.py", line 115, in get_response
    response = callback(request, *callback_args, **callback_kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/contrib/admin/options.py", line 371, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/utils/decorators.py", line 91, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/views/decorators/cache.py", line 52, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/contrib/admin/sites.py", line 202, in inner
    return view(request, *args, **kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/utils/decorators.py", line 25, in _wrapper
    return bound_func(*args, **kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/utils/decorators.py", line 91, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/utils/decorators.py", line 21, in bound_func
    return func(self, *args2, **kwargs2)
  File "/home/hirokiky/django/kydjango/tests/django/db/transaction.py", line 208, in inner
    return func(*args, **kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/contrib/admin/options.py", line 1064, in change_view
    current_app=self.admin_site.name))
  File "/home/hirokiky/django/kydjango/tests/django/utils/decorators.py", line 25, in _wrapper
    return bound_func(*args, **kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/utils/decorators.py", line 91, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/utils/decorators.py", line 21, in bound_func
    return func(self, *args2, **kwargs2)
  File "/home/hirokiky/django/kydjango/tests/django/db/transaction.py", line 208, in inner
    return func(*args, **kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/contrib/admin/options.py", line 989, in add_view
    prefix=prefix, queryset=inline.queryset(request))
  File "/home/hirokiky/django/kydjango/tests/django/forms/models.py", line 717, in __init__
    queryset=qs, **kwargs)
  File "/home/hirokiky/django/kydjango/tests/django/forms/models.py", line 441, in __init__
    super(BaseModelFormSet, self).__init__(**defaults)
  File "/home/hirokiky/django/kydjango/tests/django/forms/formsets.py", line 53, in __init__
    self._construct_forms()
  File "/home/hirokiky/django/kydjango/tests/django/forms/formsets.py", line 122, in _construct_forms
    self.forms.append(self._construct_form(i))
  File "/home/hirokiky/django/kydjango/tests/django/forms/models.py", line 730, in _construct_form
    form.data[form.add_prefix(self._pk_field.name)] = None
  File "/home/hirokiky/django/kydjango/tests/django/http/request.py", line 304, in __setitem__
    self._assert_mutable()
  File "/home/hirokiky/django/kydjango/tests/django/http/request.py", line 301, in _assert_mutable
    raise AttributeError("This QueryDict instance is immutable")
AttributeError: This QueryDict instance is immutable

like this.

One idea solving this is putting a line on a setting to avoid constructing request as immutable.
And next step is solving these errors to handle request as immutable clearly.
Finally, delete the avoiding setting.

Any ideas?

comment:5 Changed 15 months ago by hirokiky

  • Has patch set
  • Needs documentation set
  • Needs tests set
  • Owner changed from nobody to hirokiky
  • Patch needs improvement set
  • Status changed from new to assigned

comment:6 Changed 6 months ago by claudep

#18553 was marked as a duplicate

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as assigned
The owner will be changed from hirokiky to anonymous. Next status will be 'assigned'
The ticket will be disowned. Next status will be 'new'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.