From e73838b6ddcc7b37c03f9eee04fa6e6a283fedb3 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Fri, 25 May 2012 19:03:15 +0200 Subject: [PATCH] Fixed #17371 -- Made the test client more flexible The OPTIONS, PUT and DELETE methods no longer apply arbitrary data encoding (in the query string or in the request body). --- django/test/client.py | 70 +++++++++---------- docs/releases/1.5.txt | 22 +++++- docs/topics/testing.txt | 57 +++++++++------ .../conditional_processing/models.py | 4 +- .../test_client_regress/models.py | 21 ++---- 5 files changed, 93 insertions(+), 81 deletions(-) diff --git a/django/test/client.py b/django/test/client.py index c1764e2394..3cc6a6e537 100644 --- a/django/test/client.py +++ b/django/test/client.py @@ -155,7 +155,6 @@ def encode_file(boundary, key, file): ] - class RequestFactory(object): """ Class that lets you create mock Request objects for use in testing. @@ -227,7 +226,7 @@ class RequestFactory(object): return urllib.unquote(parsed[2]) def get(self, path, data={}, **extra): - "Construct a GET request" + "Construct a GET request." parsed = urlparse(path) r = { @@ -270,49 +269,39 @@ class RequestFactory(object): r.update(extra) return self.request(**r) - def options(self, path, data={}, **extra): - "Constrict an OPTIONS request" + def options(self, path, data='', content_type='application/octet-stream', + **extra): + "Construct an OPTIONS request." + return self.generic('OPTIONS', path, data, content_type, **extra) - parsed = urlparse(path) - r = { - 'PATH_INFO': self._get_path(parsed), - 'QUERY_STRING': urlencode(data, doseq=True) or parsed[4], - 'REQUEST_METHOD': 'OPTIONS', - } - r.update(extra) - return self.request(**r) - - def put(self, path, data={}, content_type=MULTIPART_CONTENT, + def put(self, path, data='', content_type='application/octet-stream', **extra): "Construct a PUT request." + return self.generic('PUT', path, data, content_type, **extra) - put_data = self._encode_data(data, content_type) + def delete(self, path, data='', content_type='application/octet-stream', + **extra): + "Construct a DELETE request." + return self.generic('DELETE', path, data, content_type, **extra) + def generic(self, method, path, + data='', content_type='application/octet-stream', **extra): parsed = urlparse(path) + data = smart_str(data, settings.DEFAULT_CHARSET) r = { - 'CONTENT_LENGTH': len(put_data), - 'CONTENT_TYPE': content_type, 'PATH_INFO': self._get_path(parsed), 'QUERY_STRING': parsed[4], - 'REQUEST_METHOD': 'PUT', - 'wsgi.input': FakePayload(put_data), + 'REQUEST_METHOD': method, } + if data: + r.update({ + 'CONTENT_LENGTH': len(data), + 'CONTENT_TYPE': content_type, + 'wsgi.input': FakePayload(data), + }) r.update(extra) return self.request(**r) - def delete(self, path, data={}, **extra): - "Construct a DELETE request." - - parsed = urlparse(path) - r = { - 'PATH_INFO': self._get_path(parsed), - 'QUERY_STRING': urlencode(data, doseq=True) or parsed[4], - 'REQUEST_METHOD': 'DELETE', - } - r.update(extra) - return self.request(**r) - - class Client(RequestFactory): """ A class that can act as a client for testing purposes. @@ -445,30 +434,35 @@ class Client(RequestFactory): response = self._handle_redirects(response, **extra) return response - def options(self, path, data={}, follow=False, **extra): + def options(self, path, data='', content_type='application/octet-stream', + follow=False, **extra): """ Request a response from the server using OPTIONS. """ - response = super(Client, self).options(path, data=data, **extra) + response = super(Client, self).options(path, + data=data, content_type=content_type, **extra) if follow: response = self._handle_redirects(response, **extra) return response - def put(self, path, data={}, content_type=MULTIPART_CONTENT, + def put(self, path, data='', content_type='application/octet-stream', follow=False, **extra): """ Send a resource to the server using PUT. """ - response = super(Client, self).put(path, data=data, content_type=content_type, **extra) + response = super(Client, self).put(path, + data=data, content_type=content_type, **extra) if follow: response = self._handle_redirects(response, **extra) return response - def delete(self, path, data={}, follow=False, **extra): + def delete(self, path, data='', content_type='application/octet-stream', + follow=False, **extra): """ Send a DELETE request to the server. """ - response = super(Client, self).delete(path, data=data, **extra) + response = super(Client, self).delete(path, + data=data, content_type=content_type, **extra) if follow: response = self._handle_redirects(response, **extra) return response diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index 46b599a622..851fed69f7 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -56,7 +56,7 @@ previously loaded. For example, with the tutorial's models:: True In Django 1.5, the third line no longer triggers a new SQL query to fetch -``first_choice.poll``; it was set when by the second line. +``first_choice.poll``; it was set by the second line. For one-to-one relationships, both sides can be cached. For many-to-one relationships, only the single side of the relationship can be cached. This @@ -101,6 +101,26 @@ year|date:"Y" }}``. ``next_year`` and ``previous_year`` were also added in the context. They are calculated according to ``allow_empty`` and ``allow_future``. +OPTIONS, PUT and DELETE requests in the test client +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Unlike GET and POST, these HTTP methods aren't implemented by web browsers. +Rather, they're used in APIs, which transfer data in various formats such as +JSON or XML. Since such requests may contain arbitrary data, Django doesn't +attempt to decode their body. + +However, the test client used to build a query string for OPTIONS and DELETE +requests like for GET, and a request body for PUT requests like for POST. This +encoding was arbitrary and inconsistent with Django's behavior when it +receives the requests, so it was removed in Django 1.5. + +If you were using the ``data`` parameter in an OPTIONS or a DELETE request, +you must convert it to a query string and append it to the ``path`` parameter. + +If you were using the ``data`` parameter in a PUT request without a +``content_type``, you must encode your data before passing it to the test +client and set the ``content_type`` argument. + Features deprecated in 1.5 ========================== diff --git a/docs/topics/testing.txt b/docs/topics/testing.txt index d5ccc2d8fc..f35c545c30 100644 --- a/docs/topics/testing.txt +++ b/docs/topics/testing.txt @@ -805,45 +805,56 @@ arguments at time of construction: .. method:: Client.head(path, data={}, follow=False, **extra) - Makes a HEAD request on the provided ``path`` and returns a ``Response`` - object. Useful for testing RESTful interfaces. Acts just like - :meth:`Client.get` except it does not return a message body. + Makes a HEAD request on the provided ``path`` and returns a + ``Response`` object. This method works just like :meth:`Client.get`, + including the ``follow`` and ``extra`` arguments, except it does not + return a message body. - If you set ``follow`` to ``True`` the client will follow any redirects - and a ``redirect_chain`` attribute will be set in the response object - containing tuples of the intermediate urls and status codes. - - .. method:: Client.options(path, data={}, follow=False, **extra) + .. method:: Client.options(path, data='', content_type='application/octet-stream', follow=False, **extra) Makes an OPTIONS request on the provided ``path`` and returns a ``Response`` object. Useful for testing RESTful interfaces. - If you set ``follow`` to ``True`` the client will follow any redirects - and a ``redirect_chain`` attribute will be set in the response object - containing tuples of the intermediate urls and status codes. + When ``data`` is provided, it is used as the request body, and + a ``Content-Type`` header is set to ``content_type``. - The ``extra`` argument acts the same as for :meth:`Client.get`. + .. versionchanged:: 1.5 + :meth:`Client.options` used to process ``data`` like + :meth:`Client.get`. - .. method:: Client.put(path, data={}, content_type=MULTIPART_CONTENT, follow=False, **extra) + The ``follow`` and ``extra`` arguments act the same as for + :meth:`Client.get`. + + .. method:: Client.put(path, data='', content_type='application/octet-stream', follow=False, **extra) Makes a PUT request on the provided ``path`` and returns a - ``Response`` object. Useful for testing RESTful interfaces. Acts just - like :meth:`Client.post` except with the PUT request method. + ``Response`` object. Useful for testing RESTful interfaces. - If you set ``follow`` to ``True`` the client will follow any redirects - and a ``redirect_chain`` attribute will be set in the response object - containing tuples of the intermediate urls and status codes. + When ``data`` is provided, it is used as the request body, and + a ``Content-Type`` header is set to ``content_type``. - .. method:: Client.delete(path, follow=False, **extra) + .. versionchanged:: 1.5 + :meth:`Client.put` used to process ``data`` like + :meth:`Client.post`. + + The ``follow`` and ``extra`` arguments act the same as for + :meth:`Client.get`. + + .. method:: Client.delete(path, data='', content_type='application/octet-stream', follow=False, **extra) Makes an DELETE request on the provided ``path`` and returns a ``Response`` object. Useful for testing RESTful interfaces. - If you set ``follow`` to ``True`` the client will follow any redirects - and a ``redirect_chain`` attribute will be set in the response object - containing tuples of the intermediate urls and status codes. + When ``data`` is provided, it is used as the request body, and + a ``Content-Type`` header is set to ``content_type``. + + .. versionchanged:: 1.5 + :meth:`Client.delete` used to process ``data`` like + :meth:`Client.get`. + + The ``follow`` and ``extra`` arguments act the same as for + :meth:`Client.get`. - The ``extra`` argument acts the same as for :meth:`Client.get`. .. method:: Client.login(**credentials) diff --git a/tests/regressiontests/conditional_processing/models.py b/tests/regressiontests/conditional_processing/models.py index f7f48bc9c8..97aeff5eaa 100644 --- a/tests/regressiontests/conditional_processing/models.py +++ b/tests/regressiontests/conditional_processing/models.py @@ -63,10 +63,10 @@ class ConditionalGet(TestCase): def testIfMatch(self): self.client.defaults['HTTP_IF_MATCH'] = '"%s"' % ETAG - response = self.client.put('/condition/etag/', {'data': ''}) + response = self.client.put('/condition/etag/') self.assertEqual(response.status_code, 200) self.client.defaults['HTTP_IF_MATCH'] = '"%s"' % EXPIRED_ETAG - response = self.client.put('/condition/etag/', {'data': ''}) + response = self.client.put('/condition/etag/') self.assertEqual(response.status_code, 412) def testBothHeaders(self): diff --git a/tests/regressiontests/test_client_regress/models.py b/tests/regressiontests/test_client_regress/models.py index 3fb8f894fd..ca55bd156b 100644 --- a/tests/regressiontests/test_client_regress/models.py +++ b/tests/regressiontests/test_client_regress/models.py @@ -347,7 +347,7 @@ class AssertRedirectsTests(TestCase): def test_redirect_chain_options(self): "A redirect chain will be followed from an initial OPTIONS request" response = self.client.options('/test_client_regress/redirects/', - {'nothing': 'to_send'}, follow=True) + follow=True) self.assertRedirects(response, '/test_client_regress/no_template_view/', 301, 200) self.assertEqual(len(response.redirect_chain), 3) @@ -355,7 +355,7 @@ class AssertRedirectsTests(TestCase): def test_redirect_chain_put(self): "A redirect chain will be followed from an initial PUT request" response = self.client.put('/test_client_regress/redirects/', - {'nothing': 'to_send'}, follow=True) + follow=True) self.assertRedirects(response, '/test_client_regress/no_template_view/', 301, 200) self.assertEqual(len(response.redirect_chain), 3) @@ -363,7 +363,7 @@ class AssertRedirectsTests(TestCase): def test_redirect_chain_delete(self): "A redirect chain will be followed from an initial DELETE request" response = self.client.delete('/test_client_regress/redirects/', - {'nothing': 'to_send'}, follow=True) + follow=True) self.assertRedirects(response, '/test_client_regress/no_template_view/', 301, 200) self.assertEqual(len(response.redirect_chain), 3) @@ -809,8 +809,7 @@ class RequestMethodStringDataTests(TestCase): class QueryStringTests(TestCase): def test_get_like_requests(self): # See: https://code.djangoproject.com/ticket/10571. - # Removed 'put' and 'delete' here as they are 'GET-like requests' - for method_name in ('get','head','options'): + for method_name in ('get', 'head'): # A GET-like request can pass a query string as data method = getattr(self.client, method_name) response = method("/test_client_regress/request_data/", data={'foo':'whiz'}) @@ -867,9 +866,6 @@ class UnicodePayloadTests(TestCase): response = self.client.post("/test_client_regress/parse_unicode_json/", json, content_type="application/json") self.assertEqual(response.content, json) - response = self.client.put("/test_client_regress/parse_unicode_json/", json, - content_type="application/json") - self.assertEqual(response.content, json) def test_unicode_payload_utf8(self): "A non-ASCII unicode data encoded as UTF-8 can be POSTed" @@ -878,9 +874,6 @@ class UnicodePayloadTests(TestCase): response = self.client.post("/test_client_regress/parse_unicode_json/", json, content_type="application/json; charset=utf-8") self.assertEqual(response.content, json.encode('utf-8')) - response = self.client.put("/test_client_regress/parse_unicode_json/", json, - content_type="application/json; charset=utf-8") - self.assertEqual(response.content, json.encode('utf-8')) def test_unicode_payload_utf16(self): "A non-ASCII unicode data encoded as UTF-16 can be POSTed" @@ -889,9 +882,6 @@ class UnicodePayloadTests(TestCase): response = self.client.post("/test_client_regress/parse_unicode_json/", json, content_type="application/json; charset=utf-16") self.assertEqual(response.content, json.encode('utf-16')) - response = self.client.put("/test_client_regress/parse_unicode_json/", json, - content_type="application/json; charset=utf-16") - self.assertEqual(response.content, json.encode('utf-16')) def test_unicode_payload_non_utf(self): "A non-ASCII unicode data as a non-UTF based encoding can be POSTed" @@ -900,9 +890,6 @@ class UnicodePayloadTests(TestCase): response = self.client.post("/test_client_regress/parse_unicode_json/", json, content_type="application/json; charset=koi8-r") self.assertEqual(response.content, json.encode('koi8-r')) - response = self.client.put("/test_client_regress/parse_unicode_json/", json, - content_type="application/json; charset=koi8-r") - self.assertEqual(response.content, json.encode('koi8-r')) class DummyFile(object): def __init__(self, filename):