Thanks Sergey for the approval. Need one more review. Please review the webrev : http://cr.openjdk.java.net/~jdv/8211422/webrev.01/
Regards, Jay -----Original Message----- From: Sergey Bylokhov Sent: Thursday, November 15, 2018 10:58 PM 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 Looks fine. On 14/11/2018 04:35, Jayathirth D V wrote: > 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. > -- Best regards, Sergey.
