Skip to content

fix(vl): raise a clear error on malformed data URLs - #4837

Open
SuperMarioYL wants to merge 1 commit into
InternLM:mainfrom
SuperMarioYL:fix/malformed-data-url-crash
Open

fix(vl): raise a clear error on malformed data URLs#4837
SuperMarioYL wants to merge 1 commit into
InternLM:mainfrom
SuperMarioYL:fix/malformed-data-url-crash

Conversation

@SuperMarioYL

Copy link
Copy Markdown
Contributor

Motivation

_load_data_url (in lmdeploy/vl/media/connection.py) parses data: URLs with two bare tuple-unpacks:

data_spec, data = url_spec_path.split(',', 1)
media_type, data_type = data_spec.split(';', 1)

A malformed data: URL that omits the comma and payload (e.g. data:image/png, data:,, data:image/png;base64) makes str.split return a single-element list, so the unpack raises an opaque ValueError: not enough values to unpack (expected 2, got 1). That error is swallowed by the surrounding except in the request path (lmdeploy/serve/core/async_engine.py) into a generic "in prompt processing error", hiding the actual cause from both the caller and the logs.

Change

Replace the bare unpacks with length-checked splits that raise a clear, debuggable ValueError at the media-loader boundary (the layer that owns data: URL parsing), pointing the caller at the expected data:<media-type>;<type>,<payload> shape:

  • missing comma / empty payload → Malformed data URL: expected "data:<media-type>;<type>,<payload>" but got "data:..."
  • missing ;<type>Malformed data URL media type: expected "<media-type>;<type>" but got "..."

The existing NotImplementedError('Only base64 data URLs are supported for now.') for non-base64 payloads is unchanged. The generic catch in async_engine.py is intentionally left untouched — it now surfaces a clear root-cause message instead of an opaque unpack error.

Why a guard rather than relying on the existing catch

The generic catch already prevents the crash from surfacing as an internal error, but it converts a structural input bug into an unactionable "in prompt processing error": a caller sending data:image/png cannot tell from the response that their data URL is malformed. Raising the clear error at the loader boundary (the layer that owns data-URL parsing) is the smallest root-cause clarification at the correct layer — no engine-path change, mirroring the empty-prompt guard landed in #4803.

Tests

Added test_load_data_url_rejects_malformed (parametrized over the three malformed shapes) and test_load_data_url_loads_well_formed_base64. The malformed cases are red on main (raise not enough values to unpack, which does not match Malformed data URL) and green on this branch. Well-formed base64 loading is unchanged.

26 passed (4 new) on branch; 3 failed / 23 passed on main (the 3 new malformed cases).

Scope

Two files: lmdeploy/vl/media/connection.py (the guard) and tests/test_lmdeploy/test_vl/test_safe_url.py (the tests). No engine-layer or serving-path changes.

_load_data_url did bare tuple-unpacks on the comma and the ';<type>'
separators, so a malformed data: URL (e.g. "data:image/png", "data:,",
"data:image/png;base64") crashed with an opaque
"not enough values to unpack" ValueError. That opaque error was swallowed
by the generic except in async_engine into a generic "in prompt processing
error", hiding the root cause from users and debuggers.

Replace the bare unpacks with length-checked splits that raise a clear,
debuggable ValueError('Malformed data URL: ...') at the media-loader
boundary (the correct layer), mirroring the empty-prompt guard pattern of
InternLM#4803. The async_engine generic catch is intentionally left untouched; it
now surfaces a clear message instead of an opaque unpack error.

Adds test_load_data_url_rejects_malformed (red on master, green here) plus
a positive well-formed base64 case to test_safe_url.py.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant