Support PATCH with body in curl_httpclient.

This previously required `allow_nonstandard_methods`.

Refine the body sanity checks and tests (followup to #1531).

diff --git a/tornado/curl_httpclient.py b/tornado/curl_httpclient.py
index baa616ba0646083069d5d07877f706d0c0a543dc..22f25023224a7659bf4f3158d8f62062118df7eb 100644 (file)
--- a/tornado/curl_httpclient.py
+++ b/tornado/curl_httpclient.py
@@ -387,24 +387,28 @@ class CurlAsyncHTTPClient(AsyncHTTPClient):
         else:
             raise KeyError('unknown method ' + request.method)
 
-        # Handle curl's cryptic options for every individual HTTP method
-        if request.method == "GET":
-            # Even with `allow_nonstandard_methods` we disallow GET with a
-            # body. While the spec doesn't forbid clients from sending a body,
-            # it arguably disallows the server from doing anything with them.
-            if request.body is not None:
-                raise ValueError('Body must be None for GET request')
-        if request.method in ("POST", "PUT") or request.body:
-            # Fail in case POST or PUT method has no body, unless the user has
+        body_expected = request.method in ("POST", "PATCH", "PUT")
+        body_present = request.body is not None
+        if not request.allow_nonstandard_methods:
+            # Some HTTP methods nearly always have bodies while others
+            # almost never do. Fail in this case unless the user has
             # opted out of sanity checks with allow_nonstandard_methods.
-            if request.body is not None:
-                request_buffer = BytesIO(utf8(request.body))
-            elif not request.allow_nonstandard_methods:
+            if ((body_expected and not body_present) or
+                    (body_present and not body_expected)):
                 raise ValueError(
-                    'Body must not be None for method %s (unless '
-                    'allow_nonstandard_methods is true)' % request.method)
-            else:
-                request_buffer = BytesIO()
+                    'Body must %sbe None for method %s (unless '
+                    'allow_nonstandard_methods is true)' %
+                    ('not ' if body_expected else '', request.method))
+
+        if body_expected or body_present:
+            if request.method == "GET":
+                # Even with `allow_nonstandard_methods` we disallow
+                # GET with a body (because libcurl doesn't allow it
+                # unless we use CUSTOMREQUEST). While the spec doesn't
+                # forbid clients from sending a body, it arguably
+                # disallows the server from doing anything with them.
+                raise ValueError('Body must be None for GET request')
+            request_buffer = BytesIO(utf8(request.body or ''))
 
             def ioctl(cmd):
                 if cmd == curl.IOCMD_RESTARTREAD:

diff --git a/tornado/test/httpclient_test.py b/tornado/test/httpclient_test.py
index dcf6decb10c4ba481b5fb25a9c66e5405186a983..79de77bb1a1c9a26b078a1e1087fdaff461cb542 100644 (file)
--- a/tornado/test/httpclient_test.py
+++ b/tornado/test/httpclient_test.py
@@ -470,36 +470,32 @@ X-XSS-Protection: 1;
                               allow_nonstandard_methods=True)
         self.assertEqual(response.body, b'OTHER')
 
-    @gen_test
     def test_body_sanity_checks(self):
-        hello_url = self.get_url('/hello')
-        with self.assertRaises(ValueError) as context:
-            yield self.http_client.fetch(hello_url, body='data')
-
-        self.assertTrue('must be None' in str(context.exception))
-
-        with self.assertRaises(ValueError) as context:
-            yield self.http_client.fetch(hello_url, method='POST')
-
-        self.assertTrue('must not be None' in str(context.exception))
-
-    @gen_test
-    def test_ignore_body_sanity_checks_when_allow_nonstandard_methods(self):
-        all_methods_url = self.get_url('/all_methods')
-        for method in ('POST', 'PUT'):
-            response = yield self.http_client.fetch(
-                all_methods_url, method=method, body=None,
-                allow_nonstandard_methods=True)
-            self.assertEqual(response.code, 200)
-            self.assertIsNone(response.request.body)
-
-        # Don't test for GET with a body. Curl client does not allow it.
-        for method in ('PATCH', 'DELETE', 'OPTIONS'):
-            response = yield self.http_client.fetch(
-                all_methods_url, method=method, body=utf8(method),
-                allow_nonstandard_methods=True)
-            self.assertEqual(response.code, 200)
-            self.assertEqual(response.body, utf8(method))
+        # These methods require a body.
+        for method in ('POST', 'PUT', 'PATCH'):
+            with self.assertRaises(ValueError) as context:
+                resp = self.fetch('/all_methods', method=method)
+                resp.rethrow()
+            self.assertIn('must not be None', str(context.exception))
+
+            resp = self.fetch('/all_methods', method=method,
+                              allow_nonstandard_methods=True)
+            self.assertEqual(resp.code, 200)
+
+        # These methods don't allow a body.
+        for method in ('GET', 'DELETE', 'OPTIONS'):
+            with self.assertRaises(ValueError) as context:
+                resp = self.fetch('/all_methods', method=method, body=b'asdf')
+                resp.rethrow()
+            self.assertIn('must be None', str(context.exception))
+
+            # In most cases this can be overridden, but curl_httpclient
+            # does not allow body with a GET at all.
+            if method != 'GET':
+                resp = self.fetch('/all_methods', method=method, body=b'asdf',
+                                  allow_nonstandard_methods=True)
+                resp.rethrow()
+                self.assertEqual(resp.code, 200)
 
     # This test causes odd failures with the combination of
     # curl_httpclient (at least with the version of libcurl available