Fixed issue #5926: Ensure that html forms (multipart form data) respe… by anx-ckreuzberger · Pull Request #5927 · encode/django-rest-framework (original) (raw)

I'm describing all calls of html.parse_html_list in the order that @tomchristie requested.

TL;DR: I found a potential issue in ListSerializer.to_internal_value while reviewing my changes, so please hold on merging.

1: rest_framework/fields.py - ListField

ListField.get_value

Here we are using return html.parse_html_list(dictionary, prefix=self.field_name, default=empty) to copy the behaviour for the case that the input is not a multipart-form (html input), but JSON/XML. Look at the following source code:

https://github.com/anx-ckreuzberger/django-rest-framework/blob/1cfd42d46c13bc2ad7639cc8e642ee79e8e30af7/rest_framework/fields.py#L1606-L1619

It says return dictionary.get(self.field_name, empty), meaning that if the requested field is not listed in the dictionary, it should return empty. So when parse_html_list is not able to find the requested field, it will also return empty.

ListField.to_internal_value

Here we are using data = html.parse_html_list(data, default=[]) (used to be data = html.parse_html_list(data). This part would have returned an empty list before my changes. We've changed the behaviour of parse_html_list, such that it would return None instead of an empty list, hence we are overriding this by setting the param default=[].
Furthermore, this method requires to always return a list (or something similar to a list).

2. rest_framework/serializers.py - ListSerializer

ListSerializer.get_value

Here we are using return html.parse_html_list(dictionary, prefix=self.field_name, default=empty).
Same as with ListField.get_value, ListSerializer.get_value returns emptyin the case of non multipart-form:
return dictionary.get(self.field_name, empty)
So we are just making sure that the behaviour is the same here.

ListSerializer.to_internal_value

Here are using data = html.parse_html_list(data, default=empty). This method of ListSerializer expects a list, else it will raise an Exception:
https://github.com/anx-ckreuzberger/django-rest-framework/blob/1cfd42d46c13bc2ad7639cc8e642ee79e8e30af7/rest_framework/serializers.py#L640-L646

I am a little bit unsure on why I am having default=empty here instead of default=[] (I guess it should be the same as in ListField.to_internal_value).
I will try to verify that tomorrow, so please hold on merging until I can confirm that this behaviour is right, or wrong. Maybe I'll have to add a unit test for it.