From 5dd7d8f0aa9bcadedffbc05f25d274565ea261aa Mon Sep 17 00:00:00 2001 From: "TAHRI Ahmed R." Date: Tue, 22 Oct 2024 20:57:01 +0200 Subject: [PATCH] :bug: fix exception leak from urllib3-future when resolving lazy responses (#163) fix #162 --- .pre-commit-config.yaml | 2 +- HISTORY.md | 6 ++ README.md | 76 +++++++++---------- src/niquests/__version__.py | 4 +- src/niquests/adapters.py | 142 +++++++++++++++++++++++++++++++++++- src/niquests/models.py | 2 +- 6 files changed, 189 insertions(+), 43 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8227d5d15b..b22eda2971 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -23,7 +23,7 @@ repos: # Run the formatter. - id: ruff-format - repo: https://github.com/pre-commit/mirrors-mypy - rev: v1.11.2 + rev: v1.12.1 hooks: - id: mypy args: [--check-untyped-defs] diff --git a/HISTORY.md b/HISTORY.md index 07d85445a9..e1d5e75d17 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,12 @@ Release History =============== +3.10.1 (2024-10-22) +------------------ + +**Fixed** +- Exception leak from urllib3-future when gathering / resolving lazy responses. + 3.10.0 (2024-10-21) ------------------ diff --git a/README.md b/README.md index b67616c6b4..3d80f1c663 100644 --- a/README.md +++ b/README.md @@ -12,41 +12,41 @@ Niquests, is the “**Safest**, **Fastest[^10]**, **Easiest**, and **Most advanc
👆 Look at the feature table comparison against requests, httpx and aiohttp! -| Feature | niquests | requests | httpx | aiohttp | -|-------------------------------------|:--------------:|:---------:|:-------------:|---------------| -| `HTTP/1.1` | ✅ | ✅ | ✅ | ✅ | -| `HTTP/2` | ✅ | ❌ | ✅[^7] | ❌ | -| `HTTP/3 over QUIC` | ✅ | ❌ | ❌ | ❌ | -| `Synchronous` | ✅ | ✅ | ✅ | _N/A_[^1] | -| `Asynchronous` | ✅ | ❌ | ✅ | ✅ | -| `Thread Safe` | ✅ | ✅ | ❌[^5] | _N/A_[^1] | -| `Task Safe` | ✅ | _N/A_[^2] | ✅ | ✅ | -| `OS Trust Store` | ✅ | ❌ | ❌ | ❌ | -| `Multiplexing` | ✅ | ❌ | _Limited_[^3] | ❌ | -| `DNSSEC` | ✅[^11] | ❌ | ❌ | ❌ | -| `Customizable DNS Resolution` | ✅ | ❌ | ❌ | ✅ | -| `DNS over HTTPS` | ✅ | ❌ | ❌ | ❌ | -| `DNS over QUIC` | ✅ | ❌ | ❌ | ❌ | -| `DNS over TLS` | ✅ | ❌ | ❌ | ❌ | -| `Multiple DNS Resolver` | ✅ | ❌ | ❌ | ❌ | -| `Network Fine Tuning & Inspect` | ✅ | ❌ | _Limited_[^6] | _Limited_[^6] | -| `Certificate Revocation Protection` | ✅ | ❌ | ❌ | ❌ | -| `Session Persistence` | ✅ | ✅ | ✅ | ✅ | -| `In-memory Certificate CA & mTLS` | ✅ | ❌ | _Limited_[^4] | _Limited_[^4] | -| `SOCKS 4/5 Proxies` | ✅ | ✅ | ✅ | ❌ | -| `HTTP/HTTPS Proxies` | ✅ | ✅ | ✅ | ✅ | -| `TLS-in-TLS Support` | ✅ | ✅ | ✅ | ✅ | -| `Direct HTTP/3 Negotiation` | ✅[^9] | N/A[^8] | N/A[^8] | N/A[^8] | -| `Happy Eyeballs` | ✅ | ❌ | ❌ | ✅ | -| `Package / SLSA Signed` | ✅ | ❌ | ❌ | ✅ | -| `HTTP/2 with prior knowledge (h2c)` | ✅ | ❌ | ✅ | ❌ | -| `Post-Quantum Security` | _Limited_[^12] | ❌ | ❌ | ❌ | -| `HTTP Trailers` | ✅ | ❌ | ❌ | ❌ | -| `Early Responses` | ✅ | ❌ | ❌ | ❌ | -| `WebSocket over HTTP/1` | ✅ | ❌[^14] | ❌[^14] | ✅ | -| `WebSocket over HTTP/2 and HTTP/3` | ✅[^13] | ❌ | ❌ | ❌ | -| `Automatic Ping for HTTP/2+` | ✅ | N/A | ❌ | N/A | -| `Automatic Connection Upgrade / Downgrade` | ✅ | N/A | ❌ | N/A | +| Feature | niquests | requests | httpx | aiohttp | +|--------------------------------------------|:--------------:|:---------:|:-------------:|---------------| +| `HTTP/1.1` | ✅ | ✅ | ✅ | ✅ | +| `HTTP/2` | ✅ | ❌ | ✅[^7] | ❌ | +| `HTTP/3 over QUIC` | ✅ | ❌ | ❌ | ❌ | +| `Synchronous` | ✅ | ✅ | ✅ | _N/A_[^1] | +| `Asynchronous` | ✅ | ❌ | ✅ | ✅ | +| `Thread Safe` | ✅ | ✅ | ❌[^5] | _N/A_[^1] | +| `Task Safe` | ✅ | _N/A_[^2] | ✅ | ✅ | +| `OS Trust Store` | ✅ | ❌ | ❌ | ❌ | +| `Multiplexing` | ✅ | ❌ | _Limited_[^3] | ❌ | +| `DNSSEC` | ✅[^11] | ❌ | ❌ | ❌ | +| `Customizable DNS Resolution` | ✅ | ❌ | ❌ | ✅ | +| `DNS over HTTPS` | ✅ | ❌ | ❌ | ❌ | +| `DNS over QUIC` | ✅ | ❌ | ❌ | ❌ | +| `DNS over TLS` | ✅ | ❌ | ❌ | ❌ | +| `Multiple DNS Resolver` | ✅ | ❌ | ❌ | ❌ | +| `Network Fine Tuning & Inspect` | ✅ | ❌ | _Limited_[^6] | _Limited_[^6] | +| `Certificate Revocation Protection` | ✅ | ❌ | ❌ | ❌ | +| `Session Persistence` | ✅ | ✅ | ✅ | ✅ | +| `In-memory Certificate CA & mTLS` | ✅ | ❌ | _Limited_[^4] | _Limited_[^4] | +| `SOCKS 4/5 Proxies` | ✅ | ✅ | ✅ | ❌ | +| `HTTP/HTTPS Proxies` | ✅ | ✅ | ✅ | ✅ | +| `TLS-in-TLS Support` | ✅ | ✅ | ✅ | ✅ | +| `Direct HTTP/3 Negotiation` | ✅[^9] | N/A[^8] | N/A[^8] | N/A[^8] | +| `Happy Eyeballs` | ✅ | ❌ | ❌ | ✅ | +| `Package / SLSA Signed` | ✅ | ❌ | ❌ | ✅ | +| `HTTP/2 with prior knowledge (h2c)` | ✅ | ❌ | ✅ | ❌ | +| `Post-Quantum Security` | _Limited_[^12] | ❌ | ❌ | ❌ | +| `HTTP Trailers` | ✅ | ❌ | ❌ | ❌ | +| `Early Responses` | ✅ | ❌ | ❌ | ❌ | +| `WebSocket over HTTP/1` | ✅ | ❌[^14] | ❌[^14] | ✅ | +| `WebSocket over HTTP/2 and HTTP/3` | ✅[^13] | ❌ | ❌ | ❌ | +| `Automatic Ping for HTTP/2+` | ✅ | N/A | ❌ | N/A | +| `Automatic Connection Upgrade / Downgrade` | ✅ | N/A | ❌ | N/A |
@@ -67,9 +67,9 @@ _Scenario:_ Fetch a thousand requests using 10 tasks or threads, each with a hun | Client | Average Delay to Complete | Notes | |---------------|---------------------------|------------------------------| | requests core | 643 ms or ~1555 req/s | ThreadPoolExecutor. HTTP/1.1 | -| httpx core | 530 ms or ~1886 req/s | Asyncio. HTTP/2 | +| httpx core | 490 ms or ~2000 req/s | Asyncio. HTTP/2 | | aiohttp | 210 ms or ~4762 req/s | Asyncio. HTTP/1.1 | -| niquests core | 170 ms or ~5882 req/s | Asyncio. HTTP/2 | +| niquests core | 160 ms or ~6200 req/s | Asyncio. HTTP/2 | Did you give up on HTTP/2 due to performance concerns? Think again! Do you realize that you can get 3 times faster with the same CPU if you ever switched to Niquests from Requests? Multiplexing and response lazyness open up a wide range of possibilities! Want to learn more about the tests? scripts? reasoning? @@ -83,6 +83,8 @@ Take a deeper look at https://github.com/Ousret/niquests-stats >>> import niquests >>> s = niquests.Session(resolver="doh+google://", multiplexed=True) >>> r = s.get('https://pie.dev/basic-auth/user/pass', auth=('user', 'pass')) +>>> r + >>> r.status_code 200 >>> r.headers['content-type'] diff --git a/src/niquests/__version__.py b/src/niquests/__version__.py index 68008f3a72..99d104e593 100644 --- a/src/niquests/__version__.py +++ b/src/niquests/__version__.py @@ -9,9 +9,9 @@ __url__: str = "https://niquests.readthedocs.io" __version__: str -__version__ = "3.10.0" +__version__ = "3.10.1" -__build__: int = 0x031000 +__build__: int = 0x031001 __author__: str = "Kenneth Reitz" __author_email__: str = "me@kennethreitz.org" __license__: str = "Apache-2.0" diff --git a/src/niquests/adapters.py b/src/niquests/adapters.py index 4e6a8d574c..e69f71f71e 100644 --- a/src/niquests/adapters.py +++ b/src/niquests/adapters.py @@ -1232,7 +1232,42 @@ def gather(self, *responses: Response, max_fetch: int | None = None) -> None: self._orphaned.remove(low_resp) if low_resp is None: - low_resp = self.poolmanager.get_response() + try: + low_resp = self.poolmanager.get_response() + except (ProtocolError, OSError) as err: + raise ConnectionError(err) + + except MaxRetryError as e: + if isinstance(e.reason, ConnectTimeoutError): + # TODO: Remove this in 3.0.0: see #2811 + if not isinstance(e.reason, NewConnectionError): + raise ConnectTimeout(e) + + if isinstance(e.reason, ResponseError): + raise RetryError(e) + + if isinstance(e.reason, _ProxyError): + raise ProxyError(e) + + if isinstance(e.reason, _SSLError): + # This branch is for urllib3 v1.22 and later. + raise SSLError(e) + + raise ConnectionError(e) + + except _ProxyError as e: + raise ProxyError(e) + + except (_SSLError, _HTTPError) as e: + if isinstance(e, _SSLError): + # This branch is for urllib3 versions earlier than v1.22 + raise SSLError(e) + elif isinstance(e, ReadTimeoutError): + raise ReadTimeout(e) + elif isinstance(e, _InvalidHeader): + raise InvalidHeader(e) + else: + raise if low_resp is None: break @@ -1278,6 +1313,40 @@ def gather(self, *responses: Response, max_fetch: int | None = None) -> None: ) except ValueError: low_resp = None + except (ProtocolError, OSError) as err: + raise ConnectionError(err) + + except MaxRetryError as e: + if isinstance(e.reason, ConnectTimeoutError): + # TODO: Remove this in 3.0.0: see #2811 + if not isinstance(e.reason, NewConnectionError): + raise ConnectTimeout(e) + + if isinstance(e.reason, ResponseError): + raise RetryError(e) + + if isinstance(e.reason, _ProxyError): + raise ProxyError(e) + + if isinstance(e.reason, _SSLError): + # This branch is for urllib3 v1.22 and later. + raise SSLError(e) + + raise ConnectionError(e) + + except _ProxyError as e: + raise ProxyError(e) + + except (_SSLError, _HTTPError) as e: + if isinstance(e, _SSLError): + # This branch is for urllib3 versions earlier than v1.22 + raise SSLError(e) + elif isinstance(e, ReadTimeoutError): + raise ReadTimeout(e) + elif isinstance(e, _InvalidHeader): + raise InvalidHeader(e) + else: + raise if low_resp is None: raise MultiplexingError( @@ -2257,7 +2326,42 @@ async def gather( self._orphaned.remove(low_resp) if low_resp is None: - low_resp = await self.poolmanager.get_response() + try: + low_resp = await self.poolmanager.get_response() + except (ProtocolError, OSError) as err: + raise ConnectionError(err) + + except MaxRetryError as e: + if isinstance(e.reason, ConnectTimeoutError): + # TODO: Remove this in 3.0.0: see #2811 + if not isinstance(e.reason, NewConnectionError): + raise ConnectTimeout(e) + + if isinstance(e.reason, ResponseError): + raise RetryError(e) + + if isinstance(e.reason, _ProxyError): + raise ProxyError(e) + + if isinstance(e.reason, _SSLError): + # This branch is for urllib3 v1.22 and later. + raise SSLError(e) + + raise ConnectionError(e) + + except _ProxyError as e: + raise ProxyError(e) + + except (_SSLError, _HTTPError) as e: + if isinstance(e, _SSLError): + # This branch is for urllib3 versions earlier than v1.22 + raise SSLError(e) + elif isinstance(e, ReadTimeoutError): + raise ReadTimeout(e) + elif isinstance(e, _InvalidHeader): + raise InvalidHeader(e) + else: + raise if low_resp is None: break @@ -2303,6 +2407,40 @@ async def gather( ) except ValueError: low_resp = None + except (ProtocolError, OSError) as err: + raise ConnectionError(err) + + except MaxRetryError as e: + if isinstance(e.reason, ConnectTimeoutError): + # TODO: Remove this in 3.0.0: see #2811 + if not isinstance(e.reason, NewConnectionError): + raise ConnectTimeout(e) + + if isinstance(e.reason, ResponseError): + raise RetryError(e) + + if isinstance(e.reason, _ProxyError): + raise ProxyError(e) + + if isinstance(e.reason, _SSLError): + # This branch is for urllib3 v1.22 and later. + raise SSLError(e) + + raise ConnectionError(e) + + except _ProxyError as e: + raise ProxyError(e) + + except (_SSLError, _HTTPError) as e: + if isinstance(e, _SSLError): + # This branch is for urllib3 versions earlier than v1.22 + raise SSLError(e) + elif isinstance(e, ReadTimeoutError): + raise ReadTimeout(e) + elif isinstance(e, _InvalidHeader): + raise InvalidHeader(e) + else: + raise if low_resp is None: raise MultiplexingError( diff --git a/src/niquests/models.py b/src/niquests/models.py index cc04286630..1d04e86d3d 100644 --- a/src/niquests/models.py +++ b/src/niquests/models.py @@ -1136,7 +1136,7 @@ def __repr__(self) -> str: or self.request.conn_info is None or self.request.conn_info.http_version is None ): - return "" + return f"" # HTTP/2.0 is not preferred, cast it to HTTP/2 instead. http_revision = self.request.conn_info.http_version.value.replace(".0", "")