Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 507172 - sys-apps/portage: Include QA check for broken .png image files
Summary: sys-apps/portage: Include QA check for broken .png image files
Status: RESOLVED FIXED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Enhancement/Feature Requests (show other bugs)
Hardware: All All
: Low enhancement (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-08 20:45 UTC by Samuli Suominen (RETIRED)
Modified: 2014-08-13 08:57 UTC (History)
4 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
Add a QA check using pngfix (0001-Use-pngfix-to-find-broken-PNG-files.patch,1.14 KB, patch)
2014-04-09 12:08 UTC, Michał Górny
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Samuli Suominen (RETIRED) gentoo-dev 2014-04-08 20:45:26 UTC
See, http://bugs.gentoo.org/show_bug.cgi?id=466190#c11

QA: "/usr/share/pixmaps/foobar.png: libpng error: IDAT: invalid distance too far back, report to http://bugzilla.gentoo.org/"

I'm not very versed with python, unfortunately, the original script is written by Tobias Klausmann, migration is required like for paths, and behaving nicely (be a no-op) when convert binary is not installed

Plus there should be a way to disable the check for performance

It's the `pngfix` binary that comes with libpng16 (like, latest) that can fix them.
Comment 1 Samuli Suominen (RETIRED) gentoo-dev 2014-04-08 20:48:16 UTC
(In reply to Samuli Suominen from comment #0)
> performance

as for performance, I haven't actually timed it, it's propably very quick anyways
Comment 2 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-04-08 21:09:32 UTC
I'd say spawning 'convert' to check that kinda sucks. Do you think this could be detected without implementing too much of PNG support?
Comment 3 Tobias Klausmann (RETIRED) gentoo-dev 2014-04-09 07:32:20 UTC
I can change the original script for more options and higher resilience. Just let me know what kind of interface/commandline would be best.

AFAICT, hacking up a PNG reader that can detect the broken PNGs reliably is non-trivial. 

We could go for blindly fixing all PNGs unless the package has opted out by setting a variable.

I originally used convert to check for brokenness because, at the time, it was the first binary I could find that reliably detected the bug.
Comment 4 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-04-09 07:49:53 UTC
The point is not to have a script with some kind of interface, but a plain bit of code that could easily incorporated in portage post-merge checks and that has almost no external dependencies.

Could you point me to some small known-still-to-be-broken package so that I could test this?
Comment 5 Francesco Riosa 2014-04-09 09:28:42 UTC
IDAT is the image data compressed with zlib.
"invalid distance too far back" error come from zlib indeed.

a checker could be built using "The Python Standard Library" extracting the IDATs from the .png files and trying to decompress them (to /dev/null)

the non trivial part may be to locate the IDATs in the file, since it seem to require knowledge of how the various components of a png files ar packet togheder.
Comment 6 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-04-09 09:36:28 UTC
media-libs/libpng:0 installs a 'pngfix' tool.

$ pngfix --help
[...]
EXIT CODES
  *** SUBJECT TO CHANGE ***
  The program exit code is value in the range 0..127 holding a bit mask of
  the following codes.  Notice that the results for each file are combined
  together - check one file at a time to get a meaningful error code!
    0x01: The zlib too-far-back error existed in at least one chunk.


I think the simplest solution would be to run it on all installed .png files. Since 'by default nothing is written' it will just check the files for conformance and return with exit code for portage processing.
Comment 7 Francesco Riosa 2014-04-09 10:59:02 UTC
(In reply to Michał Górny from comment #6)
> I think the simplest solution would be to run it on all installed .png
> files. Since 'by default nothing is written' it will just check the files
> for conformance and return with exit code for portage processing.

wouldn't this bring any sort of problems with cross-compiling et friends?
But the pngfix.c file look indeed complex and not easy to translate in pyton.

may i suggest to make this an opt-in (maybe with dedicated eclass) thing?

There are also png which should NOT be fixed, mainly used for test purpose:


IDAT ERR 0c read Success Success damaged_PNG_stream /usr/lib/go/src/pkg/image/png/testdata/invalid-noend.png

IDAT ERR 00 zlib Success Success could_not_uncompress_IDAT /usr/lib/go/src/pkg/image/png/testdata/invalid-zlib.png

IEND ERR 0c read Success Success damaged_PNG_stream /usr/lib/go/src/pkg/image/png/testdata/invalid-trunc.png

IDAT ERR 0c read Success Success damaged_PNG_stream /usr/share/doc/gd-2.0.35-r3/html/tests/png/bug00033.png

!is! ERR 00 libpng Success Success not_a_PNG_(signature) /usr/share/doc/qt-4.8.5/src/images/conceptprocessor.png
Comment 8 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-04-09 12:08:51 UTC
Created attachment 374596 [details, diff]
Add a QA check using pngfix

Please try this patch. It seems not to break anything but I can't find any broken package on my system so I didn't test it on known-broken file.
Comment 9 Francesco Riosa 2014-04-09 14:39:27 UTC
for what it count I've tested it with adding an ERR use case and it identify and warn about broken png (for example invalid-noend.png)

QA Notice: broken .png files found:
   /usr/lib/go/src/pkg/image/png/testdata/invalid-trunc.png: broken ERR

it also leave the files unmodified (which is good)

neither my system has broken TFB files
Comment 10 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-04-09 16:54:42 UTC
(In reply to Francesco Riosa from comment #9)
> for what it count I've tested it with adding an ERR use case and it identify
> and warn about broken png (for example invalid-noend.png)
> 
> QA Notice: broken .png files found:
>    /usr/lib/go/src/pkg/image/png/testdata/invalid-trunc.png: broken ERR

I've specifically omitted that to avoid the issues you listed :). I'd assume we aim for catching the specific libpng bug and not doing a catch-all for mis-formatted files.
Comment 11 Francesco Riosa 2014-04-09 23:26:09 UTC
(In reply to Michał Górny from comment #10)
> (In reply to Francesco Riosa from comment #9)
> > for what it count I've tested it with adding an ERR use case and it identify
> > and warn about broken png (for example invalid-noend.png)
> > 
> > QA Notice: broken .png files found:
> >    /usr/lib/go/src/pkg/image/png/testdata/invalid-trunc.png: broken ERR
> 
> I've specifically omitted that to avoid the issues you listed :). I'd assume
> we aim for catching the specific libpng bug and not doing a catch-all for
> mis-formatted files.

sure, should have explained myself better;
I've added the ERR check to "test" the behaviour of the procedure, NOT because it should be added.

there is still one thing, pngfix will change the file in place if it found a correctable error, is this ok?
otherwise it could be forced to write to a temp file
Comment 12 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-04-21 07:55:47 UTC
Any guide or article we could point developers to in the message?
Comment 13 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-08-13 08:57:42 UTC
This has been included in 2.2.12.