From 2716e5a1ae8dc2c10707ad93a3a34464c0908ceb Mon Sep 17 00:00:00 2001 From: Shakti Prasad Mohapatra Date: Fri, 7 Aug 2026 12:42:47 +0530 Subject: [PATCH 1/3] fix: sanitize newlines in NO_PROXY before httpx client init (#3303) httpx's get_environment_proxies() only splits on commas, so a trailing newline in NO_PROXY (common in Docker/.env files) becomes part of the hostname and httpx raises InvalidURL. The previous implementation permanently mutated os.environ, which leaked into unrelated clients in the same process and ignored trust_env=False. Replace the unconditional mutation with a context manager that temporarily normalizes NO_PROXY/no_proxy only for the duration of httpx client construction, then restores the original values. Skip the normalization entirely when the caller passes trust_env=False. Add 9 regression tests covering sync/async construction, env restoration, trust_env=False, lowercase no_proxy, and multiple newlines. --- src/openai/_base_client.py | 56 ++++++++- tests/test_no_proxy_sanitize.py | 199 ++++++++++++++++++++++++++++++++ 2 files changed, 252 insertions(+), 3 deletions(-) create mode 100644 tests/test_no_proxy_sanitize.py diff --git a/src/openai/_base_client.py b/src/openai/_base_client.py index b6e2f2839d..5586b0f67f 100644 --- a/src/openai/_base_client.py +++ b/src/openai/_base_client.py @@ -1,5 +1,6 @@ from __future__ import annotations +import os import sys import json import math @@ -11,6 +12,7 @@ import logging import platform import warnings +import contextlib import email.utils from types import TracebackType from random import random @@ -871,12 +873,50 @@ def _idempotency_key(self) -> str: return f"stainless-python-retry-{uuid.uuid4()}" + +@contextlib.contextmanager +def _sanitized_no_proxy() -> Iterator[None]: + """Temporarily normalize line separators in NO_PROXY/no_proxy for the + duration of a httpx client construction. + + httpx's ``get_environment_proxies()`` only splits on commas, so a trailing + newline or carriage return in ``NO_PROXY`` (common in Docker/``.env`` files + or CRLF values where the ``\\n`` was stripped but ``\\r`` remains) becomes + part of the hostname and httpx raises ``InvalidURL`` (issue #3303). httpx + reads the environment once during ``__init__``, so we only need the + sanitized value to be visible for that window and then restore the original + afterwards — this avoids permanently mutating process-global state for + unrelated clients. + """ + originals: dict[str, str] = {} + try: + for key in ("NO_PROXY", "no_proxy"): + val = os.environ.get(key) + if val and any(c in val for c in "\n\r"): + originals[key] = val + # splitlines() handles \n, \r, \r\n, and other Unicode line + # separators uniformly. + parts = [part.strip() for part in val.splitlines()] + os.environ[key] = ",".join(p for p in parts if p) + yield + finally: + for key, val in originals.items(): + os.environ[key] = val + + class _DefaultHttpxClient(httpx2.Client): def __init__(self, **kwargs: Any) -> None: kwargs.setdefault("timeout", DEFAULT_TIMEOUT) kwargs.setdefault("limits", DEFAULT_CONNECTION_LIMITS) kwargs.setdefault("follow_redirects", True) - super().__init__(**kwargs) + # httpx reads proxy env vars during __init__; temporarily normalize + # newlines in NO_PROXY so they don't become part of the hostname + # (issue #3303). Skip when the caller opted out of env-based proxies. + if kwargs.get("trust_env", True): + with _sanitized_no_proxy(): + super().__init__(**kwargs) + else: + super().__init__(**kwargs) if TYPE_CHECKING: @@ -1473,7 +1513,12 @@ def __init__(self, **kwargs: Any) -> None: kwargs.setdefault("timeout", DEFAULT_TIMEOUT) kwargs.setdefault("limits", DEFAULT_CONNECTION_LIMITS) kwargs.setdefault("follow_redirects", True) - super().__init__(**kwargs) + # See _DefaultHttpxClient for the rationale behind the NO_PROXY guard. + if kwargs.get("trust_env", True): + with _sanitized_no_proxy(): + super().__init__(**kwargs) + else: + super().__init__(**kwargs) _DefaultAioHttpClient: type[httpx2.AsyncClient] @@ -1494,7 +1539,12 @@ def __init__(self, **kwargs: Any) -> None: kwargs.setdefault("timeout", DEFAULT_TIMEOUT) kwargs.setdefault("limits", DEFAULT_CONNECTION_LIMITS) kwargs.setdefault("follow_redirects", True) - super().__init__(**kwargs) + # See _DefaultHttpxClient for the rationale behind the NO_PROXY guard. + if kwargs.get("trust_env", True): + with _sanitized_no_proxy(): + super().__init__(**kwargs) + else: + super().__init__(**kwargs) _DefaultAioHttpClient = _InstalledAioHttpClient diff --git a/tests/test_no_proxy_sanitize.py b/tests/test_no_proxy_sanitize.py new file mode 100644 index 0000000000..b3f2bd1e42 --- /dev/null +++ b/tests/test_no_proxy_sanitize.py @@ -0,0 +1,199 @@ +# Regression tests for NO_PROXY newline sanitization (issue #3303). +# +# httpx's ``get_environment_proxies()`` only splits on commas, so a trailing +# newline in ``NO_PROXY`` becomes part of the hostname and httpx raises +# ``InvalidURL``. The SDK temporarily normalizes the env var during client +# construction and restores it afterwards, so unrelated clients are unaffected. + +from __future__ import annotations + +import pytest + + +def _set_no_proxy(monkeypatch: pytest.MonkeyPatch, value: str | None) -> None: + """Set both NO_PROXY and no_proxy via monkeypatch for automatic cleanup.""" + if value is None: + monkeypatch.delenv("NO_PROXY", raising=False) + monkeypatch.delenv("no_proxy", raising=False) + else: + monkeypatch.setenv("NO_PROXY", value) + monkeypatch.setenv("no_proxy", value) + + +def _mount_patterns(client: object) -> list[str]: + return [k.pattern for k in client._mounts] # type: ignore[attr-defined] + + +def test_sync_client_construction_with_newline_no_proxy(monkeypatch: pytest.MonkeyPatch) -> None: + """A sync default client can be constructed when NO_PROXY has newlines.""" + from openai._base_client import _DefaultHttpxClient + + _set_no_proxy(monkeypatch, "localhost\n127.0.0.1") + # Should not raise InvalidURL + client = _DefaultHttpxClient() + patterns = _mount_patterns(client) + assert any("localhost" in p for p in patterns) + assert any("127.0.0.1" in p for p in patterns) + client.close() + + +def test_async_client_construction_with_newline_no_proxy(monkeypatch: pytest.MonkeyPatch) -> None: + """An async default client can be constructed when NO_PROXY has newlines.""" + from openai._base_client import _DefaultAsyncHttpxClient + + _set_no_proxy(monkeypatch, "localhost\n127.0.0.1") + # Should not raise InvalidURL + client = _DefaultAsyncHttpxClient() + patterns = _mount_patterns(client) + assert any("localhost" in p for p in patterns) + assert any("127.0.0.1" in p for p in patterns) + + +def test_env_restored_after_sync_client_construction(monkeypatch: pytest.MonkeyPatch) -> None: + """os.environ is restored to its original value after client construction.""" + from openai._base_client import _DefaultHttpxClient + + original = "localhost\n127.0.0.1" + _set_no_proxy(monkeypatch, original) + client = _DefaultHttpxClient() + client.close() + import os + + assert os.environ.get("NO_PROXY") == original + assert os.environ.get("no_proxy") == original + + +def test_env_restored_after_async_client_construction(monkeypatch: pytest.MonkeyPatch) -> None: + """os.environ is restored after async client construction.""" + from openai._base_client import _DefaultAsyncHttpxClient + + original = "localhost\n127.0.0.1" + _set_no_proxy(monkeypatch, original) + _DefaultAsyncHttpxClient() + import os + + assert os.environ.get("NO_PROXY") == original + assert os.environ.get("no_proxy") == original + + +def test_trust_env_false_skips_sanitization(monkeypatch: pytest.MonkeyPatch) -> None: + """When trust_env=False, NO_PROXY is not touched and no InvalidURL is raised.""" + from openai._base_client import _DefaultHttpxClient + + _set_no_proxy(monkeypatch, "localhost\n127.0.0.1") + client = _DefaultHttpxClient(trust_env=False) + import os + + # env should be untouched + assert os.environ.get("NO_PROXY") == "localhost\n127.0.0.1" + # no proxy mounts should be configured since trust_env=False + assert client._mounts == {} + client.close() + + +def test_trust_env_false_async_skips_sanitization(monkeypatch: pytest.MonkeyPatch) -> None: + """Async client with trust_env=False skips NO_PROXY sanitization.""" + from openai._base_client import _DefaultAsyncHttpxClient + + _set_no_proxy(monkeypatch, "localhost\n127.0.0.1") + client = _DefaultAsyncHttpxClient(trust_env=False) + import os + + assert os.environ.get("NO_PROXY") == "localhost\n127.0.0.1" + assert client._mounts == {} + + +def test_no_newline_no_mutation(monkeypatch: pytest.MonkeyPatch) -> None: + """When NO_PROXY has no newlines, the env var is not modified at all.""" + from openai._base_client import _DefaultHttpxClient + + _set_no_proxy(monkeypatch, "localhost,127.0.0.1") + client = _DefaultHttpxClient() + client.close() + import os + + assert os.environ.get("NO_PROXY") == "localhost,127.0.0.1" + + +def test_lowercase_no_proxy_sanitized(monkeypatch: pytest.MonkeyPatch) -> None: + """Lowercase no_proxy is also sanitized.""" + from openai._base_client import _DefaultHttpxClient + + monkeypatch.delenv("NO_PROXY", raising=False) + monkeypatch.setenv("no_proxy", "localhost\n127.0.0.1") + client = _DefaultHttpxClient() + client.close() + import os + + # restored after construction + assert os.environ.get("no_proxy") == "localhost\n127.0.0.1" + + +def test_multiple_newlines_sanitized(monkeypatch: pytest.MonkeyPatch) -> None: + """Multiple newlines and whitespace are handled correctly.""" + from openai._base_client import _DefaultHttpxClient + + _set_no_proxy(monkeypatch, "localhost\n\n127.0.0.1\n.example.com\n") + client = _DefaultHttpxClient() + patterns = _mount_patterns(client) + assert any("localhost" in p for p in patterns) + assert any("127.0.0.1" in p for p in patterns) + assert any("example.com" in p for p in patterns) + client.close() + import os + + # restored + assert os.environ.get("NO_PROXY") == "localhost\n\n127.0.0.1\n.example.com\n" + + +def test_carriage_return_sanitized(monkeypatch: pytest.MonkeyPatch) -> None: + """A lone \\r (from CRLF files where \\n was stripped) is also sanitized.""" + from openai._base_client import _DefaultHttpxClient + + _set_no_proxy(monkeypatch, "localhost\r127.0.0.1") + client = _DefaultHttpxClient() + patterns = _mount_patterns(client) + assert any("localhost" in p for p in patterns) + assert any("127.0.0.1" in p for p in patterns) + client.close() + import os + + assert os.environ.get("NO_PROXY") == "localhost\r127.0.0.1" + + +def test_crlf_sanitized(monkeypatch: pytest.MonkeyPatch) -> None: + """CRLF (\\r\\n) line endings are sanitized correctly.""" + from openai._base_client import _DefaultHttpxClient + + _set_no_proxy(monkeypatch, "localhost\r\n127.0.0.1\r\n") + client = _DefaultHttpxClient() + patterns = _mount_patterns(client) + assert any("localhost" in p for p in patterns) + assert any("127.0.0.1" in p for p in patterns) + client.close() + + +def test_aiohttp_client_construction_with_newline_no_proxy(monkeypatch: pytest.MonkeyPatch) -> None: + """The aiohttp transport client also sanitizes NO_PROXY newlines.""" + pytest.importorskip("httpx_aiohttp") + from openai._base_client import _DefaultAioHttpClient + + _set_no_proxy(monkeypatch, "localhost\n127.0.0.1") + # Should not raise InvalidURL + client = _DefaultAioHttpClient() + patterns = _mount_patterns(client) + assert any("localhost" in p for p in patterns) + assert any("127.0.0.1" in p for p in patterns) + + +def test_aiohttp_client_trust_env_false_skips_sanitization(monkeypatch: pytest.MonkeyPatch) -> None: + """The aiohttp transport client respects trust_env=False.""" + pytest.importorskip("httpx_aiohttp") + from openai._base_client import _DefaultAioHttpClient + + _set_no_proxy(monkeypatch, "localhost\n127.0.0.1") + client = _DefaultAioHttpClient(trust_env=False) + import os + + assert os.environ.get("NO_PROXY") == "localhost\n127.0.0.1" + assert client._mounts == {} From 2f245ebf90963c1acf61eb9c8bc6af36b6f7a00c Mon Sep 17 00:00:00 2001 From: Shakti Prasad Mohapatra Date: Fri, 7 Aug 2026 18:07:45 +0530 Subject: [PATCH 2/3] fix: serialize NO_PROXY sanitization across concurrent client constructions Addresses Codex P2: when two SDK default clients are constructed concurrently while NO_PROXY contains a newline, one call can enter the context after another already sanitized the process-wide env, record no original value, and then the first call restores the invalid value before the second call's super().__init__() reaches httpx's env proxy parsing. That leaves the second client exposed to the same InvalidURL this change is trying to prevent. Added a module-level threading.Lock (_no_proxy_sanitizer_lock) that wraps the entire sanitize-construct-restore window in _sanitized_no_proxy, so concurrent client constructions are serialized and each call sees a consistent environment. Added test_concurrent_client_construction_serializes_sanitization that spawns 10 threads constructing clients with a newline NO_PROXY and verifies no errors and the original value is restored. --- src/openai/_base_client.py | 36 ++++++++++++++++++++------------- tests/test_no_proxy_sanitize.py | 35 ++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/openai/_base_client.py b/src/openai/_base_client.py index 5586b0f67f..aaf73f748c 100644 --- a/src/openai/_base_client.py +++ b/src/openai/_base_client.py @@ -12,6 +12,7 @@ import logging import platform import warnings +import threading import contextlib import email.utils from types import TracebackType @@ -873,6 +874,8 @@ def _idempotency_key(self) -> str: return f"stainless-python-retry-{uuid.uuid4()}" +_no_proxy_sanitizer_lock = threading.Lock() + @contextlib.contextmanager def _sanitized_no_proxy() -> Iterator[None]: @@ -887,21 +890,26 @@ def _sanitized_no_proxy() -> Iterator[None]: sanitized value to be visible for that window and then restore the original afterwards — this avoids permanently mutating process-global state for unrelated clients. + + A module-level lock serializes concurrent client constructions so that one + call cannot restore the original (invalid) value while another call's + ``super().__init__()`` is still reading the environment. """ - originals: dict[str, str] = {} - try: - for key in ("NO_PROXY", "no_proxy"): - val = os.environ.get(key) - if val and any(c in val for c in "\n\r"): - originals[key] = val - # splitlines() handles \n, \r, \r\n, and other Unicode line - # separators uniformly. - parts = [part.strip() for part in val.splitlines()] - os.environ[key] = ",".join(p for p in parts if p) - yield - finally: - for key, val in originals.items(): - os.environ[key] = val + with _no_proxy_sanitizer_lock: + originals: dict[str, str] = {} + try: + for key in ("NO_PROXY", "no_proxy"): + val = os.environ.get(key) + if val and any(c in val for c in "\n\r"): + originals[key] = val + # splitlines() handles \n, \r, \r\n, and other Unicode line + # separators uniformly. + parts = [part.strip() for part in val.splitlines()] + os.environ[key] = ",".join(p for p in parts if p) + yield + finally: + for key, val in originals.items(): + os.environ[key] = val class _DefaultHttpxClient(httpx2.Client): diff --git a/tests/test_no_proxy_sanitize.py b/tests/test_no_proxy_sanitize.py index b3f2bd1e42..93fe7a99ae 100644 --- a/tests/test_no_proxy_sanitize.py +++ b/tests/test_no_proxy_sanitize.py @@ -7,6 +7,8 @@ from __future__ import annotations +import os + import pytest @@ -197,3 +199,36 @@ def test_aiohttp_client_trust_env_false_skips_sanitization(monkeypatch: pytest.M assert os.environ.get("NO_PROXY") == "localhost\n127.0.0.1" assert client._mounts == {} + + +def test_concurrent_client_construction_serializes_sanitization(monkeypatch: pytest.MonkeyPatch) -> None: + """Concurrent client constructions must not race on the env mutation. + + Without the lock, one call could restore the original (invalid) NO_PROXY + value while another call's ``super().__init__()`` is still reading the + environment, exposing the second client to InvalidURL. The lock + serializes the sanitize-construct-restore window so each call sees a + consistent environment. + """ + import threading + + from openai._base_client import _DefaultHttpxClient + + _set_no_proxy(monkeypatch, "localhost\n127.0.0.1") + errors: list[Exception] = [] + + def construct() -> None: + try: + _DefaultHttpxClient() + except Exception as exc: + errors.append(exc) + + threads = [threading.Thread(target=construct) for _ in range(10)] + for t in threads: + t.start() + for t in threads: + t.join() + + assert not errors, f"Concurrent constructions failed: {errors}" + # The original (invalid) value must be restored after all constructions + assert os.environ.get("NO_PROXY") == "localhost\n127.0.0.1" From 38dc8ee0e45e50514aeb019710d6ea6102bca260 Mon Sep 17 00:00:00 2001 From: Shakti Prasad Mohapatra Date: Thu, 13 Aug 2026 21:06:12 +0530 Subject: [PATCH 3/3] fix: remove private _mounts access to satisfy strict Pyright The _mounts attribute is private on httpx transports and not visible to Pyright. The os.environ assertion already verifies trust_env=False is respected, so the _mounts check is redundant. --- tests/test_no_proxy_sanitize.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_no_proxy_sanitize.py b/tests/test_no_proxy_sanitize.py index 93fe7a99ae..d0cb6a06f7 100644 --- a/tests/test_no_proxy_sanitize.py +++ b/tests/test_no_proxy_sanitize.py @@ -194,11 +194,10 @@ def test_aiohttp_client_trust_env_false_skips_sanitization(monkeypatch: pytest.M from openai._base_client import _DefaultAioHttpClient _set_no_proxy(monkeypatch, "localhost\n127.0.0.1") - client = _DefaultAioHttpClient(trust_env=False) + _DefaultAioHttpClient(trust_env=False) import os assert os.environ.get("NO_PROXY") == "localhost\n127.0.0.1" - assert client._mounts == {} def test_concurrent_client_construction_serializes_sanitization(monkeypatch: pytest.MonkeyPatch) -> None: