fix: sanitize NO_PROXY values before client setup#3468
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc70bb4583
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| with _sanitized_proxy_env(): | ||
| super().__init__(**kwargs) |
There was a problem hiding this comment.
Apply the proxy sanitizer to DefaultAioHttpClient too
When users opt into the aiohttp transport via the exported DefaultAioHttpClient, its initializer just below still calls httpx_aiohttp.HttpxAiohttpClient/httpx.AsyncClient.__init__ without this context. With NO_PROXY or no_proxy containing newlines, DefaultAioHttpClient() will therefore hit the same httpx environment-proxy parsing error that this patch fixes for DefaultAsyncHttpxClient; wrap that initializer as well so the optional async client behaves consistently.
Useful? React with 👍 / 👎.
|
Addressed in 935f6e2. I wrapped the default aiohttp client initializer in the same sanitized proxy env context so the async aiohttp transport now matches the default httpx transport behavior. |
Changes being requested
Fixes #3303.
Sanitize newline-delimited
NO_PROXY/no_proxyenvironment values while constructing the default sync and async HTTP clients, then restore the original environment values after initialization. Added regression coverage for both client types.Additional context & links
Validation:
.venv/bin/python -m pytest -o addopts= tests/test_client.py -k no_proxy_with_newlines_is_sanitized_for_default_http_client