[OpenJDK 2D-Dev] [12] RFR JDK-8211422: Reading PNG with corrupt CRC for IEND chunk throws IIOException

[Sergey Bylokhov]

Hi, Jay.

Probably it will be better to not to change the code inside switch, and only add the check below
around of reading CRC:
   "if (chunkType != IEND_TYPE) {"

The current fix may throw "Invalid chunk length" when seek/flush the data for IEND_TYPE,
which is not correct.


On 12/11/2018 20:22, Jayathirth D V wrote:
> Hello All,
> 
> Gentle reminder for review.
> 
> Thanks,
> 
> Jay
> 
> *From:*Jayathirth Rao
> *Sent:* Tuesday, October 23, 2018 7:09 PM
> *To:* 2d-dev at openjdk.java.net
> *Subject:* [OpenJDK 2D-Dev] [12] RFR JDK-8211422: Reading PNG with corrupt CRC for IEND chunk throws IIOException
> 
> Hello All,
> 
> Please review the following fix in JDK12:
> 
> Bug : https://bugs.openjdk.java.net/browse/JDK-8211422
> 
> Webrev: http://cr.openjdk.java.net/~jdv/8211422/webrev.00/
> 
> Issue : When we try to read PNG image with corrupt/no 4 byte CRC data for IEND chunk we throw IIOException. We see this issue only after JDK-8164971 <https://bugs.openjdk.java.net/browse/JDK-8164971>.
> 
> Root cause : In JDK-8164971 <https://bugs.openjdk.java.net/browse/JDK-8164971> fix we made changes to continue reading metadata until we reach IEND chunk. Before JDK-8164971 <https://bugs.openjdk.java.net/browse/JDK-8164971> change we used to stop reading metadata as soon as we hit first IDAT chunk. According to PNG spec there can be more than one IDAT chunk/ more headers after IDAT chunk. So we need to read metadata until we hit IEND chunk. But in case of this bug we have IEND chunk but it has corrupt/no CRC chunk, so we throw IIOException(According to PNG spec every PNG chunk should contain 4 byte CRC). But lot of other decoders accept these kind of images which has no CRC chunk for IEND chunk.
> 
> Solution : There is no need to verify CRC for IEND chunk(Chunk data length for IEND is 0). Stop reading metadata once we hit Chunk type info for IEND chunk.
> 
> Thanks,
> 
> Jay
> 


-- 
Best regards, Sergey.