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

[Jayathirth D V]

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 at 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 at 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 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.
> 


--
Best regards, Sergey.