Opened 12 years ago
Last modified 2 years ago
#17235 assigned Bug
Multipartparser shouldn't leave request.POST/request.FILES mutable
Reported by: | Florian Apolloner | Owned by: | vinay karanam |
---|---|---|---|
Component: | HTTP handling | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | hirokiky@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | 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)
Change History (14)
comment:1 Changed 12 years ago by
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
Cc: | hirokiky@… added |
---|
Changed 10 years ago by
Attachment: | t17235.patch added |
---|
Improved requests POST and FILES to be handled as immutable.
comment:4 Changed 10 years ago by
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 10 years ago by
Has patch: | set |
---|---|
Needs documentation: | set |
Needs tests: | set |
Owner: | changed from nobody to Hiroki Kiyohara |
Patch needs improvement: | set |
Status: | new → assigned |
comment:7 Changed 9 years ago by
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.
comment:8 Changed 7 years ago by
Needs documentation: | unset |
---|---|
Needs tests: | unset |
Owner: | Hiroki Kiyohara deleted |
Patch needs improvement: | unset |
Status: | assigned → new |
comment:10 Changed 6 years ago by
Has patch: | unset |
---|---|
Summary: | Multipartparser shouldn't leave the QueryDict mutable → Multipartparser shouldn't leave request.POST/request.FILES mutable |
The ticket remains open to address request.FILES
.
comment:12 Changed 3 years ago by
Has patch: | set |
---|
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.