[Django] #17235: Multipartparser shouldn't leave the QueryDict mutable
Django
-------------------------------+--------------------
Reporter: apollo13 | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: SVN
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/17235>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Type: Bug | Status: new
Component: HTTP handling | Version: SVN
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by lukeplant):
* needs_docs: => 0
* needs_better_patch: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:1>
Django
Reporter: apollo13 | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:2>
Django
Reporter: apollo13 | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hirokiky):
* cc: hirokiky@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:3>
Django
Reporter: apollo13 | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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?
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:4>
Django
Reporter: apollo13 | Owner: hirokiky
Type: Bug | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by hirokiky):
* status: new => assigned
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
* owner: nobody => hirokiky
* needs_docs: 0 => 1
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:5>
Django
Reporter: apollo13 | Owner: hirokiky
Type: Bug | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by claudep):
#18553 was marked as a duplicate
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:6>
Django
Reporter: apollo13 | Owner: hirokiky
Type: Bug | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by tomchristie):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:7>
Django
Reporter: Florian Apolloner | Owner: (none)
Type: Bug | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* owner: Hiroki Kiyohara => (none)
* needs_better_patch: 1 => 0
* status: assigned => new
* needs_tests: 1 => 0
* needs_docs: 1 => 0
Comment:
[https://github.com/django/django/pull/7565 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:8>
Django
Reporter: Florian Apolloner | Owner: (none)
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"4a246a02bdcbc13b15480c014f51cb0682af7c1e" 4a246a0]:
{{{
#!CommitTicketReference repository=""
revision="4a246a02bdcbc13b15480c014f51cb0682af7c1e"
Refs #17235 -- Made MultiPartParser leave request.POST immutable.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:9>
Django
-----------------------------------+------------------------------------
Reporter: Florian Apolloner | Owner: (none)
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* has_patch: 1 => 0
Comment:
The ticket remains open to address `request.FILES`.
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Florian Apolloner | Owner: vinay
| karanam
Type: Bug | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by vinay karanam):
* owner: (none) => vinay karanam
* status: new => assigned
Comment:
[https://github.com/django/django/pull/10110 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:11>
Django
-------------------------------------+-------------------------------------
| karanam
Type: Bug | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:12>
Django
-------------------------------------+-------------------------------------
| karanam
Type: Bug | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 0 => 1
Comment:
Per [https://github.com/django/django/pull/10110#issuecomment-775851908
Florian's comment].
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:13>
Django
-----------------------------------+------------------------------------
Type: Bug | Status: assigned
Component: HTTP handling | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Changes (by bcail):
* cc: bcail (added)
Comment:
I opened a [https://github.com/django/django/pull/17991 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:14>
Django
-----------------------------------+------------------------------------
Reporter: Florian Apolloner | Owner: bcail
Type: Bug | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Changes (by Sarah Boyce):
* needs_better_patch: 0 => 1
Comment:
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:15>
Django
-----------------------------------+--------------------------------------
Reporter: Florian Apolloner | Owner: Hridesh MG
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Hridesh MG):
* owner: bcail => Hridesh MG
Comment:
Hi there, I'll give this a try as per
https://github.com/django/django/pull/17991#issuecomment-2107642203. Let
me know if you'd prefer to continue working on it yourself!
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:16>
Django
-----------------------------------+------------------------------------
Reporter: Florian Apolloner | Owner: bcail
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Hridesh MG):
* owner: Hridesh MG => bcail
Comment:
Apologies, in my haste i missed reading this
[https://github.com/django/django/pull/17991#issuecomment-2076674420
comment] on the PR. Since I'm still a beginner, I'll try to pick an easier
issue to contribute to. Sorry for the noise.
--
Ticket URL: <https://code.djangoproject.com/ticket/17235#comment:17>