Fix image URL loading crashes and add a download timeout

[BarneyChambers]

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 #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

#289 and #294 fixed crashes in image sizing/config after loading; this PR is about loading itself. #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

[juliendenize]

Thanks for your contribution it makes a lot of sense.

My concern is that I think timeout should be configurable by the environment with a nice error message to inform user they can increase it.

[juliendenize]

Can we not make it a default here ?

[BarneyChambers]

Thanks for your contribution it makes a lot of sense.

My concern is that I think timeout should be configurable by the environment with a nice error message to inform user they can increase it.

Thanks so much for your review @juliendenize. I think your recommendations make sense, I have implemented them in the most recent commit.

download_image no longer defaults to timeout=10.0. Unset reads MISTRAL_COMMON_IMAGE_DOWNLOAD_TIMEOUT (10s if missing), and image_from_chunk passes that in explicitly.

On timeout the error is:
timed out after {n} seconds. Pass a larger timeout or set the environment variable MISTRAL_COMMON_IMAGE_DOWNLOAD_TIMEOUT to increase the timeout.

So a heavy image pipeline can raise the cap without threading timeout through the tokenizer.

[juliendenize]

Small follow-up to our earlier discussion: image_from_chunk resolves the timeout and download_image resolves it again when timeout=None — two resolution paths for one value. These suggestions keep the resolution in exactly one place (download_image's timeout is None branch) and update the test to guard that image_from_chunk doesn't override the default resolution.

Also flagging: the latest commit (dcc54b7) leaves image_from_chunk in the tests/test_image.py import block with a tab instead of spaces and no trailing comma, which is a syntax error — pytest tests/test_image.py fails at collection on this branch. Suggestion attached.

Verified locally on top of this branch: pytest tests/test_image.py → 51 passed; ruff check + ruff format --check clean.

[juliendenize]

Small follow-up to our earlier discussion: image_from_chunk resolves the timeout and download_image resolves it again when timeout=None — two resolution paths for one value. These suggestions keep the resolution in exactly one place (download_image's timeout is None branch) and update the test to guard that image_from_chunk doesn't override the default resolution.

Also flagging: the latest commit (dcc54b7) leaves image_from_chunk in the tests/test_image.py import block with a tab instead of spaces and no trailing comma, which is a syntax error — pytest tests/test_image.py fails at collection on this branch. Suggestion attached.

Verified locally on top of this branch: pytest tests/test_image.py → 51 passed; ruff check + ruff format --check clean.

[juliendenize]

Hmm sorry about the comment it's generated obviously, i'm just fixing some stuff related to style / minor refactoring and then i'll merge. Thanks a lot for iterating and the contrib !

[BarneyChambers]

Hmm sorry about the comment it's generated obviously, i'm just fixing some stuff related to style / minor refactoring and then i'll merge. Thanks a lot for iterating and the contrib !

No problem at all, thanks for merging!