gifio, jpegio: a header ahead of its check, and lengths that overreport

[peterbay]

Code written by Claude Code, guided and corrected by @peterbay.

The problem

Five defects in gifio and jpegio: an argument read from the wrong place, a header written before the data behind it was checked, a buffer sized for the frames but not for the header that goes in first, and two length results that report more than happened.

The changes

• OnDiskGif read its filename from the wrong place. The constructor took all_args[0] rather than the parsed argument, so a fully keyword call handed it the keyword rather than the value: OnDiskGif(filename="/x.gif") tried to open a file called filename.

• GifWriter.add_frame wrote the frame header before checking the frame. Nineteen bytes — the graphic control extension and the image descriptor — went out ahead of the mp_get_index that bounds the source buffer, so a short frame raised but left its header behind. The check moves ahead of the header, where it also replaces the three per-branch checks that followed it.

• GifWriter's buffer was sized for a frame, not for the header. nblocks * 128 + 4 covers the block data, but the constructor first writes a fixed header into the same buffer: six bytes of signature, the screen descriptor, a 384 byte palette and the loop extension, 416 bytes in all. For a small frame that is more than the buffer holds — 14 by 9 pixels gives one block and an allocation of 132 bytes. write_data does not flush, it only asserts, and the assert is compiled out of a release build.

• jpegio's discard path reported the full length at end of stream. It returns what it actually read now, so the decoder is not told it passed over data that was not there.

• jpegio's two early exits compared a value with itself. Both tests subtracted the same rect origin from each side, so y2 < y1 reduced to lim.y2 < lim.y1 and x2 < x1 to lim.x2 < lim.x1, neither of which can hold for a valid limit. They compare against the block instead.

Testing

Seeed XIAO nRF52840 Sense, on two builds differing only by these changes.

	before	after
OnDiskGif(filename="/test.gif")	OSError: [Errno 2] No such file/directory: filename	opens it, 16x16
OnDiskGif("/test.gif") positionally	opens it, unchanged either way	opens it
a short frame rejected, then a good one, then deinit	720 bytes — nineteen of them the rejected frame's header	701 bytes
one good frame on its own	701 bytes, unchanged either way	701 bytes

The buffer size is arithmetic rather than a measurement: the same test prints that the constructor writes 416 bytes, and the allocation for a 14 by 9 frame is 1 * 128 + 4. Nothing visible happens on this board — the write goes into whatever the allocator handed out next and the test finished before anything read it back.

The two jpegio changes do not alter what comes out. The early exits are a short cut out of work that the copy loop below them clips away anyway, so the pixels are the same either way; a limit rect and a truncated file both decoded identically on the two builds. They are corrected because the tests as written cannot fire, not because the output was wrong.

No new translatable strings.

[peterbay]

Testing and diagnostic script.
gifio_jpegio_buffers_and_arguments.py

[tannewt]

Thank you!

[tannewt]

Do we have tests we can add for these fixes too?