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.
