fix: sanitize newline characters in NO_PROXY env var before httpx reads it#3412
fix: sanitize newline characters in NO_PROXY env var before httpx reads it#3412arinsoni wants to merge 2 commits into
Conversation
Docker environments and .env files sometimes contain NO_PROXY values with newline-separated entries (e.g. `localhost\n192.168.1.1`). httpx splits only on commas, so the newline stays embedded in the hostname pattern and triggers `httpx.InvalidURL: Invalid non-printable ASCII character in URL` when the client is initialized. Normalize NO_PROXY and no_proxy by replacing newlines with commas in both _DefaultHttpxClient and _DefaultAsyncHttpxClient before delegating to httpx. Closes openai#3303 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9885a5dcc
ℹ️ 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".
| kwargs.setdefault("timeout", DEFAULT_TIMEOUT) | ||
| kwargs.setdefault("limits", DEFAULT_CONNECTION_LIMITS) | ||
| kwargs.setdefault("follow_redirects", True) | ||
| _sanitize_no_proxy_env_vars() |
There was a problem hiding this comment.
When callers construct DefaultHttpxClient(trust_env=False), httpx would not read proxy environment variables, so a newline in NO_PROXY would not block client creation. This unconditional call still rewrites os.environ for the entire process, changing later code that reads NO_PROXY/no_proxy despite the caller explicitly opting out of environment proxy handling; the async client has the same unconditional call. Gate the sanitization on env proxy handling actually being enabled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the follow-up commit — sanitization is now gated on kwargs.get("trust_env", True) in both _DefaultHttpxClient and _DefaultAsyncHttpxClient. Added tests covering the trust_env=False case to confirm
os.environ is not mutated when env proxies are disabled.
When DefaultHttpxClient/DefaultAsyncHttpxClient is constructed with trust_env=False, httpx never reads proxy environment variables, so mutating NO_PROXY/no_proxy in os.environ would be an unnecessary side-effect on the rest of the process. Gate the sanitization call on trust_env being enabled (the default). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Closes #3303
Docker environments and
.envfiles sometimes containNO_PROXYvalues with newline-separated entries (e.g.NO_PROXY=localhost\n192.168.1.1). The stdlib'sgetproxies()returns the raw value including the newline; httpx then splits only on commas, leaving the newline embedded in the hostname pattern. When httpx initializes its URL structures, it rejects the newline as an invalid non-printable ASCII character, raisinghttpx.InvalidURLbefore the client can be used at all.The fix is in
_sanitize_no_proxy_env_vars(), called from both_DefaultHttpxClient.__init__and_DefaultAsyncHttpxClient.__init__before delegating to httpx. It replaces any newlines inNO_PROXY/no_proxywith commas and strips empty entries, normalizing the value in-place so httpx always sees a well-formed string. The sanitization is idempotent — clean values pass through unchanged.Unit tests added for both sync and async clients using
pytest'smonkeypatchto inject a newline-containingNO_PROXYvalue and assert it is normalized and the client initializes without error.This contribution was made with AI-agent assistance (Claude Code).