Multipartparser shouldn't leave request.POST/request.FILES mutable

[Florian Apolloner]

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.

[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.

[Hiroki Kiyohara]

Improved requests POST and FILES to be handled as immutable.

[Hiroki Kiyohara]

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?

[Claude Paroz]

#18553 was marked as a duplicate

[Tom Christie]

Nothing concrete to help progress the ticket at this point, but chiming in that I do agree that having request.POST and request.FILES as immutable data structures would be an improvement.

[Tim Graham]

​PR

[Tim Graham <timograham@…>]

In 4a246a0:

Refs #17235 -- Made MultiPartParser leave request.POST immutable.

[Tim Graham]

The ticket remains open to address request.FILES.

[vinay karanam]

​PR

[Mariusz Felisiak]

Per ​Florian's comment.

[bcail]

I opened a ​new PR, based on the previous one. I updated the code to make MultiValueDict able to be mutable or immutable, like QueryDict. Is that a good direction to go?

[Sarah Boyce]

Is that a good direction to go?

I think it looks good to me as a direction, added some comments to the PR.