A damaged image upload answers 500 instead of a worded refusal #1065

New issue

Closed

opened 2026-10-04 03:38:16 +00:00 by claude-bot · 1 comment

claude-bot commented

2026-10-04 03:38:16 +00:00

Collaborator

Found on 2026-10-04 while checking the map upload in the journeys stack.

Seen

An image whose header reads but whose pixel data is broken gets 500 {"detail":"Internal server error"}. Three of 23 image variants tried did this: a PNG whose IDAT is not a zlib stream, a PNG with an empty IDAT, and a JPEG cut in half. A sound image uploads and tiles correctly.

app/services/image_service.py  validate_and_reencode
  img = img.convert("RGB") / img.save(out, …)
OSError: image file is truncated (20 bytes not processed)
OSError: broken data stream when reading image file

Cause

validate_and_reencode wrapped only Image.open and verify() in its try. Those read the header and the PNG chunk checksums. The pixels are decoded later, in the re-encode, which was outside the guard. The same pipeline serves map images, page pictures, attachments and imports, so any of them could hit it.

No test caught it because every upload test builds its image with Pillow in-process, so every image sent is sound. The refusal tests cover wrong type, over the cap and empty, never a header that reads with a body that does not.

Fix

The re-encode goes inside the guard and raises the service's ImageError ("This image is damaged or incomplete, so it could not be read."), which every caller already turns into a 422. Branch fix/map-upload-500, with failing-first tests.

Found on 2026-10-04 while checking the map upload in the journeys stack. ## Seen An image whose header reads but whose pixel data is broken gets `500 {"detail":"Internal server error"}`. Three of 23 image variants tried did this: a PNG whose IDAT is not a zlib stream, a PNG with an empty IDAT, and a JPEG cut in half. A sound image uploads and tiles correctly. ``` app/services/image_service.py validate_and_reencode img = img.convert("RGB") / img.save(out, …) OSError: image file is truncated (20 bytes not processed) OSError: broken data stream when reading image file ``` ## Cause `validate_and_reencode` wrapped only `Image.open` and `verify()` in its `try`. Those read the header and the PNG chunk checksums. The pixels are decoded later, in the re-encode, which was outside the guard. The same pipeline serves map images, page pictures, attachments and imports, so any of them could hit it. No test caught it because every upload test builds its image with Pillow in-process, so every image sent is sound. The refusal tests cover wrong type, over the cap and empty, never a header that reads with a body that does not. ## Fix The re-encode goes inside the guard and raises the service's `ImageError` ("This image is damaged or incomplete, so it could not be read."), which every caller already turns into a 422. Branch `fix/map-upload-500`, with failing-first tests.

claude-bot referenced this issue from a pull request that will close it,

2026-10-04 03:38:26 +00:00

A damaged image is refused in words, not with a 500 (#1065) #1066

claude-bot closed this issue

2026-10-04 04:30:05 +00:00

claude-bot referenced this issue from a commit

2026-10-04 04:30:06 +00:00

Merge PR #1066: a damaged image upload is refused in words, not with a 500 (#1065)

claude-bot commented

2026-10-04 04:30:13 +00:00

Author

Collaborator

Fixed in PR #1066, merged to main. CI was green on the PR's head.

The change: image_service.validate_and_reencode now decodes the pixels inside its guard. A damaged image raises ImageError("This image is damaged or incomplete, so it could not be read."), which every caller already turns into a 422.
Covers: map images, page pictures, attachments and imports.
Ships in: the next prerelease (v4.5.0-rc3) and 4.5.0.

No follow-ups.

**Fixed in PR #1066, merged to main. CI was green on the PR's head.** - **The change:** `image_service.validate_and_reencode` now decodes the pixels inside its guard. A damaged image raises `ImageError("This image is damaged or incomplete, so it could not be read.")`, which every caller already turns into a 422. - **Covers:** map images, page pictures, attachments and imports. - **Ships in:** the next prerelease (`v4.5.0-rc3`) and 4.5.0. No follow-ups.