Fix `OverflowError` when TLS is used on some Python versions
ADD
changelog/2513.bugfix.rst
+Fixed reading 2 or more GiB of data at a time using ``urllib3.response.HTTPResponse.read``.
\ No newline at end of file

MODIFY
src/urllib3/response.py
src/urllib3/response.py
@@ -2,6 +2,7 @@ import io
import json as _json
import logging
import re
+import sys
import zlib
from contextlib import contextmanager
from http.client import HTTPMessage as _HttplibHTTPMessage
@@ -44,6 +45,7 @@ try:
except (AttributeError, ImportError, ValueError): # Defensive:
zstd = None
+from . import util
from ._collections import HTTPHeaderDict
from .connection import _TYPE_BODY, BaseSSLError, HTTPConnection, HTTPException
from .exceptions import (
@@ -665,6 +667,54 @@ class HTTPResponse(BaseHTTPResponse):
if self._original_response and self._original_response.isclosed():
self.release_conn()
+ def _fp_read(self, amt: Optional[int] = None) -> bytes:
+ """
+ Read a response with the thought that reading the number of bytes
+ larger than can fit in a 32-bit int at a time via SSL in some
+ known cases leads to an overflow error that has to be prevented
+ if `amt` or `self.length_remaining` indicate that a problem may
+ happen.
+
+ The known cases:
+ * 3.8 <= CPython < 3.9.7 because of a bug
+ https://github.com/urllib3/urllib3/issues/2513#issuecomment-1152559900.
+ * urllib3 injected with pyOpenSSL-backed SSL-support.
+ * CPython < 3.10 only when `amt` does not fit 32-bit int.
+ """
+ assert self._fp
+ c_int_max = 2**31 - 1
+ if (
+ (
+ (amt and amt > c_int_max)
+ or (self.length_remaining and self.length_remaining > c_int_max)
+ )
+ and not util.IS_SECURETRANSPORT
+ and (util.IS_PYOPENSSL or sys.version_info < (3, 10))
+ ):
+ buffer = io.BytesIO()
+ # Besides `max_chunk_amt` being a maximum chunk size, it
+ # affects memory overhead of reading a response by this
+ # method in CPython.
+ # `c_int_max` equal to 2 GiB - 1 byte is the actual maximum
+ # chunk size that does not lead to an overflow error, but
+ # 256 MiB is a compromise.
+ max_chunk_amt = 2**28
+ while amt is None or amt != 0:
+ if amt is not None:
+ chunk_amt = min(amt, max_chunk_amt)
+ amt -= chunk_amt
+ else:
+ chunk_amt = max_chunk_amt
+ data = self._fp.read(chunk_amt)
+ if not data:
+ break
+ buffer.write(data)
+ del data # to reduce peak memory usage by `max_chunk_amt`.
+ return buffer.getvalue()
+ else:
+ # StringIO doesn't like amt=None
+ return self._fp.read(amt) if amt is not None else self._fp.read()
+
def read(
self,
amt: Optional[int] = None,
@@ -702,13 +752,11 @@ class HTTPResponse(BaseHTTPResponse):
fp_closed = getattr(self._fp, "closed", False)
with self._error_catcher():
+ data = self._fp_read(amt) if not fp_closed else b""
if amt is None:
- # cStringIO doesn't like amt=None
- data = self._fp.read() if not fp_closed else b""
flush_decoder = True
else:
cache_content = False
- data = self._fp.read(amt) if not fp_closed else b""
if (
amt != 0 and not data
): # Platform-specific: Buggy versions of Python.

MODIFY
test/with_dummyserver/test_socketlevel.py
test/with_dummyserver/test_socketlevel.py
@@ -8,6 +8,7 @@ import select
import shutil
import socket
import ssl
+import sys
import tempfile
import time
from collections import OrderedDict
@@ -1491,6 +1492,63 @@ class TestSSL(SocketDummyServerTestCase):
pool.request("GET", "/", retries=False, timeout=LONG_TIMEOUT)
assert server_closed.wait(LONG_TIMEOUT), "The socket was not terminated"
+ # SecureTransport can read only small pieces of data at the moment.
+ # https://github.com/urllib3/urllib3/pull/2674
+ @notSecureTransport()
+ @pytest.mark.skipif(
+ os.environ.get("CI") == "true" and sys.implementation.name == "pypy",
+ reason="too slow to run in CI",
+ )
+ @pytest.mark.parametrize(
+ "preload_content,read_amt", [(True, None), (False, None), (False, 2**31)]
+ )
+ def test_requesting_large_resources_via_ssl(
+ self, preload_content: bool, read_amt: Optional[int]
+ ) -> None:
+ """
+ Ensure that it is possible to read 2 GiB or more via an SSL
+ socket.
+ https://github.com/urllib3/urllib3/issues/2513
+ """
+ content_length = 2**31 # (`int` max value in C) + 1.
+ ssl_ready = Event()
+
+ def socket_handler(listener: socket.socket) -> None:
+ sock = listener.accept()[0]
+ ssl_sock = ssl.wrap_socket(
+ sock,
+ server_side=True,
+ keyfile=DEFAULT_CERTS["keyfile"],
+ certfile=DEFAULT_CERTS["certfile"],
+ ca_certs=DEFAULT_CA,
+ )
+ ssl_ready.set()
+
+ while not ssl_sock.recv(65536).endswith(b"\r\n\r\n"):
+ continue
+
+ ssl_sock.send(
+ b"HTTP/1.1 200 OK\r\n"
+ b"Content-Type: text/plain\r\n"
+ b"Content-Length: %d\r\n\r\n" % content_length
+ )
+
+ chunks = 2
+ for i in range(chunks):
+ ssl_sock.sendall(bytes(content_length // chunks))
+
+ ssl_sock.close()
+ sock.close()
+
+ self._start_server(socket_handler)
+ ssl_ready.wait(5)
+ with HTTPSConnectionPool(
+ self.host, self.port, ca_certs=DEFAULT_CA, retries=False
+ ) as pool:
+ response = pool.request("GET", "/", preload_content=preload_content)
+ data = response.data if preload_content else response.read(read_amt)
+ assert len(data) == content_length
+
class TestErrorWrapping(SocketDummyServerTestCase):
def test_bad_statusline(self) -> None:

Fix `OverflowError` when TLS is used on some Python versions

Co-authored-by: Seth Michael Larson <sethmichaellarson@gmail.com>