Re: not enough compressed data

TL/DR: This happens because the progressive reader and the sequential
reader do not agree on what to do with Mozilla's test image. They
should agree.

Glenn Randers-Pehrson wrote:
> See https://bugzilla.mozilla.org/show_bug.cgi?id=1422694
> for the test case and the request.

I looked at it, and my understanding so far is that the error is
caused by an incomplete zlib stream, although the PNG pixel data is
completely present.

> Their test case has some other undelying bug.  There is sufficient
> data when read by the sequential reader  but comes up short
> when read progresively.   I'm not well enoguh to track that down
> right nowl

Their test case has been produced by an (accidental or deliberate)
faulty zlib encoder. Zlib streams are required to have an
end-of-stream code (Z_STREAM_END), and this one doesn't.

The progressive reader behaves differently from the sequential one.
The progressive reader pretends that the zlib end-of-stream code
exists when all pixel data is present. (See how PNG_FLAG_ZSTREAM_ENDED
is set inside png_process_IDAT_data.) The sequential reader doesn't do
such thing.

> One possibillity is t make the error benign only
> after reading all rows or mayb all but one.

I disagree with the "all but one" idea, but bailing out after reading
all row, all pixels, making both readers agree on that, is a
reasonable thing to do.

Here are the specifics of what we're currently having:

- The progressive reader sees spurious data coming after the last
pixel, and issues a warning "Extra compressed data in IDAT", without
caring what comes after. It could be another byte, or it could be
another gigabyte, with or without Z_STREAM_END, the progressive reader
doesn't care.

- The sequential reader performs decompression on all upcoming data,
all the way to the end, does not see Z_STREAM_END, and ends with a
severe error.

So the solution that I would personally consider desirable is not to
turn this error into a benign one, but to make the sequential reader
do what the progressive reader does: expect in the decompressor to see
Z_STREAM_END right after the last pixel, issue an "Extra compressed
data in IDAT" if the decompressor sees something else, and then stop
the decompression.

This particular kind of error is benign indeed, but needs not be
explicitly flagged as such.

One more observation: the sequential reader already does issue "Extra
compressed data" as a benign error (not as a warning, as in the case
of the progressive reader), but it does so after all zlib
decompression is performed (without stopping at the point of reading
all pixel data, as in the case of the progressive reader).

Tom Lane wrote:
>> ISTM that "not enough compressed data" necessarily has to be read as
>> "your image data is corrupt".  If that's a benign error, I have to
>> wonder what rates as not benign.

I agree.

>> Admittedly, since no context was provided for this question, I might
>> be missing something that makes it sensible in context.

Right. I believe this is a context where this sort of behaviour is
indeed sensible.

Sincerely,
Cosmin

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot