Merge: #23539: Set Content-Length to 0 for PUT, POST, and PATCH if body is None.

5 files changed, 106 insertions, 41 deletions

diff --git a/Doc/library/http.client.rst b/Doc/library/http.client.rst
index 5e27a36..a7af016 100644
--- a/Doc/library/http.client.rst
+++ b/Doc/library/http.client.rst
@@ -207,23 +207,33 @@ HTTPConnection Objects
 .. method:: HTTPConnection.request(method, url, body=None, headers={})
 
    This will send a request to the server using the HTTP request
-   method *method* and the selector *url*.  If the *body* argument is
-   present, it should be string or bytes object of data to send after
-   the headers are finished.  Strings are encoded as ISO-8859-1, the
-   default charset for HTTP.  To use other encodings, pass a bytes
-   object.  The Content-Length header is set to the length of the
-   string.
-
-   The *body* may also be an open :term:`file object`, in which case the
-   contents of the file is sent; this file object should support ``fileno()``
-   and ``read()`` methods. The header Content-Length is automatically set to
-   the length of the file as reported by stat. The *body* argument may also be
-   an iterable and Content-Length header should be explicitly provided when the
-   body is an iterable.
+   method *method* and the selector *url*.
+
+   If *body* is specified, the specified data is sent after the headers are
+   finished.  It may be a string, a :term:`bytes-like object`, an open
+   :term:`file object`, or an iterable of :term:`bytes-like object`\s.  If
+   *body* is a string, it is encoded as ISO-8851-1, the default for HTTP.  If
+   it is a bytes-like object the bytes are sent as is.  If it is a :term:`file
+   object`, the contents of the file is sent; this file object should support
+   at least the ``read()`` method.  If the file object has a ``mode``
+   attribute, the data returned by the ``read()`` method will be encoded as
+   ISO-8851-1 unless the ``mode`` attribute contains the substring ``b``,
+   otherwise the data returned by ``read()`` is sent as is.  If *body* is an
+   iterable, the elements of the iterable are sent as is until the iterable is
+   exhausted.
 
    The *headers* argument should be a mapping of extra HTTP
    headers to send with the request.
 
+   If *headers* does not contain a Content-Length item, one is added
+   automatically if possible.  If *body* is ``None``, the Content-Length header
+   is set to ``0`` for methods that expect a body (``PUT``, ``POST``, and
+   ``PATCH``).  If *body* is a string or bytes object, the Content-Length
+   header is set to its length.  If *body* is a :term:`file object` and it
+   works to call :func:`~os.fstat` on the result of its ``fileno()`` method,
+   then the Content-Length header is set to the ``st_size`` reported by the
+   ``fstat`` call.  Otherwise no Content-Length header is added.
+
    .. versionadded:: 3.2
       *body* can now be an iterable.
 
diff --git a/Lib/http/client.py b/Lib/http/client.py
index d2709ae..f5889ed 100644
--- a/Lib/http/client.py
+++ b/Lib/http/client.py
@@ -138,6 +138,10 @@ _MAXHEADERS = 100
 _is_legal_header_name = re.compile(rb'[^:\s][^:\r\n]*').fullmatch
 _is_illegal_header_value = re.compile(rb'\n(?![ \t])|\r(?![ \t\n])').search
 
+# We always set the Content-Length header for these methods because some
+# servers will otherwise respond with a 411
+_METHODS_EXPECTING_BODY = {'PATCH', 'POST', 'PUT'}
+
 
 class HTTPMessage(email.message.Message):
     # XXX The only usage of this method is in
@@ -1068,19 +1072,26 @@ class HTTPConnection:
         """Send a complete request to the server."""
         self._send_request(method, url, body, headers)
 
-    def _set_content_length(self, body):
-        # Set the content-length based on the body.
+    def _set_content_length(self, body, method):
+        # Set the content-length based on the body. If the body is "empty", we
+        # set Content-Length: 0 for methods that expect a body (RFC 7230,
+        # Section 3.3.2). If the body is set for other methods, we set the
+        # header provided we can figure out what the length is.
         thelen = None
-        try:
-            thelen = str(len(body))
-        except TypeError as te:
-            # If this is a file-like object, try to
-            # fstat its file descriptor
+        method_expects_body = method.upper() in _METHODS_EXPECTING_BODY
+        if body is None and method_expects_body:
+            thelen = '0'
+        elif body is not None:
             try:
-                thelen = str(os.fstat(body.fileno()).st_size)
-            except (AttributeError, OSError):
-                # Don't send a length if this failed
-                if self.debuglevel > 0: print("Cannot stat!!")
+                thelen = str(len(body))
+            except TypeError:
+                # If this is a file-like object, try to
+                # fstat its file descriptor
+                try:
+                    thelen = str(os.fstat(body.fileno()).st_size)
+                except (AttributeError, OSError):
+                    # Don't send a length if this failed
+                    if self.debuglevel > 0: print("Cannot stat!!")
 
         if thelen is not None:
             self.putheader('Content-Length', thelen)
@@ -1096,8 +1107,8 @@ class HTTPConnection:
 
         self.putrequest(method, url, **skips)
 
-        if body is not None and ('content-length' not in header_names):
-            self._set_content_length(body)
+        if 'content-length' not in header_names:
+            self._set_content_length(body, method)
         for hdr, value in headers.items():
             self.putheader(hdr, value)
         if isinstance(body, str):
diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
index ca6bda5..b4ec63c 100644
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -1,6 +1,7 @@
 import errno
 from http import client
 import io
+import itertools
 import os
 import array
 import socket
@@ -148,21 +149,59 @@ class HeaderTests(TestCase):
                     self.content_length = kv[1].strip()
                 list.append(self, item)
 
-        # POST with empty body
-        conn = client.HTTPConnection('example.com')
-        conn.sock = FakeSocket(None)
-        conn._buffer = ContentLengthChecker()
-        conn.request('POST', '/', '')
-        self.assertEqual(conn._buffer.content_length, b'0',
-                        'Header Content-Length not set')
-
-        # PUT request with empty body
-        conn = client.HTTPConnection('example.com')
-        conn.sock = FakeSocket(None)
-        conn._buffer = ContentLengthChecker()
-        conn.request('PUT', '/', '')
-        self.assertEqual(conn._buffer.content_length, b'0',
-                        'Header Content-Length not set')
+        # Here, we're testing that methods expecting a body get a
+        # content-length set to zero if the body is empty (either None or '')
+        bodies = (None, '')
+        methods_with_body = ('PUT', 'POST', 'PATCH')
+        for method, body in itertools.product(methods_with_body, bodies):
+            conn = client.HTTPConnection('example.com')
+            conn.sock = FakeSocket(None)
+            conn._buffer = ContentLengthChecker()
+            conn.request(method, '/', body)
+            self.assertEqual(
+                conn._buffer.content_length, b'0',
+                'Header Content-Length incorrect on {}'.format(method)
+            )
+
+        # For these methods, we make sure that content-length is not set when
+        # the body is None because it might cause unexpected behaviour on the
+        # server.
+        methods_without_body = (
+             'GET', 'CONNECT', 'DELETE', 'HEAD', 'OPTIONS', 'TRACE',
+        )
+        for method in methods_without_body:
+            conn = client.HTTPConnection('example.com')
+            conn.sock = FakeSocket(None)
+            conn._buffer = ContentLengthChecker()
+            conn.request(method, '/', None)
+            self.assertEqual(
+                conn._buffer.content_length, None,
+                'Header Content-Length set for empty body on {}'.format(method)
+            )
+
+        # If the body is set to '', that's considered to be "present but
+        # empty" rather than "missing", so content length would be set, even
+        # for methods that don't expect a body.
+        for method in methods_without_body:
+            conn = client.HTTPConnection('example.com')
+            conn.sock = FakeSocket(None)
+            conn._buffer = ContentLengthChecker()
+            conn.request(method, '/', '')
+            self.assertEqual(
+                conn._buffer.content_length, b'0',
+                'Header Content-Length incorrect on {}'.format(method)
+            )
+
+        # If the body is set, make sure Content-Length is set.
+        for method in itertools.chain(methods_without_body, methods_with_body):
+            conn = client.HTTPConnection('example.com')
+            conn.sock = FakeSocket(None)
+            conn._buffer = ContentLengthChecker()
+            conn.request(method, '/', ' ')
+            self.assertEqual(
+                conn._buffer.content_length, b'1',
+                'Header Content-Length incorrect on {}'.format(method)
+            )
 
     def test_putheader(self):
         conn = client.HTTPConnection('example.com')
diff --git a/Misc/ACKS b/Misc/ACKS
index 0c71aa2..ec48e59 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -1201,6 +1201,7 @@ Sam Rushing
 Mark Russell
 Rusty Russell
 Nick Russo
+James Rutherford
 Chris Ryland
 Constantina S.
 Patrick Sabin
diff --git a/Misc/NEWS b/Misc/NEWS
index 072c874..d405a05 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -23,6 +23,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #23539: If body is None, http.client.HTTPConnection.request now sets
+  Content-Length to 0 for PUT, POST, and PATCH headers to avoid 411 errors from
+  some web servers.
+
 - Issue #22351: The nntplib.NNTP constructor no longer leaves the connection
   and socket open until the garbage collector cleans them up.  Patch by
   Martin Panter.