mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
fix(custom_httpx): propagate ssl_verify through aiohttp handler + retry path
Fixes #30778 (and the related Bug 2: AsyncHTTPHandler retry drops ssl_verify). Two related bugs cause `ssl_verify: false` to be silently ignored: 1. `BaseLLMAIOHTTPHandler` never accepted `ssl_verify` — its constructor had no such parameter, `_get_or_create_transport()` called `AsyncHTTPHandler._create_aiohttp_transport()` without SSL args, and `_make_common_async_call()` called `client_session.post()` without an `ssl=` kwarg. As a result, aiohttp applied a default SSL context even on plain http:// URLs (visible as the `ssl:<ssl.SSLContext>` suffix in OllamaException messages). 2. `AsyncHTTPHandler` retry path dropped `ssl_verify` — on `ConnectError`/`RemoteProtocolError`, the post/put/patch/delete methods called `self.create_client(timeout=..., event_hooks=...)` *without* forwarding ssl_verify. The default (`None`) resolves to `True` via `get_ssl_configuration`, so the retry attempt had SSL verification re-enabled regardless of the original setting. Fix: - Add `ssl_verify` param to `BaseLLMAIOHTTPHandler.__init__` and pass it through to `_create_aiohttp_transport` so the lazily-created `LiteLLMAiohttpTransport` honors the caller's SSL setting per-request. - Store `ssl_verify` as `self._ssl_verify` on `AsyncHTTPHandler` and forward it to the retry `create_client` call in all 4 retry blocks (post/put/patch/delete). Tests (2 new): - `test_base_llm_aiohttp_handler_accepts_ssl_verify` — verifies the constructor stores the value and the lazy transport passes it to `_create_aiohttp_transport`. - `test_async_handler_retry_forwards_ssl_verify` — patches `create_client` on an `AsyncHTTPHandler(ssl_verify=False)` instance and forces a `ConnectError` on the first send so the retry branch runs; asserts the captured ssl_verify is `False`, not the default. All 41 existing tests in `test_http_handler.py` still pass (the 1 unrelated failure on `test_default_user_agent_is_litellm_version` is a pre-existing environment issue: missing `importlib_metadata`).
This commit is contained in:
parent
17b88719a2
commit
14024df9a7
3 changed files with 103 additions and 5 deletions
|
|
@ -38,6 +38,7 @@ class BaseLLMAIOHTTPHandler:
|
|||
client_session: Optional[aiohttp.ClientSession] = None,
|
||||
transport: Optional[LiteLLMAiohttpTransport] = None,
|
||||
connector: Optional[aiohttp.BaseConnector] = None,
|
||||
ssl_verify: Optional[Union[bool, str]] = None,
|
||||
):
|
||||
self.client_session = client_session
|
||||
self._owns_session = (
|
||||
|
|
@ -54,6 +55,11 @@ class BaseLLMAIOHTTPHandler:
|
|||
connector is None
|
||||
) # Track if we own the connector for cleanup
|
||||
|
||||
# Stored so a lazily-created transport / per-request ssl= kwarg can
|
||||
# honor the caller's SSL setting. Without this, aiohttp attaches a
|
||||
# default SSL context even on plain http:// URLs (see #30778).
|
||||
self.ssl_verify = ssl_verify
|
||||
|
||||
def _get_or_create_transport(self) -> Optional[LiteLLMAiohttpTransport]:
|
||||
"""Get existing transport or create a new one if needed."""
|
||||
if self.transport:
|
||||
|
|
@ -61,7 +67,9 @@ class BaseLLMAIOHTTPHandler:
|
|||
|
||||
# Create a transport using AsyncHTTPHandler's logic
|
||||
try:
|
||||
self.transport = AsyncHTTPHandler._create_aiohttp_transport()
|
||||
self.transport = AsyncHTTPHandler._create_aiohttp_transport(
|
||||
ssl_verify=self.ssl_verify,
|
||||
)
|
||||
self._owns_transport = True
|
||||
return self.transport
|
||||
except Exception:
|
||||
|
|
|
|||
|
|
@ -527,6 +527,10 @@ class AsyncHTTPHandler:
|
|||
):
|
||||
self.timeout = timeout
|
||||
self.event_hooks = event_hooks
|
||||
# Stored for the ConnectError/RemoteProtocolError retry path so the
|
||||
# retry attempt keeps the caller's ssl_verify (False to disable) rather
|
||||
# than silently re-enabling SSL verification. See #30778.
|
||||
self._ssl_verify = ssl_verify
|
||||
self.client = self.create_client(
|
||||
timeout=timeout,
|
||||
event_hooks=event_hooks,
|
||||
|
|
@ -649,7 +653,8 @@ class AsyncHTTPHandler:
|
|||
except (httpx.RemoteProtocolError, httpx.ConnectError):
|
||||
# Retry the request with a new session if there is a connection error
|
||||
new_client = self.create_client(
|
||||
timeout=timeout, event_hooks=self.event_hooks
|
||||
timeout=timeout, event_hooks=self.event_hooks,
|
||||
ssl_verify=self._ssl_verify,
|
||||
)
|
||||
try:
|
||||
return await self.single_connection_post_request(
|
||||
|
|
@ -712,7 +717,8 @@ class AsyncHTTPHandler:
|
|||
except (httpx.RemoteProtocolError, httpx.ConnectError):
|
||||
# Retry the request with a new session if there is a connection error
|
||||
new_client = self.create_client(
|
||||
timeout=timeout, event_hooks=self.event_hooks
|
||||
timeout=timeout, event_hooks=self.event_hooks,
|
||||
ssl_verify=self._ssl_verify,
|
||||
)
|
||||
try:
|
||||
return await self.single_connection_post_request(
|
||||
|
|
@ -773,7 +779,8 @@ class AsyncHTTPHandler:
|
|||
except (httpx.RemoteProtocolError, httpx.ConnectError):
|
||||
# Retry the request with a new session if there is a connection error
|
||||
new_client = self.create_client(
|
||||
timeout=timeout, event_hooks=self.event_hooks
|
||||
timeout=timeout, event_hooks=self.event_hooks,
|
||||
ssl_verify=self._ssl_verify,
|
||||
)
|
||||
try:
|
||||
return await self.single_connection_post_request(
|
||||
|
|
@ -834,7 +841,8 @@ class AsyncHTTPHandler:
|
|||
except (httpx.RemoteProtocolError, httpx.ConnectError):
|
||||
# Retry the request with a new session if there is a connection error
|
||||
new_client = self.create_client(
|
||||
timeout=timeout, event_hooks=self.event_hooks
|
||||
timeout=timeout, event_hooks=self.event_hooks,
|
||||
ssl_verify=self._ssl_verify,
|
||||
)
|
||||
try:
|
||||
return await self.single_connection_post_request(
|
||||
|
|
|
|||
|
|
@ -851,3 +851,85 @@ async def test_async_get_forwards_per_request_timeout():
|
|||
}
|
||||
finally:
|
||||
await handler.close()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_base_llm_aiohttp_handler_accepts_ssl_verify():
|
||||
"""
|
||||
Regression for #30778: ``BaseLLMAIOHTTPHandler.__init__`` must accept an
|
||||
``ssl_verify`` argument and forward it to the lazily-created
|
||||
``AsyncHTTPHandler._create_aiohttp_transport`` so the transport picks up
|
||||
the caller's SSL setting (False to disable, etc.) instead of inheriting
|
||||
aiohttp's default SSL context on plain http:// URLs.
|
||||
"""
|
||||
from litellm.llms.custom_httpx.aiohttp_handler import BaseLLMAIOHTTPHandler
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_create_aiohttp_transport(ssl_verify=None, **kwargs):
|
||||
captured["ssl_verify"] = ssl_verify
|
||||
return MagicMock()
|
||||
|
||||
original = AsyncHTTPHandler._create_aiohttp_transport
|
||||
AsyncHTTPHandler._create_aiohttp_transport = staticmethod(
|
||||
fake_create_aiohttp_transport
|
||||
)
|
||||
try:
|
||||
handler = BaseLLMAIOHTTPHandler(ssl_verify=False)
|
||||
# The constructor should have stored the value.
|
||||
assert handler.ssl_verify is False
|
||||
# Force lazy transport creation.
|
||||
handler._get_or_create_transport()
|
||||
assert captured["ssl_verify"] is False
|
||||
finally:
|
||||
AsyncHTTPHandler._create_aiohttp_transport = original
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_async_handler_retry_forwards_ssl_verify():
|
||||
"""
|
||||
Regression for #30778: the ``ConnectError``/``RemoteProtocolError`` retry
|
||||
path on ``AsyncHTTPHandler.post/put/patch/delete`` must forward
|
||||
``ssl_verify`` to the new ``create_client`` call. Previously the retry
|
||||
silently re-enabled SSL verification (the default) and broke callers
|
||||
relying on ``ssl_verify=False`` for plain-http services like Ollama.
|
||||
"""
|
||||
handler = AsyncHTTPHandler(ssl_verify=False)
|
||||
# Sanity: the stored value matches what was passed in.
|
||||
assert handler._ssl_verify is False
|
||||
|
||||
captured_ssl_verify = {}
|
||||
|
||||
def fake_create_client(*, timeout, event_hooks, ssl_verify=None, shared_session=None):
|
||||
captured_ssl_verify["value"] = ssl_verify
|
||||
# Return a real httpx client with a MockTransport so the post() body
|
||||
# can still execute the retry path without touching the network.
|
||||
return httpx.AsyncClient(
|
||||
transport=httpx.MockTransport(
|
||||
lambda req: httpx.Response(200, request=req, json={"ok": True})
|
||||
)
|
||||
)
|
||||
|
||||
handler.create_client = fake_create_client # type: ignore[assignment]
|
||||
|
||||
# Force the first send to raise ConnectError so we hit the retry branch.
|
||||
call_count = {"n": 0}
|
||||
|
||||
def maybe_fail(req):
|
||||
call_count["n"] += 1
|
||||
if call_count["n"] == 1:
|
||||
raise httpx.ConnectError("boom", request=req)
|
||||
return httpx.Response(200, request=req, json={"ok": True})
|
||||
|
||||
handler.client = httpx.AsyncClient(transport=httpx.MockTransport(maybe_fail))
|
||||
try:
|
||||
await handler.post(
|
||||
"http://example.invalid/post",
|
||||
json={"x": 1},
|
||||
)
|
||||
# The retry path should have invoked create_client with the stored
|
||||
# ssl_verify, NOT the default None/True.
|
||||
assert captured_ssl_verify["value"] is False
|
||||
finally:
|
||||
await handler.client.aclose()
|
||||
await handler.close()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue