Fix DictField HTML input returning empty dict for missing fields#9891
Fix DictField HTML input returning empty dict for missing fields#9891veeceey wants to merge 5 commits intoencode:mainfrom
Conversation
…input (encode#6234) When using DictField with HTML form (multipart/form-data) input, parse_html_dict always returned an empty MultiValueDict when no matching keys were found. This made it impossible to distinguish between an unspecified field and an empty input, causing issues with required/default field handling. This aligns parse_html_dict with parse_html_list by adding a default parameter that is returned when no matching keys are found. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hi maintainers, friendly ping on this PR. It's been open for about 10 days without any review activity. Would really appreciate any feedback or direction when you get a chance. Happy to make adjustments if needed. Thank you! |
rest_framework/fields.py
Outdated
| """ | ||
| if html.is_html_input(data): | ||
| data = html.parse_html_dict(data) | ||
| data = html.parse_html_dict(data, default=data) |
There was a problem hiding this comment.
Wouldn't a better default be an empty dict here?
| data = html.parse_html_dict(data, default=data) | |
| data = html.parse_html_dict(data, default={}) |
There was a problem hiding this comment.
I tried this but it actually breaks test_querydict_dict_input. When to_internal_value gets called, the data is already a MultiValueDict with parsed keys. parse_html_dict with no prefix looks for dot-prefixed keys which don't match, so default={} would lose the already-parsed data. Keeping default=data as a fallback preserves it correctly. Happy to discuss further though!
|
|
||
|
|
||
| def parse_html_dict(dictionary, prefix=''): | ||
| def parse_html_dict(dictionary, prefix='', default=None): |
There was a problem hiding this comment.
Looks consistent with what we do for parsing html list 👍🏻 :
There was a problem hiding this comment.
Pull request overview
This pull request fixes a bug where DictField could not distinguish between a field that was not specified in HTML form data versus a field that was specified but had no values. This caused issues with required fields, default values, and partial updates when using multipart/form-data requests.
Changes:
- Added a
defaultparameter toparse_html_dict()to return a specified value when no matching keys are found (consistent withparse_html_list) - Updated
DictField.get_value()andSerializer.get_value()to passdefault=emptyso missing fields are correctly detected - Updated
DictField.to_internal_value()to passdefault=datato preserve inner MultiValueDict data when no nested keys exist - Added comprehensive test coverage for DictField HTML form input scenarios
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| rest_framework/utils/html.py | Added default parameter to parse_html_dict() to handle missing fields correctly |
| rest_framework/fields.py | Updated DictField.get_value() and to_internal_value() to use the new default parameter |
| rest_framework/serializers.py | Updated Serializer.get_value() to use explicit default=empty parameter for consistency |
| tests/test_fields.py | Added 4 test cases covering various DictField HTML form input scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for the review! I'll address all three points:
Pushing the fixes now. |
- Add regression test for nested serializer with QueryDict to cover
the serializers.py get_value change (test_nested_serializer_not_required_with_querydict)
- Use `assert serializer.is_valid(), serializer.errors` for better
test output on failures
- Keep `default=data` in to_internal_value as `default={}` would
discard already-parsed MultiValueDict keys from get_value
|
Hey @browniebroke, pushed the updates:
|
Change default from data to {} in parse_html_dict call within
DictField.to_internal_value(), falling back to a dict conversion
of the MultiValueDict when no dot-separated keys are found. This
is cleaner than using the input data as its own default.
|
Hey @browniebroke, circling back on the default={} suggestion — I ended up finding a way to make it work after all. Pushed a commit that uses |
browniebroke
left a comment
There was a problem hiding this comment.
The scope of this PR is doing more than fixing the bug reported in #6234, it is actually improving support for nested objects in HTML forms which was requested a long time ago.
I'm not convinced this is worth doing in terms of risking a regression but will keep this open to revisit in a while
| def test_querydict_dict_input(self): | ||
| """ | ||
| DictField should correctly parse HTML form (QueryDict) input | ||
| with dot-separated keys. | ||
| """ | ||
| class TestSerializer(serializers.Serializer): | ||
| data = serializers.DictField(child=serializers.CharField()) | ||
|
|
||
| serializer = TestSerializer(data=QueryDict('data.a=1&data.b=2')) | ||
| assert serializer.is_valid(), serializer.errors | ||
| assert serializer.validated_data == {'data': {'a': '1', 'b': '2'}} |
|
I was leaning more towards #9906 because the fix was more targeted, but after more careful review I can see some limits to that other simpler approach, which has some regression. Although this one seemingly has the same issue, how do one clears a value from a def test_partial_update_can_clear_html_dict_field(self):
class TestSerializer(serializers.Serializer):
field_name = serializers.DictField(required=False)
serializer = TestSerializer(data=QueryDict('field_name='), partial=True)
assert serializer.is_valid()
assert 'field_name' in serializer.validated_data |
Summary
Fixes #6234
parse_html_dictalways returned an emptyMultiValueDictwhen no matching keys were found in the HTML form input, making it impossible to distinguish between:This caused
DictFieldto treat missing fields as if they contained an empty dict, breakingrequired,default, and partial update logic formultipart/form-datarequests.Changes
rest_framework/utils/html.py: Added adefaultparameter toparse_html_dict(consistent with howparse_html_listalready works). Returnsdefaultwhen no matching keys are found in the input.rest_framework/fields.py: UpdatedDictField.get_valueto passdefault=emptyso that missing fields are correctly treated as not provided. Also updatedDictField.to_internal_valueto passdefault=dataso inner MultiValueDict data is preserved when no dot-separated sub-keys are found.rest_framework/serializers.py: UpdatedSerializer.get_valueto use the newdefault=parameter instead ofor emptyfor consistency.tests/test_fields.py: Added 4 new test cases for DictField HTML form input covering:Test plan
test_fields.py,test_serializer.py, andtest_parsers.pysuites pass (401 passed, 1 pre-existing failure unrelated to this change)multipart/form-datarequests that DictField partial updates work correctly