diff --git a/CHANGES.rst b/CHANGES.rst index 3bbd414dcb..a8d2902cca 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,11 @@ +2.8.901 (2024-06-27) +==================== + +- Improved compatibility with httplib exception for ``IncompleteRead`` that did not behave exactly like expected (repr/str format over it). +- The metric TLS handshake delay was wrongfully set when using HTTP/2 over cleartext. +- Fixed compatibility with some third-party mocking library that are injecting io.BytesIO in HTTPResponse. + In some cases, ``IncompleteRead`` might not be raised like expected. + 2.8.900 (2024-06-24) ==================== diff --git a/src/urllib3/_async/response.py b/src/urllib3/_async/response.py index a72e0c90f9..b5b9407777 100644 --- a/src/urllib3/_async/response.py +++ b/src/urllib3/_async/response.py @@ -286,7 +286,20 @@ async def _raw_read( # type: ignore[override] async with self._error_catcher(): data = (await self._fp_read(amt)) if not fp_closed else b"" - if amt is not None and amt != 0 and not data: + + # Mocking library often use io.BytesIO + # which does not auto-close when reading data + # with amt=None. + is_foreign_fp_unclosed = ( + amt is None and getattr(self._fp, "closed", False) is False + ) + + if (amt is not None and amt != 0 and not data) or is_foreign_fp_unclosed: + if is_foreign_fp_unclosed: + self._fp_bytes_read += len(data) + if self.length_remaining is not None: + self.length_remaining -= len(data) + # Platform-specific: Buggy versions of Python. # Close the connection when no data is returned # @@ -308,10 +321,11 @@ async def _raw_read( # type: ignore[override] # Content-Length are caught. raise IncompleteRead(self._fp_bytes_read, self.length_remaining) - if data: + if data and not is_foreign_fp_unclosed: self._fp_bytes_read += len(data) if self.length_remaining is not None: self.length_remaining -= len(data) + return data async def read( # type: ignore[override] diff --git a/src/urllib3/_version.py b/src/urllib3/_version.py index 48728bf16b..c8fb8156d7 100644 --- a/src/urllib3/_version.py +++ b/src/urllib3/_version.py @@ -1,4 +1,4 @@ # This file is protected via CODEOWNERS from __future__ import annotations -__version__ = "2.8.900" +__version__ = "2.8.901" diff --git a/src/urllib3/backend/_async/hface.py b/src/urllib3/backend/_async/hface.py index acd432301f..4803f62fb3 100644 --- a/src/urllib3/backend/_async/hface.py +++ b/src/urllib3/backend/_async/hface.py @@ -477,7 +477,11 @@ async def _post_conn(self) -> None: # type: ignore[override] # save the quic ticket for session resumption self.__session_ticket = self._protocol.session_ticket - if hasattr(self, "_connect_timings"): + if ( + self.conn_info.certificate_der + and hasattr(self, "_connect_timings") + and not self.conn_info.tls_handshake_latency + ): self.conn_info.tls_handshake_latency = ( datetime.now(tz=timezone.utc) - self._connect_timings[-1] ) diff --git a/src/urllib3/backend/hface.py b/src/urllib3/backend/hface.py index c90f6abd44..d0ab9546ac 100644 --- a/src/urllib3/backend/hface.py +++ b/src/urllib3/backend/hface.py @@ -539,7 +539,11 @@ def _post_conn(self) -> None: # save the quic ticket for session resumption self.__session_ticket = self._protocol.session_ticket - if hasattr(self, "_connect_timings"): + if ( + self.conn_info.certificate_der + and hasattr(self, "_connect_timings") + and not self.conn_info.tls_handshake_latency + ): self.conn_info.tls_handshake_latency = ( datetime.now(tz=timezone.utc) - self._connect_timings[-1] ) diff --git a/src/urllib3/exceptions.py b/src/urllib3/exceptions.py index 27db72918d..d8c11a8e2a 100644 --- a/src/urllib3/exceptions.py +++ b/src/urllib3/exceptions.py @@ -265,18 +265,16 @@ class IncompleteRead(ProtocolError): for ``partial`` to avoid creating large objects on streamed reads. """ - def __init__(self, partial: int, expected: int) -> None: - super().__init__( - f"peer closed connection without sending complete message body (received {partial} bytes, expected {expected} more)" - ) + def __init__(self, partial: int, expected: int | None = None) -> None: self.partial = partial self.expected = expected def __repr__(self) -> str: - return "IncompleteRead(%i bytes read, %i more expected)" % ( - self.partial, - self.expected, - ) + if self.expected is not None: + return f"IncompleteRead({self.partial} bytes read, {self.expected} more expected)" + return f"IncompleteRead({self.partial} bytes read)" + + __str__ = object.__str__ class InvalidChunkLength(ProtocolError): diff --git a/src/urllib3/response.py b/src/urllib3/response.py index 05e67af05b..4eaa756424 100644 --- a/src/urllib3/response.py +++ b/src/urllib3/response.py @@ -760,7 +760,19 @@ def _raw_read( with self._error_catcher(): data = self._fp_read(amt) if not fp_closed else b"" - if amt is not None and amt != 0 and not data: + + # Mocking library often use io.BytesIO + # which does not auto-close when reading data + # with amt=None. + is_foreign_fp_unclosed = ( + amt is None and getattr(self._fp, "closed", False) is False + ) + + if (amt is not None and amt != 0 and not data) or is_foreign_fp_unclosed: + if is_foreign_fp_unclosed: + self._fp_bytes_read += len(data) + if self.length_remaining is not None: + self.length_remaining -= len(data) # Platform-specific: Buggy versions of Python. # Close the connection when no data is returned # @@ -782,10 +794,11 @@ def _raw_read( # Content-Length are caught. raise IncompleteRead(self._fp_bytes_read, self.length_remaining) - if data: + if data and not is_foreign_fp_unclosed: self._fp_bytes_read += len(data) if self.length_remaining is not None: self.length_remaining -= len(data) + return data def read( diff --git a/test/test_response.py b/test/test_response.py index 41ab63fb2a..e347f08b59 100644 --- a/test/test_response.py +++ b/test/test_response.py @@ -138,7 +138,7 @@ def test_preload(self) -> None: r = HTTPResponse(fp, preload_content=True) - assert fp.tell() == len(b"foo") + assert fp.closed is True assert r.data == b"foo" def test_no_preload(self) -> None: @@ -148,7 +148,7 @@ def test_no_preload(self) -> None: assert fp.tell() == 0 assert r.data == b"foo" - assert fp.tell() == len(b"foo") + assert fp.closed is True def test_decode_bad_data(self) -> None: fp = BytesIO(b"\x00" * 10) diff --git a/test/with_dummyserver/test_socketlevel.py b/test/with_dummyserver/test_socketlevel.py index 91cb0a494f..d5113dadc2 100644 --- a/test/with_dummyserver/test_socketlevel.py +++ b/test/with_dummyserver/test_socketlevel.py @@ -2047,7 +2047,7 @@ def socket_handler(listener: socket.socket) -> None: ) data = get_response.stream(100) with pytest.raises( - IncompleteRead, match="received 12 bytes, expected 10 more" + IncompleteRead, match=r"12 bytes read\, 10 more expected" ): next(data) done_event.set()