fix: sanitize newlines in NO_PROXY env var before httpx client init

src/openai/_base_client.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import os
 import sys
 import json
 import math
@@ -11,6 +12,8 @@
 import logging
 import platform
 import warnings
+import threading
+import contextlib
 import email.utils
 from types import TracebackType
 from random import random
@@ -871,12 +874,57 @@ 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]:
+    """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.
+
+    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.
+    """
+    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):
     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 +1521,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 +1547,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
 

tests/test_no_proxy_sanitize.py

@@ -0,0 +1,233 @@
+# 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 os
+
+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")
+    _DefaultAioHttpClient(trust_env=False)
+    import os
+
+    assert os.environ.get("NO_PROXY") == "localhost\n127.0.0.1"
+
+
+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"