Refs #17235 -- Made MultiPartParser leave request.FILES immutable. #17991
Conversation
| @@ -361,7 +361,7 @@ def _parse(self): | |||
| # Signal that the upload has completed. | |||
| # any() shortcircuits if a handler's upload_complete() returns a value. | |||
| any(handler.upload_complete() for handler in handlers) | |||
| self._post._mutable = False | |||
| self._post._mutable = self._files._mutable = False | |||
There was a problem hiding this comment.
| self._post._mutable = self._files._mutable = False | |
| self._post._mutable = False | |
| self._files._mutable = False |
I find this easier to read
| @@ -69,8 +69,11 @@ class MultiValueDict(dict): | |||
| single name-value pairs. | |||
| """ | |||
|
|
|||
| def __init__(self, key_to_list_mapping=()): | |||
| _mutable = True | |||
There was a problem hiding this comment.
| _mutable = True | |
| # This is reset in __init__, but is specified here at the class level so | |
| # that unpickling will have valid values | |
| _mutable = True |
Think maybe we should have the same comment as QueryDict here
| def __init__(self, key_to_list_mapping=()): | ||
| _mutable = True | ||
|
|
||
| def __init__(self, key_to_list_mapping=(), mutable=True): |
There was a problem hiding this comment.
In QueryDict we have mutable=False as the default, have you had a look as to whether that would make sense here?
| def pop(self, key, *args): | ||
| self._assert_mutable() | ||
| return super().pop(key, *args) | ||
|
|
||
| def popitem(self): | ||
| self._assert_mutable() | ||
| return super().popitem() | ||
|
|
||
| def clear(self): | ||
| self._assert_mutable() | ||
| super().clear() |
There was a problem hiding this comment.
As QueryDict inherits MultiValueDict, you might be able to remove/refactor some of the code there.
Might be more than just these
|
I wonder about backward compatability here. It's been this way forever so, while it's not recommended, I'm sure someone will be relying on it being mutable. One suggestion would be to add this change in behaviour as part of the new content type parsing (#17546). The next step there is for someone to write a DEP so this behaviour change could form part of that. |
Oh you make a good point 🤔
@bcail what do you think? Being part of the push for #17546 would be amazing. I really appreciate the work you've been doing and I think having a look at this and the DEP process could be a valuable experience if you're up for it? |
Trac ticket number
ticket-17235
Branch description
Made request.FILES immutable. Continued the work from PR #10110 by @vinayinvicible.
Checklist
mainbranch.