fix(client): sanitize newlines in proxy env vars before httpx sees them - #3631
fix(client): sanitize newlines in proxy env vars before httpx sees them#3631Xsidz wants to merge 1 commit into
Conversation
Fixes openai#3303: NO_PROXY (and other proxy env vars) can contain newline characters in Docker/dotenv environments. httpx splits NO_PROXY only by comma, so a newline becomes part of the hostname and triggers InvalidURL. Replace newlines with commas at client construction time.
![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: 9c2fe85791
ℹ️ 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".
|
|
||
| WORKLOAD_IDENTITY_API_KEY_PLACEHOLDER = "workload-identity-auth" | ||
|
|
||
| _PROXY_ENV_VARS = ("HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy") |
There was a problem hiding this comment.
When ALL_PROXY or all_proxy is the configured fallback proxy and contains a trailing or embedded newline, it remains untouched and HTTPX2 still receives the invalid character during default client construction. The existing proxy tests explicitly clear both spellings because HTTPX2 recognizes them, so include them alongside the other supported proxy environment variables.
Useful? React with 👍 / 👎.
| parsed[line[:colon].strip()] = line[colon + 1 :].strip() | ||
| default_headers = {**parsed, **(default_headers if is_mapping_t(default_headers) else {})} | ||
|
|
||
| _sanitize_proxy_env_vars() |
There was a problem hiding this comment.
When callers follow the documented OpenAI(http_client=DefaultHttpx2Client(...)) pattern, Python constructs the HTTPX2 client before entering OpenAI.__init__, so a malformed listed proxy variable raises InvalidURL before this call can sanitize it. The same ordering affects DefaultAsyncHttpx2Client; sanitization therefore also needs to occur in the exported client helpers or otherwise before those clients consume the environment.
Useful? React with 👍 / 👎.
| if val is not None and "\n" in val: | ||
| os.environ[key] = ",".join(p.strip() for p in val.splitlines() if p.strip()) |
There was a problem hiding this comment.
When a proxy value contains a lone \r, this guard skips sanitization even though splitlines() would remove it and HTTPX2 rejects it as a non-printable ASCII character. This can occur when shell command substitution reads a CRLF-terminated file and strips the final \n while retaining \r, so the check should recognize carriage returns as well as line feeds.
Useful? React with 👍 / 👎.
Summary
Fixes #3303.
Docker,
.envfiles, and some shell scripts can produce proxy env vars with embedded newlines (e.g.NO_PROXY=localhost\n192.168.1.1). httpx splitsNO_PROXYonly on,, so the newline character becomes part of a hostname string, which triggershttpx.InvalidURL: Invalid non-printable ASCII character in URL.A fix in httpx upstream is blocked (project not accepting external PRs). The fix here sanitizes
HTTP_PROXY,HTTPS_PROXY,NO_PROXY, and their lowercase equivalents inos.environbefore the httpx client is constructed — both inOpenAI.__init__andAsyncOpenAI.__init__.Test plan
os.environ['NO_PROXY'] = 'localhost\n192.168.1.1'thenOpenAI(api_key='...')no longer raisesInvalidURL; env var is cleaned to'localhost,192.168.1.1'.tests/test_client.py: 198 passed, 2 skipped, 0 failures.