🎨 minor adjustments for compatibility purposes (#128)

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.
jawah · Jun 27, 2024 · b8037d0 · b8037d0
1 parent 39f656d
commit b8037d0
Show file tree

Hide file tree

Showing 9 changed files with 59 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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()