Skip to content

fix: JPEG detection fails on files with trailing data (Motion Photos) - #379

Open
tetsugakusha256 wants to merge 1 commit into
3rd:masterfrom
tetsugakusha256:fix/jpeg-trailing-data-detection
Open

tetsugakusha256 wants to merge 1 commit into
3rd:masterfrom
tetsugakusha256:fix/jpeg-trailing-data-detection

Conversation

@tetsugakusha256

Copy link
Copy Markdown

detect_format() in magic.lua has a check that requires FFD9 (JPEG EOI) to
be the literal last 2 bytes of the file. That breaks on any JPEG that has
data appended after the actual image — Google/Samsung Motion Photos being
the common case (JPEG + embedded MP4 + trailer footer).

For these files the real EOI is somewhere in the middle, so
has_jpeg_end_signature() returns false and detect_format() returns nil for
the whole file, even though it's a perfectly valid JPEG. ImageMagick has
no problem with these files (tested manually with identify/convert), it
just stops reading at EOI like it's supposed to.

I don't think this check is worth keeping — no other format in the table
gets this kind of extra validation, and detect_format() has no fallback
to the identify-based path used elsewhere in the plugin, so this was a
silent permanent failure rather than just a perf hit.

Removed the EOI check and the now-unused has_jpeg_end_signature function.

Tested:

  • Motion Photo JPEGs now detected/rendered correctly
  • truncated JPEGs still fail sensibly downstream - partial render if
    there's enough data, clean identify error ("insufficient image data")
    if not

Files with data appended after the JPEG EOI marker (Google/Samsung
Motion Photos, etc.) were being rejected by detect_format/is_image
since has_jpeg_end_signature required FFD9 to be the file's final two
bytes. These are valid, fully renderable JPEGs — ImageMagick decodes
them fine, it just stops at EOI and ignores trailing data.
Eurekaimer added a commit to Eurekaimer/cachyos-config that referenced this pull request Sep 13, 2026
…otices

image.nvim's magic.lua requires FF D9 to be the literal last two bytes of a
JPEG, so images with data appended after EOI (QQ/WeChat export form, 33 of the
56 JPEGs in the user's image bed) were rejected as "not an image" and never
rendered, although ImageMagick, browsers, and Obsidian decode them fine.

The config now wraps magic.detect_format: the stock check runs first so
PNG/GIF/WebP behaviour is unchanged, and only on failure does it verify the
JPEG SOI header and scan backwards in 64 KB chunks for FF D9. Truncated
downloads still lack EOI and stay rejected. Upstream PR 3rd/image.nvim#379
fixes the same bug by deleting the check outright; keeping it preserves the
truncation guard.

The downloader also gained --retry/--retry-all-errors, because the Markdown
integration re-requests every visible remote image on each render pass and the
local proxy drops TLS intermittently (curl 35/56), which produced one error
notification per attempt even when a later attempt succeeded. Failures are now
reported at most once per URL per session.

Verified: 7/7 headless detection cases; three JPEGs render in real Kitty with
the real config and note; zero notifications observed; 12/12 downloads clean;
5 concurrent failures produce exactly one notification (was 5).
@Eurekaimer

Eurekaimer commented Sep 13, 2026

Copy link
Copy Markdown

I independently reproduced this on current master, and it seems to affect more than just Motion Photos.

In one of my image repositories, 33 out of 56 regular JPEGs have trailing data after a valid FF D9 EOI. They decode correctly with ImageMagick, Pillow, browsers, and Obsidian, but current image.nvim rejects them because the EOI is not the final two bytes.

Removing this check also fixes rendering for these images on my setup.

So +1 for this fix. It might also be useful to add a regression test with a valid JPEG plus some trailing data to prevent this from coming back.

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