From 44c8b89c6051483677e72a6fc657b1e0457182d1 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Thu, 12 May 2011 16:03:14 +0100 Subject: [PATCH] _perform_form_overloading becomes transparent --- djangorestframework/mixins.py | 113 +++++++++------------------ djangorestframework/tests/content.py | 7 +- djangorestframework/tests/files.py | 4 +- djangorestframework/tests/methods.py | 1 - djangorestframework/views.py | 6 +- 5 files changed, 44 insertions(+), 87 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index caaff7e0f..524e92682 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -54,7 +54,7 @@ class RequestMixin(object): Returns the HTTP method. """ if not hasattr(self, '_method'): - self._load_metadata() + self._load_method_and_content_type() return self._method @@ -64,7 +64,7 @@ class RequestMixin(object): Returns the content type header. """ if not hasattr(self, '_content_type'): - self._load_metadata() + self._load_method_and_content_type() return self._content_type @@ -92,12 +92,16 @@ class RequestMixin(object): """ Parse the request content into self.DATA and self.FILES. """ - stream = self._get_stream() - (self._data, self._files) = self._parse(stream, self.content_type) + if not hasattr(self, '_content_type'): + self._load_method_and_content_type() - def _load_metadata(self): + if not hasattr(self, '_data'): + (self._data, self._files) = self._parse(self._get_stream(), self._content_type) + + + def _load_method_and_content_type(self): """ - Set the method and content_type and then check if they've been overridden. + Set the method and content_type, and then check if they've been overridden. """ self._method = self.request.method self._content_type = self.request.META.get('HTTP_CONTENT_TYPE', self.request.META.get('CONTENT_TYPE', '')) @@ -108,86 +112,45 @@ class RequestMixin(object): """ Returns an object that may be used to stream the request content. """ - if not hasattr(self, '_stream'): - request = self.request + request = self.request - try: - content_length = int(request.META.get('CONTENT_LENGTH', request.META.get('HTTP_CONTENT_LENGTH'))) - except (ValueError, TypeError): - content_length = 0 + try: + content_length = int(request.META.get('CONTENT_LENGTH', request.META.get('HTTP_CONTENT_LENGTH'))) + except (ValueError, TypeError): + content_length = 0 - # TODO: Add 1.3's LimitedStream to compat and use that. - # Currently only supports parsing request body as a stream with 1.3 - if content_length == 0: - return None - elif hasattr(request, 'read'): - # UPDATE BASED ON COMMENT BELOW: - # - # Yup, this was a bug in Django - fixed and waiting check in - see ticket 15785. - # http://code.djangoproject.com/ticket/15785 - # - # COMMENT: - # - # It's not at all clear if this needs to be byte limited or not. - # Maybe I'm just being dumb but it looks to me like there's some issues - # with that in Django. - # - # Either: - # 1. It *can't* be treated as a limited byte stream, and you _do_ need to - # respect CONTENT_LENGTH, in which case that ought to be documented, - # and there probably ought to be a feature request for it to be - # treated as a limited byte stream. - # 2. It *can* be treated as a limited byte stream, in which case there's a - # minor bug in the test client, and potentially some redundant - # code in MultiPartParser. - # - # It's an issue because it affects if you can pass a request off to code that - # does something like: - # - # while stream.read(BUFFER_SIZE): - # [do stuff] - # - #try: - # content_length = int(request.META.get('CONTENT_LENGTH',0)) - #except (ValueError, TypeError): - # content_length = 0 - # self._stream = LimitedStream(request, content_length) - self._stream = request - else: - self._stream = StringIO(request.raw_post_data) - return self._stream + # TODO: Add 1.3's LimitedStream to compat and use that. + # Currently only supports parsing request body as a stream with 1.3 + if content_length == 0: + return None + elif hasattr(request, 'read'): + return request + return StringIO(request.raw_post_data) - # TODO: Modify this so that it happens implictly, rather than being called explicitly - # ie accessing any of .DATA, .FILES, .content_type, .method will force - # form overloading. def _perform_form_overloading(self): """ - Check the request to see if it is using form POST '_method'/'_content'/'_content_type' overrides. - If it is then alter self.method, self.content_type, self.CONTENT to reflect that rather than simply - delegating them to the original request. + If this is a form POST request, then we need to check if the method and content/content_type have been + overridden by setting them in hidden form fields or not. """ - # We only need to use form overloading on form POST requests - if not self._USE_FORM_OVERLOADING or self._method != 'POST' or not not is_form_media_type(self._content_type): + # We only need to use form overloading on form POST requests. + if not self._USE_FORM_OVERLOADING or self._method != 'POST' or not is_form_media_type(self._content_type): return + + # At this point we're committed to parsing the request as form data. + self._data = data = self.request.POST + self._files = self.request.FILES - # Temporarily switch to using the form parsers, then parse the content - parsers = self.parsers - self.parsers = (FormParser, MultiPartParser) - content = self.DATA - self.parsers = parsers + # Method overloading - change the method and remove the param from the content. + if self._METHOD_PARAM in data: + self._method = data[self._METHOD_PARAM].upper() - # Method overloading - change the method and remove the param from the content - if self._METHOD_PARAM in content: - self._method = content[self._METHOD_PARAM].upper() - del self._data[self._METHOD_PARAM] - - # Content overloading - rewind the stream and modify the content type - if self._CONTENT_PARAM in content and self._CONTENTTYPE_PARAM in content: - self._content_type = content[self._CONTENTTYPE_PARAM] - self._stream = StringIO(content[self._CONTENT_PARAM]) - del(self._data) + # Content overloading - modify the content type, and re-parse. + if self._CONTENT_PARAM in data and self._CONTENTTYPE_PARAM in data: + self._content_type = data[self._CONTENTTYPE_PARAM] + stream = StringIO(data[self._CONTENT_PARAM]) + (self._data, self._files) = self._parse(stream, self._content_type) def _parse(self, stream, content_type): diff --git a/djangorestframework/tests/content.py b/djangorestframework/tests/content.py index a99981fd1..fb7a2b14e 100644 --- a/djangorestframework/tests/content.py +++ b/djangorestframework/tests/content.py @@ -12,16 +12,16 @@ class TestContentParsing(TestCase): self.req = RequestFactory() def ensure_determines_no_content_GET(self, view): - """Ensure view.RAW_CONTENT returns None for GET request with no content.""" + """Ensure view.DATA returns None for GET request with no content.""" view.request = self.req.get('/') self.assertEqual(view.DATA, None) def ensure_determines_form_content_POST(self, view): - """Ensure view.RAW_CONTENT returns content for POST request with form content.""" + """Ensure view.DATA returns content for POST request with form content.""" form_data = {'qwerty': 'uiop'} view.parsers = (FormParser, MultiPartParser) view.request = self.req.post('/', data=form_data) - self.assertEqual(view.DATA, form_data) + self.assertEqual(view.DATA.items(), form_data.items()) def ensure_determines_non_form_content_POST(self, view): """Ensure view.RAW_CONTENT returns content for POST request with non-form content.""" @@ -75,5 +75,4 @@ class TestContentParsing(TestCase): view._CONTENTTYPE_PARAM: content_type} view.request = self.req.post('/', form_data) view.parsers = (PlainTextParser,) - view._perform_form_overloading() self.assertEqual(view.DATA, content) diff --git a/djangorestframework/tests/files.py b/djangorestframework/tests/files.py index afa59b4e8..3892422c1 100644 --- a/djangorestframework/tests/files.py +++ b/djangorestframework/tests/files.py @@ -24,8 +24,8 @@ class UploadFilesTests(TestCase): resource = MockResource def post(self, request, *args, **kwargs): - return {'FILE_NAME': self.CONTENT['file'].name, - 'FILE_CONTENT': self.CONTENT['file'].read()} + return {'FILE_NAME': self.CONTENT['file'][0].name, + 'FILE_CONTENT': self.CONTENT['file'][0].read()} file = StringIO.StringIO('stuff') file.name = 'stuff.txt' diff --git a/djangorestframework/tests/methods.py b/djangorestframework/tests/methods.py index 961d518b6..d8f0d9199 100644 --- a/djangorestframework/tests/methods.py +++ b/djangorestframework/tests/methods.py @@ -23,5 +23,4 @@ class TestMethodOverloading(TestCase): """POST requests can be overloaded to another method by setting a reserved form field""" view = RequestMixin() view.request = self.req.post('/', {view._METHOD_PARAM: 'DELETE'}) - view._perform_form_overloading() self.assertEqual(view.method, 'DELETE') diff --git a/djangorestframework/views.py b/djangorestframework/views.py index 2a23c49a5..211dafca4 100644 --- a/djangorestframework/views.py +++ b/djangorestframework/views.py @@ -77,11 +77,7 @@ class BaseView(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, View): prefix = '%s://%s' % (request.is_secure() and 'https' or 'http', request.get_host()) set_script_prefix(prefix) - try: - # If using a form POST with '_method'/'_content'/'_content_type' overrides, then alter - # self.method, self.content_type, self.RAW_CONTENT & self.CONTENT appropriately. - self._perform_form_overloading() - + try: # Authenticate and check request is has the relevant permissions self._check_permissions()