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,7 @@
 import logging
 import platform
 import warnings
+import threading
 import email.utils
 from types import TracebackType
 from random import random
@@ -860,12 +862,72 @@ def _idempotency_key(self) -> str:
         return f"stainless-python-retry-{uuid.uuid4()}"
 
 
+_no_proxy_lock = threading.Lock()
+
+
+def _sanitize_no_proxy_env(*, trust_env: bool) -> None:
+    """Normalize line separators in ``NO_PROXY`` / ``no_proxy`` before httpx reads them.
+
+    httpx's ``get_environment_proxies()`` splits the value on commas only, so a
+    trailing newline or carriage return (common in Docker ``.env`` files or CRLF
+    values where ``\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 normalize
+    in-place before calling ``super().__init__()``.  The caller is responsible
+    for saving and restoring the original value via ``_save_no_proxy_env`` /
+    ``_restore_no_proxy_env`` so the mutation is temporary.
+
+    When ``trust_env=False`` is explicitly passed, httpx will not read proxy
+    environment variables at all, so we skip the sanitization entirely.
+    """
+    if not trust_env:
+        return
+
+    for var in ("NO_PROXY", "no_proxy"):
+        raw = os.environ.get(var)
+        if raw is None:
+            continue
+        sanitized = ",".join(part.strip() for part in raw.replace("\r", "\n").split("\n") if part.strip())
+        if sanitized != raw:
+            os.environ[var] = sanitized
+
+
+def _save_no_proxy_env() -> dict[str, str | None]:
+    """Snapshot the current NO_PROXY/no_proxy env vars for later restoration."""
+    return {var: os.environ.get(var) for var in ("NO_PROXY", "no_proxy")}
+
+
+def _restore_no_proxy_env(saved: dict[str, str | None]) -> None:
+    """Restore NO_PROXY/no_proxy env vars to their pre-sanitization values."""
+    for var, value in saved.items():
+        if value is None:
+            os.environ.pop(var, None)
+        else:
+            os.environ[var] = value
+
+
 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)
+        # Sanitize NO_PROXY before httpx reads the environment, but only when
+        # trust_env is not explicitly disabled.  The lock ensures concurrent
+        # client constructions don't see a partially sanitized value.
+        # The original env value is restored after init so the mutation is
+        # temporary and doesn't affect other code in the same process.
+        trust_env = kwargs.get("trust_env", True)
+        if trust_env:
+            with _no_proxy_lock:
+                _saved = _save_no_proxy_env()
+                try:
+                    _sanitize_no_proxy_env(trust_env=trust_env)
+                    super().__init__(**kwargs)
+                finally:
+                    _restore_no_proxy_env(_saved)
+        else:
+            super().__init__(**kwargs)
 
 
 if TYPE_CHECKING:
@@ -1459,7 +1521,17 @@ 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)
+        trust_env = kwargs.get("trust_env", True)
+        if trust_env:
+            with _no_proxy_lock:
+                _saved = _save_no_proxy_env()
+                try:
+                    _sanitize_no_proxy_env(trust_env=trust_env)
+                    super().__init__(**kwargs)
+                finally:
+                    _restore_no_proxy_env(_saved)
+        else:
+            super().__init__(**kwargs)
 
 
 _DefaultAioHttpClient: type[httpx2.AsyncClient]

tests/test_no_proxy_sanitization.py

@@ -0,0 +1,147 @@
+"""Tests for NO_PROXY env var sanitization during httpx client init.
+
+Regression tests for issue #3303: Docker/.env files can leave newline or
+carriage return characters in NO_PROXY, which httpx's comma-only splitter
+treats as part of the hostname, causing InvalidURL.
+"""
+
+from __future__ import annotations
+
+import os
+from typing import Iterator
+
+import pytest
+
+from openai._base_client import (
+    _save_no_proxy_env,
+    _DefaultHttpxClient,
+    _restore_no_proxy_env,
+    _sanitize_no_proxy_env,
+    _DefaultAsyncHttpxClient,
+)
+
+
+@pytest.fixture(autouse=True)
+def clean_no_proxy() -> Iterator[None]:
+    """Remove NO_PROXY/no_proxy before and after each test."""
+    for var in ("NO_PROXY", "no_proxy"):
+        os.environ.pop(var, None)
+    yield
+    for var in ("NO_PROXY", "no_proxy"):
+        os.environ.pop(var, None)
+
+
+class TestSanitizeNoProxyEnv:
+    def test_newlines_replaced_with_commas(self) -> None:
+        os.environ["NO_PROXY"] = "localhost\n127.0.0.1\n.example.com"
+        _sanitize_no_proxy_env(trust_env=True)
+        assert os.environ["NO_PROXY"] == "localhost,127.0.0.1,.example.com"
+
+    def test_carriage_returns_replaced(self) -> None:
+        os.environ["NO_PROXY"] = "localhost\r127.0.0.1"
+        _sanitize_no_proxy_env(trust_env=True)
+        assert os.environ["NO_PROXY"] == "localhost,127.0.0.1"
+
+    def test_crlf_handled(self) -> None:
+        os.environ["NO_PROXY"] = "localhost\r\n127.0.0.1\r\n"
+        _sanitize_no_proxy_env(trust_env=True)
+        assert os.environ["NO_PROXY"] == "localhost,127.0.0.1"
+
+    def test_trailing_newline_stripped(self) -> None:
+        os.environ["NO_PROXY"] = "localhost,127.0.0.1\n"
+        _sanitize_no_proxy_env(trust_env=True)
+        assert os.environ["NO_PROXY"] == "localhost,127.0.0.1"
+
+    def test_already_clean_value_unchanged(self) -> None:
+        os.environ["NO_PROXY"] = "localhost,127.0.0.1"
+        _sanitize_no_proxy_env(trust_env=True)
+        assert os.environ["NO_PROXY"] == "localhost,127.0.0.1"
+
+    def test_no_env_var_unchanged(self) -> None:
+        _sanitize_no_proxy_env(trust_env=True)
+        assert "NO_PROXY" not in os.environ
+        assert "no_proxy" not in os.environ
+
+    def test_lowercase_var_sanitized(self) -> None:
+        os.environ["no_proxy"] = "localhost\n127.0.0.1"
+        _sanitize_no_proxy_env(trust_env=True)
+        assert os.environ["no_proxy"] == "localhost,127.0.0.1"
+
+    def test_trust_env_false_skips_sanitization(self) -> None:
+        os.environ["NO_PROXY"] = "localhost\n127.0.0.1"
+        _sanitize_no_proxy_env(trust_env=False)
+        # Should remain unchanged when trust_env=False
+        assert os.environ["NO_PROXY"] == "localhost\n127.0.0.1"
+
+    def test_empty_lines_skipped(self) -> None:
+        os.environ["NO_PROXY"] = "localhost\n\n127.0.0.1\n"
+        _sanitize_no_proxy_env(trust_env=True)
+        assert os.environ["NO_PROXY"] == "localhost,127.0.0.1"
+
+    def test_whitespace_stripped(self) -> None:
+        os.environ["NO_PROXY"] = "  localhost  \n  127.0.0.1  "
+        _sanitize_no_proxy_env(trust_env=True)
+        assert os.environ["NO_PROXY"] == "localhost,127.0.0.1"
+
+
+class TestSaveRestoreNoProxyEnv:
+    def test_save_and_restore_unchanged(self) -> None:
+        os.environ["NO_PROXY"] = "localhost,127.0.0.1"
+        saved = _save_no_proxy_env()
+        _sanitize_no_proxy_env(trust_env=True)
+        _restore_no_proxy_env(saved)
+        assert os.environ["NO_PROXY"] == "localhost,127.0.0.1"
+
+    def test_save_and_restore_with_newlines(self) -> None:
+        os.environ["NO_PROXY"] = "localhost\n127.0.0.1"
+        saved = _save_no_proxy_env()
+        _sanitize_no_proxy_env(trust_env=True)
+        # After sanitization, the env is changed
+        assert os.environ["NO_PROXY"] == "localhost,127.0.0.1"
+        # After restore, the original value is back
+        _restore_no_proxy_env(saved)
+        assert os.environ["NO_PROXY"] == "localhost\n127.0.0.1"
+
+    def test_save_and_restore_missing_var(self) -> None:
+        saved = _save_no_proxy_env()
+        os.environ["NO_PROXY"] = "should-be-removed"
+        _restore_no_proxy_env(saved)
+        assert "NO_PROXY" not in os.environ
+
+
+class TestClientConstruction:
+    def test_sync_client_restores_env_after_init(self) -> None:
+        """The sync client should restore the original NO_PROXY value after init."""
+        original = "localhost\n127.0.0.1"
+        os.environ["NO_PROXY"] = original
+        client = _DefaultHttpxClient()
+        client.close()
+        # The original (unsanitized) value should be restored
+        assert os.environ["NO_PROXY"] == original
+
+    def test_async_client_restores_env_after_init(self) -> None:
+        """The async client should restore the original NO_PROXY value after init."""
+        original = "localhost\n127.0.0.1"
+        os.environ["NO_PROXY"] = original
+        client = _DefaultAsyncHttpxClient()
+        # Close the client to clean up
+        import asyncio
+
+        asyncio.run(client.aclose())
+        # The original (unsanitized) value should be restored
+        assert os.environ["NO_PROXY"] == original
+
+    def test_sync_client_with_trust_env_false_no_mutation(self) -> None:
+        """When trust_env=False, NO_PROXY should not be touched at all."""
+        original = "localhost\n127.0.0.1"
+        os.environ["NO_PROXY"] = original
+        client = _DefaultHttpxClient(trust_env=False)
+        client.close()
+        assert os.environ["NO_PROXY"] == original
+
+    def test_sync_client_with_clean_no_proxy(self) -> None:
+        """A clean NO_PROXY value should work fine with the sync client."""
+        os.environ["NO_PROXY"] = "localhost,127.0.0.1"
+        client = _DefaultHttpxClient()
+        client.close()
+        assert os.environ["NO_PROXY"] == "localhost,127.0.0.1"