Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 20 additions & 1 deletion components/src/dynamo/common/multimodal/image_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@
from dynamo.common.utils.runtime import run_async

from ..http import HttpError, HttpStatusError, HttpTimeoutError, fetch_bytes
from ..http.url_validator import UrlValidationPolicy, validate_media_url
from ..http.url_validator import (
UrlValidationError,
UrlValidationPolicy,
validate_media_url,
)

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -143,6 +147,11 @@ async def _fetch_and_process(self, image_url: str) -> Image.Image:
except Image.UnidentifiedImageError as e:
logger.error(f"Unsupported image format loading: '{image_url}'")
raise HttpStatusError(415, "Unsupported Media Type", image_url) from e
except UrlValidationError as e:
# Keep the type (must precede ValueError, its base) so the batch
# caller can still map this client error to a 4xx, not a 500.
logger.error("URL rejected loading image: '%s': %s", image_url, e)
raise
except ValueError as e:
if "Unsupported image format" in str(e):
logger.error(f"Unsupported image format loading: '{image_url}'")
Expand Down Expand Up @@ -253,6 +262,8 @@ async def load_image_batch(
HttpStatusError: If any image fails with an HTTP status error
(e.g. 415 Unsupported Media Type); the status is preserved so the
frontend returns the correct client-error code instead of 500.
UrlValidationError: If a media URL is rejected by the SSRF policy;
preserved as a ValueError so the frontend returns a 4xx, not 500.
Exception: If any image fails to load for any other reason
ValueError: If enable_frontend_decoding=True but nixl_connector is None
"""
Expand Down Expand Up @@ -282,6 +293,7 @@ async def load_image_batch(
loaded_images = []
collective_exceptions = ""
status_error: HttpStatusError | None = None
url_error: UrlValidationError | None = None
for media_item, result in zip(image_mm_items, results):
if isinstance(result, Exception):
source = media_item.get(URL_VARIANT_KEY, "decoded")
Expand All @@ -295,12 +307,19 @@ async def load_image_batch(
# the first one so single-item batches keep their client-error code.
if status_error is None and isinstance(result, HttpStatusError):
status_error = result
# Same for a rejected URL (UrlValidationError is a ValueError):
# preserve it so the frontend still gets a 4xx, not a 500.
elif url_error is None and isinstance(result, UrlValidationError):
url_error = result
continue
Comment thread
KrishnanPrash marked this conversation as resolved.
loaded_images.append(result)

if status_error is not None:
raise status_error

if url_error is not None:
raise url_error
Comment thread
KrishnanPrash marked this conversation as resolved.

if collective_exceptions:
raise Exception(collective_exceptions)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from PIL import Image

from dynamo.common.http import HttpStatusError, HttpTimeoutError
from dynamo.common.http.url_validator import UrlValidationPolicy
from dynamo.common.http.url_validator import UrlValidationError, UrlValidationPolicy
from dynamo.common.multimodal.image_loader import URL_VARIANT_KEY, ImageLoader

pytestmark = [
Expand Down Expand Up @@ -272,6 +272,39 @@ async def test_unsupported_format_batch_data_url_raises_415(
assert exc_info.value.status == 415


# --- SSRF / URL-validation error contract ---


# Cloud metadata IP (link-local) -- the canonical SSRF target; rejected as a
# blocked IP literal before any DNS/connect, so this test makes no network call.
_METADATA_IP_URL = "https://169.254.169.254/latest/meta-data/"


async def test_ssrf_blocked_batch_url_raises_url_validation_error() -> None:
"""An SSRF-blocked URL must escape load_image_batch as UrlValidationError (a
ValueError -> 4xx), not the bare Exception the aggregator used to raise (500)."""
strict_loader = ImageLoader(
cache_size=4, http_timeout=30.0, url_policy=UrlValidationPolicy()
)
with pytest.raises(UrlValidationError, match="blocked range"):
await strict_loader.load_image_batch([{URL_VARIANT_KEY: _METADATA_IP_URL}])


async def test_url_validation_error_from_fetch_preserved(
loader: ImageLoader,
) -> None:
"""A UrlValidationError raised mid-fetch (redirect revalidation) must survive
_fetch_and_process's except branch, not be flattened to a plain ValueError."""
mock_fetch = _mock_fetch_bytes(
side_effect=UrlValidationError("Too many redirects (max=3)")
)
with patch(_FETCH_BYTES_PATH, mock_fetch):
with pytest.raises(UrlValidationError, match="Too many redirects"):
await loader.load_image_batch(
[{URL_VARIANT_KEY: "https://example.com/img.png"}]
)


async def test_cache_is_lru_not_fifo(loader: ImageLoader) -> None:
"""Accessing a cached entry should protect it from eviction (LRU, not FIFO)."""
loader._cache_size = 3
Expand Down
6 changes: 6 additions & 0 deletions components/src/dynamo/trtllm/multimodal_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
from safetensors.torch import load_file as safetensors_load_file
from tensorrt_llm.llmapi.tokenizer import tokenizer_factory

from dynamo.common.http import HttpStatusError
from dynamo.common.http.url_validator import UrlValidationError
from dynamo.common.multimodal.image_loader import ImageLoader
from dynamo.runtime.logging import configure_dynamo_logging

Expand Down Expand Up @@ -341,6 +343,10 @@ async def process_openai_request(
logging.info(
f"Loaded {len(pil_images)} image(s) as PIL Images"
)
except (UrlValidationError, HttpStatusError):
# Client errors: let them reach the frontend as a 4xx
# instead of the generic catch below swallowing them to None.
raise
Comment thread
KrishnanPrash marked this conversation as resolved.
Comment thread
KrishnanPrash marked this conversation as resolved.
except Exception as e:
logging.error(f"Failed to load images: {e}")
return None
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0

"""process_openai_request must let client-error types from image loading
propagate (so the frontend returns a 4xx) instead of swallowing them to None."""

from unittest.mock import AsyncMock, MagicMock

import pytest
import torch

# multimodal_processor imports tensorrt_llm, whose import needs CUDA. -m selection
# happens after collection, so this module is still imported on the CPU-only step;
# skip at collection there to avoid an ImportError. GPU runs it via the gpu_1 marker.
if not torch.cuda.is_available():
pytest.skip(
"CUDA/GPU not available, but tensorrt_llm import and the test require GPU.",
allow_module_level=True,
)

from dynamo.common.http import HttpStatusError
from dynamo.common.http.url_validator import UrlValidationError
from dynamo.trtllm.multimodal_processor import MultimodalRequestProcessor

pytestmark = [
pytest.mark.unit,
pytest.mark.trtllm,
pytest.mark.multimodal,
pytest.mark.pre_merge,
pytest.mark.gpu_1,
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
pytest.mark.profiled_vram_gib(0),
]
Comment thread
KrishnanPrash marked this conversation as resolved.


@pytest.mark.asyncio
@pytest.mark.parametrize(
"error",
[
UrlValidationError("blocked IP literal"),
HttpStatusError(415, "Unsupported Media Type", "https://example.com/x.png"),
],
)
async def test_client_errors_propagate(error) -> None:
# Mock tokenizer skips tokenizer_factory; mock loader forces the failure.
processor = MultimodalRequestProcessor(
model_type="multimodal",
model_dir="unused",
max_file_size_mb=10,
tokenizer=MagicMock(),
)
processor.image_loader.load_image_batch = AsyncMock(side_effect=error)

request = {
"multi_modal_data": {"image_url": [{"Url": "https://example.com/x.png"}]}
}
with pytest.raises(type(error)):
await processor.process_openai_request(
request, embeddings=None, ep_disaggregated_params=None
)
Loading