Create our own logic for chunked content framing
ADD
changelog/2515.feature.rst
+Changed ``HTTPConnection.request()`` to always use lowercase chunk boundaries when sending requests with ``Transfer-Encoding: chunked``.

ADD
ci/0001-Mark-100-Continue-tests-as-failing.patch
+diff --git a/tests/unit/test_awsrequest.py b/tests/unit/test_awsrequest.py
+index 22bd9a7..862a244 100644
+--- a/tests/unit/test_awsrequest.py
++++ b/tests/unit/test_awsrequest.py
+@@ -34,6 +34,7 @@ from botocore.compat import file_type, six
+ from botocore.exceptions import UnseekableStreamError
+ from tests import mock, unittest
+
++import pytest
+
+ class IgnoreCloseBytesIO(io.BytesIO):
+ def close(self):
+@@ -370,6 +371,7 @@ class TestAWSHTTPConnection(unittest.TestCase):
+ conn.response_class.return_value = self.mock_response
+ return conn
+
++ @pytest.mark.xfail(reason="https://github.com/urllib3/urllib3/pull/2565")
+ def test_expect_100_continue_returned(self):
+ with mock.patch('urllib3.util.wait_for_read') as wait_mock:
+ # Shows the server first sending a 100 continue response
+@@ -387,6 +389,7 @@ class TestAWSHTTPConnection(unittest.TestCase):
+ # Now we should verify that our final response is the 200 OK
+ self.assertEqual(response.status, 200)
+
++ @pytest.mark.xfail(reason="https://github.com/urllib3/urllib3/pull/2565")
+ def test_handles_expect_100_with_different_reason_phrase(self):
+ with mock.patch('urllib3.util.wait_for_read') as wait_mock:
+ # Shows the server first sending a 100 continue response
+@@ -412,6 +415,7 @@ class TestAWSHTTPConnection(unittest.TestCase):
+ # continue.
+ self.assertIn(b'body', s.sent_data)
+
++ @pytest.mark.xfail(reason="https://github.com/urllib3/urllib3/pull/2565")
+ def test_expect_100_sends_connection_header(self):
+ # When using squid as an HTTP proxy, it will also send
+ # a Connection: keep-alive header back with the 100 continue
+@@ -439,6 +443,7 @@ class TestAWSHTTPConnection(unittest.TestCase):
+ response = conn.getresponse()
+ self.assertEqual(response.status, 500)
+
++ @pytest.mark.xfail(reason="https://github.com/urllib3/urllib3/pull/2565")
+ def test_expect_100_continue_sends_307(self):
+ # This is the case where we send a 100 continue and the server
+ # immediately sends a 307
+@@ -461,6 +466,7 @@ class TestAWSHTTPConnection(unittest.TestCase):
+ # Now we should verify that our final response is the 307.
+ self.assertEqual(response.status, 307)
+
++ @pytest.mark.xfail(reason="https://github.com/urllib3/urllib3/pull/2565")
+ def test_expect_100_continue_no_response_from_server(self):
+ with mock.patch('urllib3.util.wait_for_read') as wait_mock:
+ # Shows the server first sending a 100 continue response
+@@ -566,6 +572,7 @@ class TestAWSHTTPConnection(unittest.TestCase):
+ response = conn.getresponse()
+ self.assertEqual(response.status, 200)
+
++ @pytest.mark.xfail(reason="https://github.com/urllib3/urllib3/pull/2565")
+ def test_state_reset_on_connection_close(self):
+ # This simulates what urllib3 does with connections
+ # in its connection pool logic.

MODIFY
dummyserver/handlers.py
dummyserver/handlers.py
@@ -279,6 +279,9 @@ class TestingApp(RequestHandler):
def headers(self, request: httputil.HTTPServerRequest) -> Response:
return Response(json.dumps(dict(request.headers)))
+ def multi_headers(self, request: httputil.HTTPServerRequest) -> Response:
+ return Response(json.dumps({"headers": list(request.headers.get_all())}))
+
def successful_retry(self, request: httputil.HTTPServerRequest) -> Response:
"""Handler which will return an error and then success

MODIFY
dummyserver/testcase.py
dummyserver/testcase.py
@@ -301,7 +301,6 @@ class ConnectionMarker:
"""
orig_request = HTTPConnection.request
- orig_request_chunked = HTTPConnection.request_chunked
def call_and_mark(target: Callable[..., None]) -> Callable[..., None]:
def part(self: HTTPConnection, *args: Any, **kwargs: Any) -> None:
@@ -312,9 +311,6 @@ class ConnectionMarker:
with monkeypatch.context() as m:
m.setattr(HTTPConnection, "request", call_and_mark(orig_request))
- m.setattr(
- HTTPConnection, "request_chunked", call_and_mark(orig_request_chunked)
- )
yield
@classmethod

MODIFY
noxfile.py
noxfile.py
@@ -92,6 +92,12 @@ def downstream_botocore(session: nox.Session) -> None:
session.cd(tmp_dir)
git_clone(session, "https://github.com/boto/botocore")
session.chdir("botocore")
+ session.run(
+ "git",
+ "apply",
+ f"{root}/ci/0001-Mark-100-Continue-tests-as-failing.patch",
+ external=True,
+ )
session.run("git", "rev-parse", "HEAD", external=True)
session.run("python", "scripts/ci/install")

MODIFY
src/urllib3/connection.py
src/urllib3/connection.py
@@ -4,7 +4,6 @@ import os
import re
import socket
import warnings
-from copy import copy
from http.client import HTTPConnection as _HTTPConnection
from http.client import HTTPException as HTTPException # noqa: F401
from socket import timeout as SocketTimeout
@@ -29,7 +28,7 @@ if TYPE_CHECKING:
from .util.ssltransport import SSLTransport
from .util.timeout import _DEFAULT_TIMEOUT, _TYPE_TIMEOUT, Timeout
-from .util.util import to_bytes, to_str
+from .util.util import to_str
try: # Compiled with SSL?
import ssl
@@ -51,6 +50,7 @@ from .exceptions import (
SystemTimeWarning,
)
from .util import SKIP_HEADER, SKIPPABLE_HEADERS, connection, ssl_
+from .util.request import body_to_chunks
from .util.ssl_ import assert_fingerprint as _assert_fingerprint
from .util.ssl_ import (
create_urllib3_context,
@@ -126,6 +126,7 @@ class HTTPConnection(_HTTPConnection):
#: certificate. If no HTTPS proxy is being used will be ``None``.
proxy_is_verified: Optional[bool] = None
+ blocksize: int
source_address: Optional[Tuple[str, int]]
socket_options: Optional[connection._TYPE_SOCKET_OPTIONS]
_tunnel_host: Optional[str]
@@ -288,71 +289,86 @@ class HTTPConnection(_HTTPConnection):
url: str,
body: Optional[_TYPE_BODY] = None,
headers: Optional[Mapping[str, str]] = None,
+ chunked: bool = False,
) -> None:
- if headers is None:
- headers = {}
- else:
- # Avoid modifying the headers passed into .request()
- headers = copy(headers)
- # Don't send bytes keys to httplib to avoid bytes/str comparison
- # HTTPHeaderDict is already safe, but other types are not
- for key, value in list(headers.items()):
- if isinstance(key, bytes):
- headers.pop(key)
- # httplib would have decoded to latin-1 anyway
- headers[key.decode("latin-1")] = value
-
- if "user-agent" not in (to_str(k.lower()) for k in headers):
- updated_headers = {"User-Agent": _get_default_user_agent()}
- updated_headers.update(headers)
- headers = updated_headers
- super().request(method, url, body=body, headers=headers)
- def request_chunked(
- self,
- method: str,
- url: str,
- body: Optional[_TYPE_BODY] = None,
- headers: Optional[Mapping[str, str]] = None,
- ) -> None:
- """
- Alternative to the common request method, which sends the
- body with chunked encoding and not as one block
- """
if headers is None:
headers = {}
- header_keys = {to_str(k.lower()) for k in headers}
+ header_keys = frozenset(to_str(k.lower()) for k in headers)
skip_accept_encoding = "accept-encoding" in header_keys
skip_host = "host" in header_keys
self.putrequest(
method, url, skip_accept_encoding=skip_accept_encoding, skip_host=skip_host
)
+
+ # Transform the body into an iterable of sendall()-able chunks
+ # and detect if an explicit Content-Length is doable.
+ chunks_and_cl = body_to_chunks(body, method=method, blocksize=self.blocksize)
+ chunks = chunks_and_cl.chunks
+ content_length = chunks_and_cl.content_length
+
+ # When chunked is explicit set to 'True' we respect that.
+ if chunked:
+ if "transfer-encoding" not in header_keys:
+ self.putheader("Transfer-Encoding", "chunked")
+ else:
+ # Detect whether a framing mechanism is already in use. If so
+ # we respect that value, otherwise we pick chunked vs content-length
+ # depending on the type of 'body'.
+ if "content-length" in header_keys:
+ chunked = False
+ elif "transfer-encoding" in header_keys:
+ chunked = True
+
+ # Otherwise we go off the recommendation of 'body_to_chunks()'.
+ else:
+ chunked = False
+ if content_length is None:
+ if chunks is not None:
+ chunked = True
+ self.putheader("Transfer-Encoding", "chunked")
+ else:
+ self.putheader("Content-Length", str(content_length))
+
+ # Now that framing headers are out of the way we send all the other headers.
if "user-agent" not in header_keys:
self.putheader("User-Agent", _get_default_user_agent())
for header, value in headers.items():
self.putheader(header, value)
- if "transfer-encoding" not in header_keys:
- self.putheader("Transfer-Encoding", "chunked")
self.endheaders()
- if body is not None:
- if isinstance(body, (str, bytes)):
- body = (to_bytes(body),)
- for chunk in body:
+ # If we're given a body we start sending that in chunks.
+ if chunks is not None:
+ for chunk in chunks:
+ # Sending empty chunks isn't allowed for TE: chunked
+ # as it indicates the end of the body.
if not chunk:
continue
- if not isinstance(chunk, bytes):
- chunk = chunk.encode("utf8")
- len_str = hex(len(chunk))[2:]
- to_send = bytearray(len_str.encode())
- to_send += b"\r\n"
- to_send += chunk
- to_send += b"\r\n"
- self.send(to_send)
-
- # After the if clause, to always have a closed body
+ if isinstance(chunk, str):
+ chunk = chunk.encode("utf-8")
+ if chunked:
+ self.send(b"%x\r\n%b\r\n" % (len(chunk), chunk))
+ else:
+ self.send(chunk)
+
+ # Regardless of whether we have a body or not, if we're in
+ # chunked mode we want to send an explicit empty chunk.
+ if chunked:
self.send(b"0\r\n\r\n")
+ def request_chunked(
+ self,
+ method: str,
+ url: str,
+ body: Optional[_TYPE_BODY] = None,
+ headers: Optional[Mapping[str, str]] = None,
+ ) -> None:
+ """
+ Alternative to the common request method, which sends the
+ body with chunked encoding and not as one block
+ """
+ self.request(method, url, body=body, headers=headers, chunked=True)
+
class HTTPSConnection(HTTPConnection):
"""

MODIFY
src/urllib3/util/request.py
src/urllib3/util/request.py
+import io
from base64 import b64encode
from enum import Enum
-from typing import IO, TYPE_CHECKING, Any, AnyStr, Dict, List, Optional, Union
+from typing import (
+ IO,
+ TYPE_CHECKING,
+ Any,
+ AnyStr,
+ Dict,
+ Iterable,
+ List,
+ NamedTuple,
+ Optional,
+ Union,
+)
from ..exceptions import UnrewindableBodyError
+from .util import to_bytes
if TYPE_CHECKING:
from typing_extensions import Final
@@ -34,6 +47,13 @@ _FAILEDTELL: "Final[_TYPE_FAILEDTELL]" = _TYPE_FAILEDTELL.token
_TYPE_BODY_POSITION = Union[int, _TYPE_FAILEDTELL]
+# When sending a request with these methods we aren't expecting
+# a body so don't need to set an explicit 'Content-Length: 0'
+# The reason we do this in the negative instead of tracking methods
+# which 'should' have a body is because unknown methods should be
+# treated as if they were 'POST' which *does* expect a body.
+_METHODS_NOT_EXPECTING_BODY = {"GET", "HEAD", "DELETE", "TRACE", "OPTIONS", "CONNECT"}
+
def make_headers(
keep_alive: Optional[bool] = None,
@@ -162,3 +182,78 @@ def rewind_body(body: IO[AnyStr], body_pos: _TYPE_BODY_POSITION) -> None:
raise ValueError(
f"body_pos must be of type integer, instead it was {type(body_pos)}."
)
+
+
+class ChunksAndContentLength(NamedTuple):
+ chunks: Optional[Iterable[bytes]]
+ content_length: Optional[int]
+
+
+def body_to_chunks(
+ body: Optional[Any], method: str, blocksize: int
+) -> ChunksAndContentLength:
+ """Takes the HTTP request method, body, and blocksize and
+ transforms them into an iterable of chunks to pass to
+ socket.sendall() and an optional 'Content-Length' header.
+
+ A 'Content-Length' of 'None' indicates the length of the body
+ can't be determined so should use 'Transfer-Encoding: chunked'
+ for framing instead.
+ """
+
+ chunks: Optional[Iterable[bytes]]
+ content_length: Optional[int]
+
+ # No body, we need to make a recommendation on 'Content-Length'
+ # based on whether that request method is expected to have
+ # a body or not.
+ if body is None:
+ chunks = None
+ if method.upper() not in _METHODS_NOT_EXPECTING_BODY:
+ content_length = 0
+ else:
+ content_length = None
+
+ # Bytes or strings become bytes
+ elif isinstance(body, (str, bytes)):
+ chunks = (to_bytes(body),)
+ content_length = len(chunks[0])
+
+ # File-like object, TODO: use seek() and tell() for length?
+ elif hasattr(body, "read"):
+
+ def chunk_readable() -> Iterable[bytes]:
+ nonlocal body, blocksize
+ encode = isinstance(body, io.TextIOBase)
+ while True:
+ datablock = body.read(blocksize) # type: ignore[union-attr]
+ if not datablock:
+ break
+ if encode:
+ datablock = datablock.encode("iso-8859-1")
+ yield datablock
+
+ chunks = chunk_readable()
+ content_length = None
+
+ # Otherwise we need to start checking via duck-typing.
+ else:
+ try:
+ # Check if the body implements the buffer API.
+ mv = memoryview(body)
+ except TypeError:
+ try:
+ # Check if the body is an iterable
+ chunks = iter(body)
+ content_length = None
+ except TypeError:
+ raise TypeError(
+ f"'body' must be a bytes-like object, file-like "
+ f"object, or iterable. Instead was {body!r}"
+ ) from None
+ else:
+ # Since it implements the buffer API can be passed directly to socket.sendall()
+ chunks = (body,)
+ content_length = mv.nbytes
+
+ return ChunksAndContentLength(chunks=chunks, content_length=content_length)

MODIFY
test/test_poolmanager.py
test/test_poolmanager.py
@@ -427,7 +427,7 @@ class TestPoolManager:
"http://example.com", {"blocksize": input_blocksize}
)
assert pool_blocksize.conn_kw["blocksize"] == expected_blocksize
- assert pool_blocksize._get_conn().blocksize == expected_blocksize # type: ignore[attr-defined]
+ assert pool_blocksize._get_conn().blocksize == expected_blocksize
@pytest.mark.parametrize(
"url",

MODIFY
test/with_dummyserver/test_poolmanager.py
test/with_dummyserver/test_poolmanager.py
@@ -344,6 +344,9 @@ class TestPoolManager(HTTPDummyServerTestCase):
assert returned_headers.get("Baz") == "quux"
def test_headers_http_header_dict(self) -> None:
+ # Test uses a list of headers to assert the order
+ # that headers are sent in the request too.
+
headers = HTTPHeaderDict()
headers.add("Foo", "bar")
headers.add("Multi", "1")
@@ -351,26 +354,31 @@ class TestPoolManager(HTTPDummyServerTestCase):
headers.add("Multi", "2")
with PoolManager(headers=headers) as http:
- r = http.request("GET", f"{self.base_url}/headers")
- returned_headers = r.json()
- assert returned_headers["Foo"] == "bar"
- assert returned_headers["Multi"] == "1, 2"
- assert returned_headers["Baz"] == "quux"
+ r = http.request("GET", f"{self.base_url}/multi_headers")
+ returned_headers = r.json()["headers"]
+ assert returned_headers[-4:] == [
+ ["Foo", "bar"],
+ ["Multi", "1"],
+ ["Multi", "2"],
+ ["Baz", "quux"],
+ ]
r = http.request(
"GET",
- f"{self.base_url}/headers",
+ f"{self.base_url}/multi_headers",
headers={
**headers,
"Extra": "extra",
"Foo": "new",
},
)
- returned_headers = r.json()
- assert returned_headers["Foo"] == "new"
- assert returned_headers["Multi"] == "1, 2"
- assert returned_headers["Baz"] == "quux"
- assert returned_headers["Extra"] == "extra"
+ returned_headers = r.json()["headers"]
+ assert returned_headers[-4:] == [
+ ["Foo", "new"],
+ ["Multi", "1, 2"],
+ ["Baz", "quux"],
+ ["Extra", "extra"],
+ ]
def test_body(self) -> None:
with PoolManager() as http:
@@ -542,6 +550,22 @@ class TestPoolManager(HTTPDummyServerTestCase):
body = {"attribute": "value"}
request(method="POST", url=f"{self.base_url}/echo", body="", json=body)
+ def test_top_level_request_with_invalid_body(self) -> None:
+ class BadBody:
+ def __repr__(self) -> str:
+ return "<BadBody>"
+
+ with pytest.raises(TypeError) as e:
+ request(
+ method="POST",
+ url=f"{self.base_url}/echo",
+ body=BadBody(), # type: ignore[arg-type]
+ )
+ assert str(e.value) == (
+ "'body' must be a bytes-like object, file-like "
+ "object, or iterable. Instead was <BadBody>"
+ )
+
@pytest.mark.skipif(not HAS_IPV6, reason="IPv6 is not supported on this system")
class TestIPv6PoolManager(IPv6HTTPDummyServerTestCase):

MODIFY
test/with_dummyserver/test_socketlevel.py
test/with_dummyserver/test_socketlevel.py
@@ -21,7 +21,7 @@ from test import (
resolvesLocalhostFQDN,
)
from threading import Event
-from typing import Any, Generator, List, Optional
+from typing import Any, Callable, Generator, List, Optional
from typing import OrderedDict as OrderedDictType
from typing import Tuple, Union
from unittest import mock
@@ -2035,12 +2035,12 @@ class TestContentFraming(SocketDummyServerTestCase):
assert b"Transfer-Encoding: chunked\r\n" in sent_bytes
assert b"User-Agent: python-urllib3/" in sent_bytes
assert b"content-length" not in sent_bytes.lower()
-
- # TODO: Remove the .lower() after solving #2515
- assert b"\r\n\r\na\r\nxxxxxxxxxx\r\n0\r\n\r\n" in sent_bytes.lower()
+ assert b"\r\n\r\na\r\nxxxxxxxxxx\r\n0\r\n\r\n" in sent_bytes
@pytest.mark.parametrize("method", ["POST", "PUT", "PATCH"])
- @pytest.mark.parametrize("body_type", ["file", "generator", "bytes"])
+ @pytest.mark.parametrize(
+ "body_type", ["file", "generator", "bytes", "bytearray", "file_text"]
+ )
def test_chunked_not_specified(self, method: str, body_type: str) -> None:
buffer = bytearray()
@@ -2079,6 +2079,15 @@ class TestContentFraming(SocketDummyServerTestCase):
body.seek(0, 0)
should_be_chunked = True
+ elif body_type == "file_text":
+ body = io.StringIO("x" * 10)
+ body.seek(0, 0)
+ should_be_chunked = True
+
+ elif body_type == "bytearray":
+ body = bytearray(b"x" * 10)
+ should_be_chunked = False
+
else:
body = b"x" * 10
should_be_chunked = False
@@ -2098,10 +2107,68 @@ class TestContentFraming(SocketDummyServerTestCase):
if should_be_chunked:
assert b"content-length" not in sent_bytes.lower()
assert b"Transfer-Encoding: chunked\r\n" in sent_bytes
- # TODO: Remove the .lower() after solving #2515
- assert b"\r\n\r\na\r\nxxxxxxxxxx\r\n0\r\n\r\n" in sent_bytes.lower()
+ assert b"\r\n\r\na\r\nxxxxxxxxxx\r\n0\r\n\r\n" in sent_bytes
else:
assert b"Content-Length: 10\r\n" in sent_bytes
assert b"transfer-encoding" not in sent_bytes.lower()
assert sent_bytes.endswith(b"\r\n\r\nxxxxxxxxxx")
+
+ @pytest.mark.parametrize(
+ "header_transform",
+ [str.lower, str.title, str.upper],
+ )
+ @pytest.mark.parametrize(
+ ["header", "header_value", "expected"],
+ [
+ ("content-length", "10", b": 10\r\n\r\nxxxxxxxx"),
+ (
+ "transfer-encoding",
+ "chunked",
+ b": chunked\r\n\r\n8\r\nxxxxxxxx\r\n0\r\n\r\n",
+ ),
+ ],
+ )
+ def test_framing_set_via_headers(
+ self,
+ header_transform: Callable[[str], str],
+ header: str,
+ header_value: str,
+ expected: bytes,
+ ) -> None:
+ buffer = bytearray()
+
+ def socket_handler(listener: socket.socket) -> None:
+ nonlocal buffer
+ sock = listener.accept()[0]
+ sock.settimeout(0)
+
+ start = time.time()
+ while time.time() - start < (LONG_TIMEOUT / 2):
+ try:
+ buffer += sock.recv(65536)
+ except OSError:
+ continue
+
+ sock.sendall(
+ b"HTTP/1.1 200 OK\r\n"
+ b"Server: example.com\r\n"
+ b"Content-Length: 0\r\n\r\n"
+ )
+ sock.close()
+
+ self._start_server(socket_handler)
+
+ with HTTPConnectionPool(
+ self.host, self.port, timeout=LONG_TIMEOUT, retries=False
+ ) as pool:
+ resp = pool.request(
+ "POST",
+ "/",
+ body=b"xxxxxxxx",
+ headers={header_transform(header): header_value},
+ )
+ assert resp.status == 200
+
+ sent_bytes = bytes(buffer)
+ assert sent_bytes.endswith(expected)

Create our own logic for chunked content framing

Co-authored-by: Quentin Pradet <quentin.pradet@gmail.com>