Skip to content

Commit

Permalink
🐛 fix exception leak from urllib3-future when resolving lazy responses (
Browse files Browse the repository at this point in the history 
#163)

fix #162
  • Loading branch information
@Ousret
Ousret authored Oct 22, 2024
1 parent 1933840 commit 5dd7d8f
Show file tree
Hide file tree
Showing 6 changed files with 189 additions and 43 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
6 changes: 6 additions & 0 deletions HISTORY.md
View file
Original file line number Diff line number Diff line change
@@ -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)
------------------

Expand Down
76 changes: 39 additions & 37 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,41 +12,41 @@ Niquests, is the “**Safest**, **Fastest[^10]**, **Easiest**, and **Most advanc
<details>
<summary>👆 <b>Look at the feature table comparison</b> against <i>requests, httpx and aiohttp</i>!</summary>

| 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 |
</details>

<details>
Expand All @@ -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?
Expand All @@ -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
<ResponsePromise HTTP/3>
>>> r.status_code
200
>>> r.headers['content-type']
Expand Down
4 changes: 2 additions & 2 deletions src/niquests/__version__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
142 changes: 140 additions & 2 deletions src/niquests/adapters.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/niquests/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<Response Dummy>"
return f"<Response Dummy [{self.status_code}]>"

# HTTP/2.0 is not preferred, cast it to HTTP/2 instead.
http_revision = self.request.conn_info.http_version.value.replace(".0", "")
Expand Down

0 comments on commit 5dd7d8f

Please sign in to comment.