Hi Sergey, Thanks for the review.
I have updated the code to not move the changes out of switch. Apart from that I have refined comments to make it clear why we are not reading CRC for IEND chunk. Also chunkCRC value needs to be initialized because of this update, initialized value of chunkCRC will not be used anywhere in the logic. Please find updated webrev for review: http://cr.openjdk.java.net/~jdv/8211422/webrev.01/ Thanks, Jay -----Original Message----- From: Sergey Bylokhov Sent: Wednesday, November 14, 2018 4:31 AM To: Jayathirth D V; 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211422: Reading PNG with corrupt CRC for IEND chunk throws IIOException 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@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.
