[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException in PNGImageReader after JD
From:       Krishna Addepalli <krishna.addepalli () oracle ! com>
Date:       2018-11-20 8:50:25
Message-ID: 4db789c9-1753-43f2-9c77-b600900d0d58 () default
[Download RAW message or body]

+1

Thanks,
Krishna

-----Original Message-----
From: Jayathirth D V 
Sent: Tuesday, November 20, 2018 1:59 PM
To: 2d-dev <2d-dev@openjdk.java.net>
Subject: Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException in \
PNGImageReader after JDK-6788458

Thanks Sergey for the review.
I need one more review.
Please review the latest webrev:
http://cr.openjdk.java.net/~jdv/8211795/webrev.01/ 

Thanks,
Jay

-----Original Message-----
From: Sergey Bylokhov 
Sent: Saturday, November 17, 2018 3:35 AM
To: Jayathirth D V; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException in \
PNGImageReader after JDK-6788458

Looks fine.

On 16/11/2018 02:16, Jayathirth D V wrote:
> Hi Sergey,
> 
> As discussed offline I did more analysis on whether we can use common variable to \
> determine number of bands. Since we have "outputSampleSize.length - 1" and \
> "inputBands + 1" kind of things. 
> Actually scale array will be used on input data(ps[]), so we should use input bands \
> value to create and use scale array. Before JDK-6788458 there was no difference \
> between input bands and output bands so we didn't see any issue. But after \
> JDK-6788458 number of bands data is different between input and output data for \
> PNG_COLOR_RGB/GRAY having tRNS chunk. 
> So I have made change to use inputBands data for creation and use of scale array. \
> Ran complete imageio jtreg and JCK tests there are no failures. Please find updated \
> webrev for review: http://cr.openjdk.java.net/~jdv/8211795/webrev.01/
> 
> Thanks,
> Jay
> 
> -----Original Message-----
> From: Jayathirth D V
> Sent: Wednesday, November 14, 2018 1:09 PM
> To: Sergey Bylokhov; 2d-dev
> Subject: RE: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException \
> in PNGImageReader after JDK-6788458 
> Hi Sergey,
> 
> Thanks for the review.
> 
> As you pointed out, yes the AIOOB is happening for ps[b]. I have updated the \
> analysis in JBS bug also. 
> Basically the calculation of numBands is not proper because we take numBands value \
> from destination image. This destination image will have extra alpha channel for \
> Gray or RGB input data(ps[]) which will throw AIOOB. 
> So we need to update the logic of how we calculate numBands different for PNG \
> Gray/RGB image havng tRNS chunk. Fortunately, webrev.00 is actually doing this job. \
>  Regarding whether we need to change scale array logic : We expect first 3 channel \
> to be RGB and first channel to be Gray for PNG_COLOR_RGB and PNG_COLOR_GRAY \
> respectively. So just updating numBands information will create proper scale array. \
> So there is no need to change the scale array logic. 
> History JDK-6788458 : Toolkit was able to show transparent color information for \
> RGB/Gray PNG image when it has tRNS chunk, but ImageIO didn't support it. To use \
> tRNS data and show transparent color in output image we needed to add extra alpha \
> channel for PNG RGB/Gray image with tRNS chunk. But fix present in JDK-6788458 \
> didn't handle the case where bitDepth adjustment is needed and we are using band \
> information from output image(having extra alpha channel) on input image which has \
> no alpha channel. Change in numBands logic for this bug fixes that issue. 
> 
> Thanks,
> Jay
> 
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Wednesday, November 14, 2018 4:07 AM
> To: Jayathirth D V; 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException \
> in PNGImageReader after JDK-6788458 
> Hi, Jay.
> 
> Can you please provide some more detail about this bug.
> 
> > Root cause : In JDK-6788458 we are adding extra alpha channel for destination \
> > whenever we have tRNS chunk. But the number of bands in bitDepth scale array was \
> > not changed when we have tRNS chunk. This is causing \
> > ArrayIndexOutOfBoundsException for scale array.
> 
> As far as I understand the AIOOB is occurred when we access ps[b] at line 1308 not \
> when we access the scale array, because the scale array is created as "scale = new \
> int[numBands][]".  So maybe numBands should depends on the passRow? or the creation \
> of scale[][xxx] should be updated? BTW this code uses +1/-1 in a lot of places \
> already, and it is not always clear why. 
> 


-- 
Best regards, Sergey.


[prev in list] [next in list] [prev in thread] [next in thread]