Skip to content

types: make Image path extension check case-insensitive#696

Open
anxkhn wants to merge 1 commit into
ollama:mainfrom
anxkhn:fix/image-extension-case-insensitive
Open

types: make Image path extension check case-insensitive#696
anxkhn wants to merge 1 commit into
ollama:mainfrom
anxkhn:fix/image-extension-case-insensitive

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Image.serialize_model decides whether a string is a file path or already-base64
data. When a string is not an existing file, it checks the extension to tell "a
path that does not exist" apart from raw base64:

if self.value.split('.')[-1] in ('png', 'jpg', 'jpeg', 'webp'):
    raise ValueError(f'File {self.value} does not exist')

That comparison is case-sensitive, but image files routinely carry uppercase
extensions (cameras emit .JPG; case-insensitive filesystems treat .PNG the
same as .png). So a non-existent path with an uppercase extension skips the
File ... does not exist branch and falls through to the base64 handling:

  • Image(value='some/path/photo.jpg').model_dump() correctly raises
    File some/path/photo.jpg does not exist.
  • Image(value='some/path/photo.JPG').model_dump() instead raises the misleading
    Invalid image data, expected base64 string or path to image file.
  • Worse, a bare filename that happens to be valid base64 is silently accepted:
    Image(value='PHOTO.PNG').model_dump() returns the literal string 'PHOTO.PNG',
    which then gets sent in the images array to /api/generate and /api/chat as
    garbage image data instead of failing fast.

The fix lowercases the extracted extension before the membership test so
.PNG/.JPG/.JPEG/.WEBP behave the same as their lowercase counterparts. Non-image
strings (e.g. model_v1.2, plain base64) still fall through to the base64 check
unchanged.

Added a parametrized regression test in tests/test_type_serialization.py
covering 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.

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 vaderollama 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.

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.

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.

2 participants