Reject critical chunks in APNG frame-data sequences - #903
Conversation
|
@ctruta, you need to upstream this to @svgeesus The base spec is unambiguous, a critical after IDAT is still an abort for the decoder, however that has always posed the problem that the decoder may well have displayed the whole image already! So the W3C spec needs to regularise the behaviour; for single images it did not matter a damn what was done in the post-IDAT phase but now there are many post-IDAT phases. In some cases this will damage existing decoders; decoders which delay operations or detectably fail post IDAT will be dinged. So detecting errors becomes not just optional but, in fact, criminal. PNG is presumptively sequential; the fix (@ProgramMax) is easy. The sequence points are the fcTls; therefore all the rules are suspended after the first correctly formed chunk beyond the last correctly formed IDAT and only resume at each fcTL. The current spec is ambiguous; for a single image no problem (it's a done deal, no errors, no warnings for ever). For APNG fcTL has to behave like IEND (or am I missing something?) All pending issues drop. The W3C has gone one foot of a mile here; fcTL and fdAT must occur in sequence otherwise there is an error. However this is too tight; rather at each fcTL the required sequence numbers (intervening fdAT) must all be present and, of course, correct. Warnings and errors must be handled at the end of each pseudo-IDAT segment; the first chunk after IDAT or the next fcTL. There is no reason or cause to handle other errors that occur before IEND; at that point a streaming decoder will have handled all the data it can. Of course a decoder may handle the errors or warnings, but such decoders will soon by abolished by popular approbation or lack thereof. |
Fixes #718.
Problem
While reading a non-default APNG frame, the sequential and progressive paths warn and skip every non-
fdATchunk encountered before the compressed frame stream ends. That is valid recovery for ancillary chunks, but it also acceptsIDAT,IHDR, and unknown critical chunks in the middle of anfdATsequence.Fix
Check the PNG ancillary bit at each APNG sequence skip site. Unknown ancillary chunks retain the existing warning-and-skip behavior; critical chunks now raise a read error. The existing, more specific
IENDerror remains unchanged where it was already present.The same rule is applied to the sequential frame-head/data paths and both progressive-reader paths so the APIs stay consistent.
Verification
I generated a two-frame 1x1 RGBA APNG whose second frame's zlib stream is split across two
fdATchunks, then inserted a zero-length chunk between them.ABCD:pngtestwarned three times, decoded the file, and exited 0.ABCD: all sequential/progressive passes reportUnexpected critical chunk in APNG sequence;pngtestexits 1.aBCD: the existing warning-and-skip behavior remains; all passes decode andpngtestexits 0.pngtestexits 0.make -f scripts/makefile.clang testpasses (9,782 zero samples).pngtestwere built with AddressSanitizer and UndefinedBehaviorSanitizer for these runs.git diff --checkpasses.