Skip to content

[Bugfix][Multimodal] Validate base64 video payloads, matching image and audio - #54323

Merged
Isotr0py merged 3 commits into
vllm-project:mainfrom
Hotragn:fix/video-base64-validate
Sep 14, 2026
Merged

Isotr0py merged 3 commits into
vllm-project:mainfrom
Hotragn:fix/video-base64-validate

Conversation

@Hotragn

@Hotragn Hotragn commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Correction (updated after re-checking the default backend). The original
title and body of this PR claimed the malformed-video path returns HTTP 500.
That is wrong for the default configuration, and I have retitled and rewritten
accordingly. Under VLLM_VIDEO_LOADER_BACKEND=opencv (the default),
open_video_capture raises ValueError("Could not open video stream"), which
already maps to 400. I verified this by reading
vllm/multimodal/video_decoders/opencv.py:73-74 rather than by observing a
live 500, which is what I should have done before opening the PR. The change
below still stands on its own, but on consistency and input-validation
grounds, not on a status-code bug. Apologies for the noise.

Follow-up (2026-09-12). The retracted claim had survived in two places the
rewrite missed: the commit subject and the new test's docstring both still said
"500 instead of 400". Both are corrected, so nothing that would land in the tree
asserts it any more. Rebased onto current main and re-verified; see
Test Result.

Purpose

VideoMediaIO.load_base64 is the only media loader in the tree that decodes
base64 without validation:

return self.load_bytes(pybase64.b64decode(data))

Without validate=True, pybase64 silently discards every character outside
the base64 alphabet and re-packs what is left. The resulting byte stream is not
the payload the client sent — every byte after the first invalid character is
shifted — and it is handed directly to the video decoder.

loader call site validates
ImageMediaIO.load_base64 media/image.py:105 yes (since #32406)
ImageEmbeddingMediaIO.load_base64 media/image.py:154 yes
AudioMediaIO.load_base64 media/audio.py:561 yes (since #53744)
AudioEmbeddingMediaIO.load_base64 media/audio.py:604 yes
VideoEmbeddingMediaIO.load_base64 media/video.py:251 yes (since #54242)
VideoMediaIO.load_base64 media/video.py:193 no

Line numbers are against main at 29332cf936. The last row is the point: since
#54242 landed VideoEmbeddingMediaIO
on 2026-08-31 — after this PR was opened — the inconsistency is no longer video versus
the other modalities. It is between two loaders in the same module, three classes
apart.

Two concrete consequences:

  1. A corrupted stream reaches the decoder. vLLM hands ffmpeg/OpenCV a byte
    sequence that no client ever sent, derived from a malformed one. Rejecting
    undecodable input before it reaches a C/C++ media parser is the same
    rationale that made [Bugfix][Multimodal] Reject malformed base64 audio with 400 instead of 500 #53744 worth merging for audio.
  2. The error a user sees is misleading. Today a malformed base64 payload
    surfaces as Could not open video stream — a message about the video,
    when the actual fault is the encoding. With validate=True the client
    gets binascii.Error mapped to 400, naming the real problem.

What this PR does not claim

ImageMediaIO/AudioMediaIO had a genuine status-code bug (LibsndfileError
inherits RuntimeError → 500). Video does not, under the default backend:
opencv's failure is already a ValueError → 400. I have not verified the
behaviour of the torchcodec or pynvvideocodec backends on shifted input,
so I am not claiming a 500 there either.

Reachability

MediaConnector._load_data_url parses data:<media-type>;base64,<data> and
dispatches straight to the loader (vllm/multimodal/media/connector.py:331), so
any chat request carrying
"video_url": {"url": "data:video/mp4;base64,<...>"} reaches
media/video.py:169. The video/jpeg branch above it is unaffected — it
delegates to image_io.load_base64, which is already strict.

Approach

Pass validate=True, matching image and audio. pybase64 then raises
binascii.Error, a ValueError subclass, which maps to 400 through the
existing handler. No new exception type and no new handler.

Behaviour change worth calling out

validate=True also rejects base64 containing line breaks or whitespace. RFC
2045 permits line-wrapped base64, so a payload produced by base64 clip.mp4 or
Java's MIME encoder decodes today and will be rejected with a 400 after this
change.

I chose to match image and audio rather than invent a third behaviour: image has
been strict since January and audio since last week, so a client that wraps its
base64 already fails on those modalities. If maintainers would rather tolerate
RFC 2045 whitespace, the right fix is to strip it in all three loaders, which I
am happy to send as a follow-up.

Relationship to #46836

#46836 also touches this line,
adding a size check immediately before it, but leaves the decode itself lenient —
it addresses payload size, not payload validity. The two changes are
complementary; whichever lands second is a one-line rebase.

Test Plan

pytest tests/multimodal/media/test_video.py -v

Adds test_load_base64_rejects_malformed, which drives
load_base64("video/mp4", ...) with a payload containing !!!@@@. The test
registers a sentinel video backend that raises AssertionError if it is ever
called, so the assertion is specifically that the malformed input is rejected
before the decoder runs — not merely that some exception escapes. This is the
property the change is actually about, and it is backend-independent, so the
test does not depend on which decoder is configured.

Test Result

Re-run 2026-09-12 on a fresh x86_64 Linux runner against current main
(d9105ea80), Python 3.12, VLLM_USE_PRECOMPILED=1 uv pip install -e ..

Before — the branch's test file checked out on top of main, so only the
source fix is missing. The sentinel backend is what makes this specific: the
failure is not "some exception was raised", it is that the decoder was entered
at all, because the lenient decode stripped !!!@@@ and reproduced 128
plausible-looking bytes.

=========== BEFORE  fix/video-base64-validate ===========
HEAD = d9105ea80

>       raise AssertionError("video decoder reached with undecodable base64")
E       AssertionError: video decoder reached with undecodable base64

tests/multimodal/media/test_video.py:43: AssertionError
FAILED tests/multimodal/media/test_video.py::test_load_base64_rejects_malformed
================ 1 failed, 38 deselected, 14 warnings in 0.80s =================

After — the whole file on this branch:

=========== AFTER   fix/video-base64-validate ===========
HEAD = c10adc57a

======================= 39 passed, 18 warnings in 17.71s =======================

Whole directory, pytest tests/multimodal/media, both revisions:

main                       158 passed, 19 warnings in 131.73s
fix/video-base64-validate  164 passed, 19 warnings in 129.49s

Zero failures on either side. The six-test gap is not this PR: one test is the
new one, and the other five arrive with #53675 and #55642, which landed between
d9105ea80 and the 29332cf936 this branch is rebased onto.

pre-commit hooks applicable to the two changed files — ruff-check,
ruff-format, typos, mypy (3.12), check-spdx-header,
check-root-lazy-imports, check-forbidden-imports, check-torch-cuda-call
all pass.

If maintainers judge that consistency alone does not clear the bar now that the
status-code claim is gone, I am happy to close this.


AI assistance was used to research and draft this change. I have reviewed every
changed line and run the tests above.

@Hotragn
Hotragn requested a review from Isotr0py as a code owner August 29, 2026 07:29

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added multi-modality Related to multi-modality (#4194) bug Something isn't working labels Aug 29, 2026
@Hotragn Hotragn changed the title [Bugfix][Multimodal] Reject malformed base64 video with 400 instead of 500 [Bugfix][Multimodal] Validate base64 video payloads, matching image and audio Aug 29, 2026
…nd audio

VideoMediaIO.load_base64 was the only base64 loader in the tree decoding
without validate=True. pybase64 then silently discards every character
outside the base64 alphabet and re-packs what is left, so the decoder is
handed a byte stream no client ever sent and the resulting error blames
the video rather than the encoding.

Pass validate=True, matching ImageMediaIO, AudioMediaIO and the
VideoEmbeddingMediaIO loader in the same module. binascii.Error is a
ValueError subclass, so it maps to 400 through the existing handler with
no new exception type.

Signed-off-by: hotragn <hotragn.pettugani_2024@woxsen.edu.in>
@Hotragn
Hotragn force-pushed the fix/video-base64-validate branch from 6b54207 to c10adc5 Compare September 12, 2026 18:20
@Hotragn

Hotragn commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and refreshed, with one correction to my own diff that I
should have caught when I retracted the 500 claim two weeks ago.

The retracted claim had survived in the diff. The body was rewritten, but the commit
subject still read "Reject malformed base64 video with 400 instead of 500" and the new
test's docstring still said "the client gets a 500 instead of a 400". Both would have
landed in the tree asserting something I had already established is false under the
default backend. Fixed in c10adc57a; the source change is one character short of
untouched (validate=True), so this is purely the wording.

The case changed shape while this sat.
#54242 added VideoEmbeddingMediaIO
to media/video.py on 2026-08-31, and it decodes with validate=True. So the
inconsistency is no longer "video behaves differently from image and audio" — it is two
loaders in the same module, three classes apart, disagreeing about whether malformed
base64 is an error:

# vllm/multimodal/media/video.py:193  VideoMediaIO
return self.load_bytes(pybase64.b64decode(data))

# vllm/multimodal/media/video.py:251  VideoEmbeddingMediaIO
return self.load_bytes(pybase64.b64decode(data, validate=True))

Fresh verification against main (full output in the body): BEFORE the sentinel
backend fires — AssertionError: video decoder reached with undecodable base64, i.e.
the corrupted stream is entered, not merely that some exception escapes. AFTER, 39
passed. pytest tests/multimodal/media is all-green on both revisions (158 vs 164
passed; five of the six extra tests arrive with #53675 and #55642, which landed between
the runner's main and the commit this branch is rebased onto).

Backends re-checked, including deepstream.py which landed since I opened this.
opencv, pynvvideocodec and deepstream all normalise decode failures to
ValueError, so all three are already 400 — consistent with the retraction.
torchcodec.make_torchcodec_decoder still hands the bytes to VideoDecoder(...)
unguarded, so whatever that raises propagates as-is; I have no torchcodec build here and
am not claiming a 500 there, only noting it is the one path I cannot account for.

The standing offer holds: if consistency alone does not clear the bar now that the
status-code claim is gone, say so and I will close this rather than leave it open.

Comment thread tests/multimodal/media/test_video.py
Comment thread tests/multimodal/media/test_video.py Outdated
Comment thread tests/multimodal/media/test_video.py Outdated
Co-authored-by: Isotr0py <2037008807@qq.com>
Signed-off-by: Isotr0py <2037008807@qq.com>
@Isotr0py
Isotr0py enabled auto-merge (squash) September 14, 2026 02:51
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 14, 2026
@Isotr0py

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #88713 for commit 6b85db07e3eb.

@Isotr0py
Isotr0py merged commit a2685f2 into vllm-project:main Sep 14, 2026
101 of 102 checks passed
Shreya-gaur pushed a commit to Shreya-gaur/vllm_private that referenced this pull request Sep 14, 2026
…nd audio (vllm-project#54323)

Signed-off-by: hotragn <hotragn.pettugani_2024@woxsen.edu.in>
Signed-off-by: Isotr0py <2037008807@qq.com>
Co-authored-by: Isotr0py <2037008807@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working multi-modality Related to multi-modality (#4194) ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants