types: make Image path extension check case-insensitive#696
Open
anxkhn wants to merge 1 commit into
Open
Conversation
The guard in Image.serialize_model compared the file extension without normalizing case, so a non-existent path with an uppercase extension (e.g. photo.PNG from a camera or a case-insensitive filesystem) skipped the "File ... does not exist" branch and fell through to the base64 path. Filenames that happen to be valid base64 (PHOTO.PNG, DCIM0001.WEBP) were then silently accepted and sent as garbage image data instead of raising, defeating the extension guard for every uppercase-extension path. Lowercase the extracted extension before the membership test so .PNG/.JPG/.JPEG/.WEBP behave the same as their lowercase counterparts. Non-image strings still correctly fall through to the base64 check. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
vaderollama
approved these changes
Jul 14, 2026
vaderollama
left a comment
There was a problem hiding this comment.
LGTM — clean one-liner fix. Case-insensitive extension check prevents uppercase image extensions (.JPG, .PNG) from being misidentified as base64 data. Tests parametrize the common uppercase variants.
@ParthSareen — ready to merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Image.serialize_modeldecides whether a string is a file path or already-base64data. When a string is not an existing file, it checks the extension to tell "a
path that does not exist" apart from raw base64:
That comparison is case-sensitive, but image files routinely carry uppercase
extensions (cameras emit
.JPG; case-insensitive filesystems treat.PNGthesame as
.png). So a non-existent path with an uppercase extension skips theFile ... does not existbranch and falls through to the base64 handling:Image(value='some/path/photo.jpg').model_dump()correctly raisesFile some/path/photo.jpg does not exist.Image(value='some/path/photo.JPG').model_dump()instead raises the misleadingInvalid image data, expected base64 string or path to image file.Image(value='PHOTO.PNG').model_dump()returns the literal string'PHOTO.PNG',which then gets sent in the
imagesarray to/api/generateand/api/chatasgarbage image data instead of failing fast.
The fix lowercases the extracted extension before the membership test so
.PNG/.JPG/.JPEG/.WEBPbehave the same as their lowercase counterparts. Non-imagestrings (e.g.
model_v1.2, plain base64) still fall through to the base64 checkunchanged.
Added a parametrized regression test in
tests/test_type_serialization.pycovering the uppercase extensions for both a path-shaped value and a bare
filename; both cases fail on the current code and pass with the fix.