From 0a167a54fdbf9ceba2a1513636a50bf4c89efa66 Mon Sep 17 00:00:00 2001 From: Alen Mujezinovic Date: Tue, 17 Jan 2012 10:58:42 +0000 Subject: [PATCH 1/4] Added an additional attribute `unknown_form_fields` to `FormResource` If the attribute is set to `True`, the validation method will not raise an `ErrorResponse` with status 400 but silently strip out unexpected fields on the form and only return the validated data. --- djangorestframework/resources.py | 13 +++++++++++-- djangorestframework/tests/validators.py | 12 ++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index 68b285b91..d2651036c 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -78,13 +78,22 @@ class FormResource(Resource): This can be overridden by a :attr:`form` attribute on the :class:`views.View`. """ + unknown_form_fields = False + """ + Flag to check for unknown fields when validating a form. If set to false and + we receive request data that is not expected by the form it raises an + :exc:`response.ErrorResponse` with status code 400. If set to true, only + expected fields are validated. + """ + def validate_request(self, data, files=None): """ Given some content as input return some cleaned, validated content. Raises a :exc:`response.ErrorResponse` with status code 400 (Bad Request) on failure. - Validation is standard form validation, with an additional constraint that *no extra unknown fields* may be supplied. + Validation is standard form validation, with an additional constraint that *no extra unknown fields* may be supplied + if :attr:`self.unknown_form_fields` is ``True``. On failure the :exc:`response.ErrorResponse` content is a dict which may contain :obj:`'errors'` and :obj:`'field-errors'` keys. If the :obj:`'errors'` key exists it is a list of strings of non-field errors. @@ -132,7 +141,7 @@ class FormResource(Resource): unknown_fields = unknown_fields - set(('csrfmiddlewaretoken', '_accept', '_method')) # TODO: Ugh. # Check using both regular validation, and our stricter no additional fields rule - if bound_form.is_valid() and not unknown_fields: + if bound_form.is_valid() and (self.unknown_form_fields or not unknown_fields): # Validation succeeded... cleaned_data = bound_form.cleaned_data diff --git a/djangorestframework/tests/validators.py b/djangorestframework/tests/validators.py index 18c8c3137..c316f209a 100644 --- a/djangorestframework/tests/validators.py +++ b/djangorestframework/tests/validators.py @@ -138,6 +138,14 @@ class TestFormValidation(TestCase): content = {'qwerty': 'uiop', 'extra': 'extra'} validator._validate(content, None, allowed_extra_fields=('extra',)) + def validation_allows_unknown_fields_if_explicitly_allowed(self, validator): + """If we set ``unknown_form_fields`` on the form resource, then don't + raise errors on unexpected request data""" + content = {'qwerty': 'uiop', 'extra': 'extra'} + validator.unknown_form_fields = True + self.assertDictEqual({'qwerty': u'uiop'}, validator.validate_request(content, None), "Resource didn't accept unknown fields.") + validator.unknown_form_fields = False + def validation_does_not_require_extra_fields_if_explicitly_set(self, validator): """If we include an allowed_extra_fields paramater on _validate, then do not fail if we do not have fields with those names.""" content = {'qwerty': 'uiop'} @@ -201,6 +209,10 @@ class TestFormValidation(TestCase): def test_validation_allows_extra_fields_if_explicitly_set(self): validator = self.MockFormResource(self.MockFormView()) self.validation_allows_extra_fields_if_explicitly_set(validator) + + def test_validation_allows_unknown_fields_if_explicitly_allowed(self): + validator = self.MockFormResource(self.MockFormView()) + self.validation_allows_unknown_fields_if_explicitly_allowed(validator) def test_validation_does_not_require_extra_fields_if_explicitly_set(self): validator = self.MockFormResource(self.MockFormView()) From 167dce008cc967755a2a433b710207a7ae7315af Mon Sep 17 00:00:00 2001 From: Alen Mujezinovic Date: Tue, 17 Jan 2012 11:08:44 +0000 Subject: [PATCH 2/4] Documentation error. --- djangorestframework/resources.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index d2651036c..de3699488 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -93,7 +93,7 @@ class FormResource(Resource): Raises a :exc:`response.ErrorResponse` with status code 400 (Bad Request) on failure. Validation is standard form validation, with an additional constraint that *no extra unknown fields* may be supplied - if :attr:`self.unknown_form_fields` is ``True``. + if :attr:`self.unknown_form_fields` is ``False``. On failure the :exc:`response.ErrorResponse` content is a dict which may contain :obj:`'errors'` and :obj:`'field-errors'` keys. If the :obj:`'errors'` key exists it is a list of strings of non-field errors. From ec5badf739908f5c26691a47955d79c42d5a39df Mon Sep 17 00:00:00 2001 From: Alen Mujezinovic Date: Thu, 19 Jan 2012 15:52:26 +0000 Subject: [PATCH 3/4] Renamed `unknown_form_fields` to `allow_unknown_form_fields` --- djangorestframework/resources.py | 8 ++++---- djangorestframework/tests/validators.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index de3699488..2e855021d 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -78,7 +78,7 @@ class FormResource(Resource): This can be overridden by a :attr:`form` attribute on the :class:`views.View`. """ - unknown_form_fields = False + allow_unknown_form_fields = False """ Flag to check for unknown fields when validating a form. If set to false and we receive request data that is not expected by the form it raises an @@ -93,7 +93,7 @@ class FormResource(Resource): Raises a :exc:`response.ErrorResponse` with status code 400 (Bad Request) on failure. Validation is standard form validation, with an additional constraint that *no extra unknown fields* may be supplied - if :attr:`self.unknown_form_fields` is ``False``. + if :attr:`self.allow_unknown_form_fields` is ``False``. On failure the :exc:`response.ErrorResponse` content is a dict which may contain :obj:`'errors'` and :obj:`'field-errors'` keys. If the :obj:`'errors'` key exists it is a list of strings of non-field errors. @@ -141,7 +141,7 @@ class FormResource(Resource): unknown_fields = unknown_fields - set(('csrfmiddlewaretoken', '_accept', '_method')) # TODO: Ugh. # Check using both regular validation, and our stricter no additional fields rule - if bound_form.is_valid() and (self.unknown_form_fields or not unknown_fields): + if bound_form.is_valid() and (self.allow_unknown_form_fields or not unknown_fields): # Validation succeeded... cleaned_data = bound_form.cleaned_data @@ -398,7 +398,7 @@ class ModelResource(FormResource): """ model_fields = set(field.name for field in self.model._meta.fields) - if fields: + if model_fields: return model_fields & set(as_tuple(self.fields)) return model_fields - set(as_tuple(self.exclude)) diff --git a/djangorestframework/tests/validators.py b/djangorestframework/tests/validators.py index c316f209a..7b6432726 100644 --- a/djangorestframework/tests/validators.py +++ b/djangorestframework/tests/validators.py @@ -142,9 +142,9 @@ class TestFormValidation(TestCase): """If we set ``unknown_form_fields`` on the form resource, then don't raise errors on unexpected request data""" content = {'qwerty': 'uiop', 'extra': 'extra'} - validator.unknown_form_fields = True + validator.allow_unknown_form_fields = True self.assertDictEqual({'qwerty': u'uiop'}, validator.validate_request(content, None), "Resource didn't accept unknown fields.") - validator.unknown_form_fields = False + validator.allow_unknown_form_fields = False def validation_does_not_require_extra_fields_if_explicitly_set(self, validator): """If we include an allowed_extra_fields paramater on _validate, then do not fail if we do not have fields with those names.""" From 4e52ce4d333ec090304000bce0e71c78cacd73b5 Mon Sep 17 00:00:00 2001 From: Alen Mujezinovic Date: Thu, 19 Jan 2012 18:45:19 +0000 Subject: [PATCH 4/4] Turns out it was `self.fields` --- djangorestframework/resources.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index 2e855021d..9a43c8af6 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -398,7 +398,7 @@ class ModelResource(FormResource): """ model_fields = set(field.name for field in self.model._meta.fields) - if model_fields: + if self.fields: return model_fields & set(as_tuple(self.fields)) return model_fields - set(as_tuple(self.exclude))