google / wuffs

Wrangling Untrusted File Formats Safely
Other
4.16k stars 131 forks source link

PNG decoder will not read trailing text chunks #58

Closed pjanx closed 2 years ago

pjanx commented 2 years ago

The state diagram doesn't seem to allow for this situation, and key-value-pairs.png.make-artificial.txt doesn't test it.

I've previously scanned all non-thumbnail PNGs that I keep around, and 845 out of 10748 have trailing text chunks (none of them have other kind of trailing chunks). The thumbnail specification I'm implementing doesn't say anything about metadata placement, and although Qt and gdk-pixbuf always write them at the beginning of a file, I should support both options.

Also, the WEBP specification, which is on the roadmap, says EXIF and XMP metadata should both be placed at the end of a file.

What API should be used to retrieve it?

nigeltao commented 2 years ago

What API should be used to retrieve it?

In the C API, wuffs_png__decoder__decode_image_config or wuffs_png__decoder__decode_frame_config will report metadata, as before. The difference is that you will now have to do so in a loop (instead of calling wuffs_png__decoder__decode_frame_config just once) if you want trailing metadata. The test/c/std/png.c change in google/wuffs@c5e41f0 has an example.

In the C++ API (which only decodes still images, or the first frame of animated images), wuffs_aux::DecodeImageCallbacks::HandleMetadata should now be called back for both leading and trailing metadata, not just leading metadata, provided that you've opted into e.g. wuffs_aux::DecodeImageArgFlags::REPORT_METADATA_KVP for textual metadata (key-value pairs).

pjanx commented 2 years ago

Thank you, I'll try to integrate this. Another beta would be nice to see soon.

nigeltao commented 2 years ago

I have just tagged v0.3.0-beta.15.

nigeltao commented 2 years ago

In case it wasn't obvious, you should be able to build and run the new script/print-image-metadata.cc program, pointing it at your PNG files, and it should pick up (leading and trailing) text chunks (marked as KVPK and KVPV for Key Value Pair {Key,Value}).

$ g++ script/print-image-metadata.cc
$ ./a.out test/data/hibiscus.primitive.png 
test/data/hibiscus.primitive.png
  PNG 
  KVPK
    53 6F 66 74 77 61 72 65 -- -- -- -- -- -- -- --    Software--------
  KVPV
    77 77 77 2E 69 6E 6B 73 63 61 70 65 2E 6F 72 67    www.inkscape.org
pjanx commented 2 years ago

Alright, my image viewer has finally stopped using Wuffs together with spng. *Tired "Yay!"*

nigeltao commented 2 years ago

my image viewer has finally stopped using Wuffs together with spng.

I assume that you don't mean "(stopped using Wuffs) and (stopped using spng)", but that you mean "stopped using (Wuffs together with spng), and now uses Wuffs only".

:-)