Skip to content

Fix image URL loading crashes and add a download timeout - #1

Open
BarneyChambers wants to merge 20 commits into
mainfrom
fix/image-url-loading
Open

BarneyChambers wants to merge 20 commits into
mainfrom
fix/image-url-loading

Conversation

@BarneyChambers

@BarneyChambers BarneyChambers commented Sep 7, 2026 •

Copy link
Copy Markdown
Owner

Summary

image_from_chunk mishandles three of the four URL shapes it accepts, and the HTTP branch can hang forever.

if chunk.get_url().startswith("data:image"):
    data = chunk.get_url().split(",")[1]          # IndexError when there is no comma
    ...
if chunk.get_url().startswith("file"):            # matches "file.png", not just file://
    return Image.open(open(chunk.get_url().replace("file://", ""), "rb"))  # handle never closed
if chunk.get_url().startswith("http"):
    return download_image(chunk.get_url())        # requests.get with no timeout

Deterministic:

data:image/png;base64,<payload>   -> works              (unchanged)
data:image/png;base64             -> IndexError         (should be RuntimeError)
data:image/png;base64,            -> UnidentifiedImageError deep in PIL (should be RuntimeError)
file.png                          -> opens ./file.png   (should be unsupported scheme)
file:///tmp/ok.png                -> works              (unchanged)
https://<hanging-server>/x.png    -> blocks forever     (should time out)

This function is on the public encode path: ImageEncoder.__call__ -> InstructTokenizerV3._encode_content_chunk -> MistralTokenizer.encode_chat_completion, which is what vLLM and the Transformers MistralCommonBackend call. ImageURLChunk content comes from the request, so all three inputs are attacker-supplied.

Fixes mistralai#307

Real world example

A serving stack runs the experimental tokenize server (mistral_common.experimental.app) or any framework that calls encode_chat_completion on user requests. A client sends:

{"messages": [{"role": "user", "content": [
  {"type": "image_url", "image_url": {"url": "data:image/png;base64"}}
]}]}

What happens today:

Step Result
image_from_chunk splits on "," IndexError: list index out of range
Experimental app handlers only catch ValueError; IndexError escapes as an unhandled 500
Same request with "url": "https://attacker.example/slow" requests.get never returns; the encode worker is gone until restart

The startswith("file") arm is quieter: a URL like file.png is opened relative to the server's working directory, so a name collision reads a local file instead of raising "Unsupported image url scheme". The handle is also never closed.

After this fix: the malformed data URL and the bare file.png raise RuntimeError (the same family the function already uses for unsupported schemes), and the HTTP branch gives up after 10 seconds with the existing "Error downloading the image" wrapping, since requests.exceptions.Timeout is a RequestException.

Fix

  • Split the data URL on the first comma with partition; raise RuntimeError when there is no payload (covers both the missing comma and the empty payload).
  • Require the file:// prefix for the local-file branch and open the file in a with block (Image.load() before close), so bare names fall through to the unsupported-scheme error and the handle is closed. Behavior note: multi-frame images (GIF/TIFF) can no longer be seeked past frame 0 after return; the encoder only ever used frame 0, and the old code merely leaked the fd that made seeking possible.
  • Pass timeout (default 10.0s) from download_image to requests.get. Exposed as a parameter so callers can tune it; existing error wrapping is unchanged.

Not a duplicate

mistralai#289 and mistralai#294 fixed crashes in image sizing/config after loading; this PR is about loading itself. mistralai#259 touches audio data URL prefixes, different file. Audio.from_url has the same missing timeout but a different grammar, so it stays out of scope here; flagged in the issue as follow-up.

Test

No existing test covered malformed data URLs, bare file names, or the timeout (the two existing mocks accept any call signature). Added:

  • test_image_from_chunk_data_url_without_payload (both ;base64 and ;base64,)
  • test_image_from_chunk_bare_file_name_is_unsupported
  • test_image_from_chunk_file_uri (regression guard for real file:// URIs)
  • test_download_image_passes_timeout (asserts timeout= reaches requests.get, and that a Timeout becomes RuntimeError)

The first, second and fourth fail on main and pass with this change:

uv run pytest tests/test_image.py -q
33 passed in 1.07s

Full unit suite: uv run pytest tests/ --ignore=tests/integrations --ignore=tests/integration -n 4 --dist loadfile -> 1233 passed, 16 skipped. Doctests, ruff check, ruff format and mypy all pass on the changed files.

Verification

  • Run the tests: see above (33 passed; 4 of the new asserts red on main)
  • Verify the thing does what it should: malformed data URLs and bare file names raise RuntimeError; requests.get receives timeout=10.0
  • Verify the thing does not do what it should not: valid data URLs, file:// URIs and mocked HTTP downloads still encode byte-identically (existing test_download_image, test_image_encoder_formats untouched apart from mock signatures accepting the new kwarg)
  • Supporting configuration: N/A; no configuration changed
  • Document: docstring for download_image updated with the new timeout arg; no other user-facing API change

image_from_chunk raised an IndexError on data URLs without a comma,
treated any URL starting with "file" (e.g. "file.png") as a local file
path, and leaked the file handle it opened. download_image called
requests.get without a timeout, so a hanging server blocked the encode
path forever.
ManoharPaturi and others added 18 commits September 8, 2026 10:15
Co-authored-by: ManoharPaturi <186662190+ManoharPaturi@users.noreply.github.com>
Co-authored-by: Vibe Nuage Agent <vibe@mistral.ai>
…-gh-pages (mistralai#319)

Co-authored-by: Vibe Nuage Agent <vibe@mistral.ai>
Co-authored-by: Vibe Nuage Agent <vibe@mistral.ai>
Co-authored-by: juliendenize <juliendenize@users.noreply.github.com>
Serving stacks cannot pass timeout= into encode_chat_completion.
Read MISTRAL_COMMON_IMAGE_DOWNLOAD_TIMEOUT (default 10s), pass it
explicitly from image_from_chunk, and name both knobs if the download
times out.
…sage (mistralai#325)

Co-authored-by: Vibe Nuage Agent <vibe@mistral.ai>
Co-authored-by: juliendenize <juliendenize@users.noreply.github.com>
…malize (mistralai#316) (mistralai#321)

Co-authored-by: Julien Denize <40604584+juliendenize@users.noreply.github.com>
Co-authored-by: Vibe Nuage Agent <vibe@mistral.ai>
Co-authored-by: juliendenize <juliendenize@users.noreply.github.com>
Co-authored-by: Julien Denize <40604584+juliendenize@users.noreply.github.com>
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.

[BUG: image_from_chunk crashes on malformed data URLs, opens bare "file*" names as local paths, and downloads with no timeout]

6 participants