From beed8402ca2b44681f939238c8ea02deeb8a06a2 Mon Sep 17 00:00:00 2001 From: R David Murray Date: Sun, 22 Mar 2015 15:18:23 -0400 Subject: [PATCH] #23539: Set Content-Length to 0 for PUT, POST, and PATCH if body is None. Some http servers will reject PUT, POST, and PATCH requests if they do not have a Content-Length header. Patch by James Rutherford, with additional cleaning up of the 'request' documentation by me. --- Doc/library/http.client.rst | 34 ++++++++++++------- Lib/http/client.py | 37 +++++++++++++------- Lib/test/test_httplib.py | 67 +++++++++++++++++++++++++++++-------- Misc/ACKS | 1 + Misc/NEWS | 4 +++ 5 files changed, 104 insertions(+), 39 deletions(-) diff --git a/Doc/library/http.client.rst b/Doc/library/http.client.rst index b6e78b520eb..fc451034743 100644 --- a/Doc/library/http.client.rst +++ b/Doc/library/http.client.rst @@ -413,23 +413,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. + method *method* and the selector *url*. - 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. + 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 3f9e67bba48..b27aa5d0cf1 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -246,6 +246,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 @@ -1126,19 +1130,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) @@ -1154,8 +1165,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 e4911d94732..df9a9e3cbda 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 @@ -125,21 +126,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') + # 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) + ) - # 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') + # 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 a23b470d2aa..cdee1bbcf8f 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1179,6 +1179,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 46c3c0a66ad..fce79569c48 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -18,6 +18,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.