Hi Martin,

yes, I agree that the reaching the length limit likely signifies that png header is corrupt. Our usual policy is to try to be tolerant to errors in the images. There are too many not very well formed images around and being too strict we may reject too much.
That's why I am inclined to avoid throwing exception.

However, we probably should be smarter here and read at most min(maxLengthFromSpec, restOfChunkData) bytes. This way we will be able to recover after single corrupted chunk
and start processing next chunk.

Thanks,
Andrew

Martin von Gagern wrote:
Hi Andrew,

Looking at the patch from the webrev you sent me via private email, and
comparing it to my previous bug6541476-corrections.patch, I have some
comments.

The addition of a maxLength parameter to readNullTerminatedString makes
sense, as it avoids some problems on malformed input. It is my
understanding, however, that all strings read via
readNullTerminatedString should in a well formed PNG actually be null
terminated. Therefore in my opinion reaching the limit should cause an
exception to be thrown, not simply return the string read so far.

There was a hunk in my patch changing the possible valies for
compressionFlag of an iTXtEntry from 1/0 to TRUE/FALSE. YOu moved this
change to your patch for 5082756. That's OK by me, but requires patches
to be applied in the corret order. That's the reason I had that change
bundled with my patch.

Greetings,
 Martin


Reply via email to