Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
ec5be86
fix(sglang): validate diffusion input_reference against traversal and…
nnshah1 Sep 8, 2026
4db60f8
fix(sglang): extend input_reference validation to video diffusion; sh…
nnshah1 Sep 8, 2026
92201a2
fix(common-http): percent-decode file:// media paths (review)
nnshah1 Sep 8, 2026
5070d4c
fix(sglang): fetch diffusion input_reference URLs SSRF-safe (review)
nnshah1 Sep 10, 2026
f7e48c0
Merge remote-tracking branch 'origin/main' into pr14435-fix
dmitry-tokarev-nv Sep 11, 2026
0debb20
fix(common-http): bound the media reference echoed into errors and logs
dmitry-tokarev-nv Sep 11, 2026
b716ec5
fix(common-http): let a caller cap how many bytes fetch_bytes will bu…
dmitry-tokarev-nv Sep 11, 2026
df50430
fix(common-http): make local_media_reference yield only a local path
dmitry-tokarev-nv Sep 11, 2026
1d215cd
fix(sglang): return 400 for a rejected diffusion input_reference
dmitry-tokarev-nv Sep 11, 2026
82aeb11
fix(common-http): bound the URL echoed by validate_url and the redire…
dmitry-tokarev-nv Sep 11, 2026
02346fe
fix(common-http): close four gaps from a second review pass
dmitry-tokarev-nv Sep 11, 2026
dbf9e1d
fix(common-http): bound HttpStatusError where the binding actually re…
dmitry-tokarev-nv Sep 11, 2026
4abc822
Merge remote-tracking branch 'origin/main' into pr14435-fix
dmitry-tokarev-nv Sep 11, 2026
6ffa332
fix(common-http): stop the HttpStatusError bound eating actionable gu…
dmitry-tokarev-nv Sep 11, 2026
ec106be
Merge remote-tracking branch 'origin/main' into pr14435-fix
dmitry-tokarev-nv Sep 14, 2026
1fad15c
Merge branch 'main' into pr14435-fix
dmitry-tokarev-nv Sep 15, 2026
3f7e3db
fix(common-http): bound the OSError diagnostic and the data-URI media…
dmitry-tokarev-nv Sep 15, 2026
3230408
fix(common-http): name aiohttp, not the removed backend, in two comments
dmitry-tokarev-nv Sep 16, 2026
75c1a6d
feat(common-http): make the media download cap operator-tunable
dmitry-tokarev-nv Sep 16, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions components/src/dynamo/common/http/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,11 @@ def get_default_client() -> HttpClient:
return _default


async def fetch_bytes(url, timeout, *, policy=None) -> bytes:
async def fetch_bytes(url, timeout, *, policy=None, max_bytes=None) -> bytes:
"""Singleton-backed convenience wrapper over :meth:`HttpClient.fetch_bytes`."""
return await get_default_client().fetch_bytes(url, timeout, policy=policy)
return await get_default_client().fetch_bytes(
url, timeout, policy=policy, max_bytes=max_bytes
)


async def close_http_client() -> None:
Expand Down
61 changes: 49 additions & 12 deletions components/src/dynamo/common/http/aiohttp_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,24 @@
import aiohttp
from yarl import URL

from .base import HttpClient, HttpConnectionError, HttpStatusError, HttpTimeoutError
from .base import (
HttpClient,
HttpConnectionError,
HttpStatusError,
HttpTimeoutError,
collect_capped,
)
from .url_validator import describe_error_detail, describe_media_source

logger = logging.getLogger(__name__)


_REDIRECT_STATUSES = frozenset({301, 302, 303, 307, 308})

# Read granularity for the capped reader. Chunks are joined, so this only
# bounds how far past the limit a single read can carry.
_READ_CHUNK = 64 * 1024


class AiohttpClient(HttpClient):
"""aiohttp-backed concrete client."""
Expand Down Expand Up @@ -72,30 +83,40 @@ async def _get_session(self) -> aiohttp.ClientSession:
)
return self._session

async def _fetch_simple(self, url: str, timeout: float) -> bytes:
async def _fetch_simple(
self, url: str, timeout: float, *, max_bytes: Optional[int] = None
) -> bytes:
session = await self._get_session()
client_timeout = self._effective_timeout(timeout)
try:
async with session.get(
url, timeout=client_timeout, allow_redirects=True
) as response:
response.raise_for_status()
return await response.read()
return await collect_capped(
response.content.iter_chunked(_READ_CHUNK), url, max_bytes
)
except aiohttp.ClientResponseError as e:
raise HttpStatusError(e.status, e.message or "", url) from e
except (asyncio.TimeoutError, aiohttp.ServerTimeoutError) as e:
raise HttpTimeoutError(f"Timeout loading {url}") from e
raise HttpTimeoutError(
f"Timeout loading {describe_media_source(url)}"
) from e
except (
aiohttp.ClientConnectionError,
aiohttp.ClientConnectorError,
aiohttp.ServerDisconnectedError,
) as e:
raise HttpConnectionError(f"Connection error loading {url}: {e}") from e
raise HttpConnectionError(
f"Connection error loading {describe_media_source(url)}: {describe_error_detail(str(e))}"
) from e
except aiohttp.ClientError as e:
raise HttpConnectionError(f"HTTP error loading {url}: {e}") from e
raise HttpConnectionError(
f"HTTP error loading {describe_media_source(url)}: {describe_error_detail(str(e))}"
) from e

async def _fetch_body_or_redirect(
self, url: str, timeout: float
self, url: str, timeout: float, *, max_bytes: Optional[int] = None
) -> tuple[bytes | None, str | None]:
session = await self._get_session()
client_timeout = self._effective_timeout(timeout)
Expand All @@ -108,23 +129,39 @@ async def _fetch_body_or_redirect(
if location:
next_url = str(response.url.join(URL(location)))
return None, next_url
return await response.read(), None
return (
await collect_capped(
response.content.iter_chunked(_READ_CHUNK), url, max_bytes
),
None,
)

try:
response.raise_for_status()
except aiohttp.ClientResponseError as e:
raise HttpStatusError(e.status, e.message or "", url) from e
return await response.read(), None
return (
await collect_capped(
response.content.iter_chunked(_READ_CHUNK), url, max_bytes
),
None,
)
except (asyncio.TimeoutError, aiohttp.ServerTimeoutError) as e:
raise HttpTimeoutError(f"Timeout loading {url}") from e
raise HttpTimeoutError(
f"Timeout loading {describe_media_source(url)}"
) from e
except (
aiohttp.ClientConnectionError,
aiohttp.ClientConnectorError,
aiohttp.ServerDisconnectedError,
) as e:
raise HttpConnectionError(f"Connection error loading {url}: {e}") from e
raise HttpConnectionError(
f"Connection error loading {describe_media_source(url)}: {describe_error_detail(str(e))}"
) from e
except aiohttp.ClientError as e:
raise HttpConnectionError(f"HTTP error loading {url}: {e}") from e
raise HttpConnectionError(
f"HTTP error loading {describe_media_source(url)}: {describe_error_detail(str(e))}"
) from e

async def close(self) -> None:
async with self._lock:
Expand Down
78 changes: 69 additions & 9 deletions components/src/dynamo/common/http/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,27 @@

import abc
import asyncio
from typing import Optional
from typing import AsyncIterator, Optional

from dynamo.common.configuration.groups.http_args import HttpConfigBase, from_env

from .url_validator import (
_MAX_REDIRECTS,
UrlValidationError,
UrlValidationPolicy,
describe_error_detail,
describe_media_source,
validate_url,
)

# Longest a client-supplied reason may render as inside an HttpStatusError.
# Matches ``dynamo.llm.exceptions.HttpError._MAX_MESSAGE_LENGTH``, which the
# binding applies to the other class it forwards on a 4xx. Generous on purpose:
# callers build real guidance here -- the trtllm decoder hint is ~480 characters
# and its test allows 2000 -- so a tight bound silently deletes the actionable
# part and leaves only a truncated prefix.
_MAX_MESSAGE_LENGTH = 8192


class HttpError(Exception):
"""Base class for all HTTP fetch failures."""
Expand All @@ -41,12 +51,46 @@ class HttpStatusError(HttpError):
"""Server responded with a non-2xx status."""

def __init__(self, status: int, message: str, url: str) -> None:
super().__init__(f"HTTP {status} for {url}: {message}")
# Both halves are client-supplied: ``url`` directly, and ``message``
# because aiohttp's own ClientResponseError text repeats the URL. The video
# diffusion handler puts str(exc) in its response body.
#
# ``message`` is bounded in the *attribute*, not just in the rendered
# string: errors.rs::extract_http_like_error reads ``.status`` and
# ``.message`` off this class by name and, per the SECURITY note there,
# forwards ``.message`` verbatim to the client on a 4xx. Nothing in the
# rendered text reaches that path. ``url`` is not part of that protocol,
# so it keeps its full value for debugging.
message = describe_error_detail(message, _MAX_MESSAGE_LENGTH)
super().__init__(f"HTTP {status} for {describe_media_source(url)}: {message}")
self.status = status
self.message = message
self.url = url


async def collect_capped(
chunks: AsyncIterator[bytes], url: str, max_bytes: Optional[int]
) -> bytes:
"""Join ``chunks`` into one body, refusing to buffer past ``max_bytes``.

The check is per chunk rather than on the finished body: a declared
Content-Length is attacker-controlled and absent on a chunked response, and
a single capped read is not enough either — aiohttp's ``read(n)`` returns
*at most* n bytes, so a short read would look like a body under the limit.
"""
out: list[bytes] = []
total = 0
async for chunk in chunks:
total += len(chunk)
if max_bytes is not None and total > max_bytes:
raise UrlValidationError(
f"Media exceeds the {max_bytes} byte download limit: "
f"{describe_media_source(url)}"
)
out.append(chunk)
return b"".join(out)


class HttpClient(abc.ABC):
"""Backend-neutral HTTP client.

Expand All @@ -66,6 +110,7 @@ async def fetch_bytes(
timeout: float,
*,
policy: Optional[UrlValidationPolicy] = None,
max_bytes: Optional[int] = None,
) -> bytes:
"""Fetch ``url`` and return the response body.

Expand All @@ -74,20 +119,29 @@ async def fetch_bytes(

``policy=None``: use the backend's built-in redirect handling.

``max_bytes`` set: refuse a body larger than that while it is being
read, so an attacker-chosen URL cannot buffer an unbounded response.
Raises :class:`UrlValidationError` — it is a verdict on a
client-supplied source, like the redirect cap below.

``policy`` set: follow redirects manually and revalidate each
hop against the policy via :func:`url_validator.validate_url`.
This is the SSRF-safe path; raises :class:`UrlValidationError`
if any hop fails or the chain exceeds ``_MAX_REDIRECTS``.
"""
if policy is None:
return await self._fetch_simple(url, timeout)
return await self._fetch_with_revalidation(url, timeout, policy)
return await self._fetch_simple(url, timeout, max_bytes=max_bytes)
return await self._fetch_with_revalidation(
url, timeout, policy, max_bytes=max_bytes
)

async def _fetch_with_revalidation(
self,
url: str,
timeout: float,
policy: UrlValidationPolicy,
*,
max_bytes: Optional[int] = None,
) -> bytes:
"""Manual redirect loop with per-hop SSRF validation (backend-neutral)."""
current = url
Expand All @@ -97,30 +151,36 @@ async def _fetch_with_revalidation(
await validate_url(current, policy)
visited.append(current)

body, redirect_to = await self._fetch_body_or_redirect(current, timeout)
body, redirect_to = await self._fetch_body_or_redirect(
current, timeout, max_bytes=max_bytes
)

if redirect_to is None:
if body is None:
raise HttpError(
f"Backend returned (None, None) for {current}; "
f"Backend returned (None, None) for {describe_media_source(current)}; "
"expected bytes on a terminal (2xx or 3xx-without-Location) response"
)
return body

if hops_remaining <= 0:
# ``visited`` holds attacker-chosen URLs; describe each one.
chain = [describe_media_source(hop) for hop in visited]
raise UrlValidationError(
f"Too many redirects (max={_MAX_REDIRECTS}); chain={visited}"
f"Too many redirects (max={_MAX_REDIRECTS}); chain={chain}"
)
hops_remaining -= 1
current = redirect_to

@abc.abstractmethod
async def _fetch_simple(self, url: str, timeout: float) -> bytes:
async def _fetch_simple(
self, url: str, timeout: float, *, max_bytes: Optional[int] = None
) -> bytes:
"""Backend's native redirect-following GET (no SSRF policy applied)."""

@abc.abstractmethod
async def _fetch_body_or_redirect(
self, url: str, timeout: float
self, url: str, timeout: float, *, max_bytes: Optional[int] = None
) -> tuple[bytes | None, str | None]:
"""Single hop with redirects disabled.

Expand Down
Loading
Loading