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

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(
From:       Jayathirth D V <jayathirth.d.v () oracle ! com>
Date:       2017-01-30 10:52:59
Message-ID: 200e0374-47a1-435c-b4fe-9bea3f586db9 () default
[Download RAW message or body]

Hi Prahalad,

Changes are fine.

Thanks,
Jay

-----Original Message-----
From: Philip Race 
Sent: Wednesday, January 25, 2017 6:48 AM
To: Prahalad Kumar Narayanan
Cc: 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException \
when calling ImageIO.read(InputStream) with RLE4 BMP

OK .. +1

-phil.

On 1/24/17, 12:57 AM, Prahalad Kumar Narayanan wrote:
> Hello Phil
> 
> I agree with you in your observation. We cannot include the image in the test case.
> 
> This morning, I created many RLE4 bitmap images using Gimp. But none of the created \
> images contained Delta sequence /or corrupt image data to cause \
> ArrayIndexOutOfBounds Exception. Henceforth, I 've removed the test case from the \
> proposed fix. I 've also substantiated the reasons in JBS for not including a test \
> case. 
> The new webrev without test-case is available under
> Link: http://cr.openjdk.java.net/~pnarayanan/8167278/webrev.03/
> 
> Kindly review at your convenience and share the feedback.
> 
> Thank you for your time
> Have a good day
> 
> Prahalad N.
> 
> 
> -----Original Message-----
> From: Phil Race
> Sent: Tuesday, January 24, 2017 2:06 AM
> To: Prahalad Kumar Narayanan; Brian Burkhalter
> Cc: 2d-dev@openjdk.java.net
> Subject: Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: 
> ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) 
> with RLE4 BMP
> 
> I just noticed something. The bug report says :-
> 
> > However, I have a public web application that allows users to upload images,
> > and I was surprised to find a failure caused by an unexpected
> > ArrayIndexOutOfBoundsException when a user uploaded an apparently-valid RLE4 BMP \
> > file. The attached code contains this BMP file, as a byte array.
> 
> This means the submitter of this bug report almost certainly does not own this \
> image ! So we cannot include it in the test to be checked in no matter how encoded.
> 
> Thus you will need to create your own RLE4 encoded BMP.
> If you can't do that then we won't be able to check in a test.
> 
> -phil.
> 
> On 01/23/2017 06:24 AM, Prahalad Kumar Narayanan wrote:
> > Hello Phil&  Brian
> > 
> > Thank you for your time in review and feedback.
> > 
> > . Testing with bmptestsuite
> > . The test suite came handy to test the changes and confirm our approach.
> > . The test suite has a collection of RLE4 compressed images that are- valid, \
> >                 questionable and corrupt.
> > . The collection includes image with Delta sequence as well.
> > 
> > . Handling of Delta sequence (0x00 0x02 xOffset yOffset)
> > . The decodeRLE4(...) function deploys line-by-line decoding of compressed bitmap \
> >                 image.
> > . Until decoding of one row (or line) of pixels is complete, the values are \
> >                 stored in intermediate scanline buffer : val.
> > . Upon completion of decoding one row of pixels (ie., with EoL, EoF sequence ), \
> > contents of val are copied to destination image's buffer. 
> > . Declaration of val buffer is given as
> > . Line 1619:                    byte[] val = new byte[width];
> > 
> > . As we see, the intermediate scanline array ' val ' is of size : width ( Not- \
> >                 height x width )
> > . Thus the offset addition to ' l ', in delta sequence will cause index out of \
> >                 bounds if accumulated offset goes beyond size of ' val ' buffer.
> > . Hence the new expression at Line 1691 that limits the offset within the \
> >                 capacity of the buffer- val.
> > . Line 1690:                     l += xoff + yoff*width;
> > . Line 1691:                     l %= width;
> > 
> > . If the yOffset shifts the decoding to another line, we should ensure to copy \
> >                 the decoded row to destination bitmap.
> > . Failing to do so, will cause the current row to be missed on the destination \
> >                 image.
> > . This is achieved with the new set of lines.
> > . Line 1677:                      if (yoff != 0) {
> > Line 1678:                              // Copy the decoded scanline to \
> > destination Line 1679:                              if \
> > (copyRLE4ScanlineToDst(lineNo, val, bdata)) { 
> > . When tested with bmptestsuite's rle4 images, Delta sequence 
> > handling required two additional changes (mentioned in 3.a and 3.b)
> > 
> > 3. Changes from previous webrev
> > 3.a. Proper update to the variable- lineNo
> > . After handling a delta sequence (xOffset yOffsest), the variable- lineNo must \
> >                 be updated correspondingly
> > . Reason: lineNo is used to locate exact row on Destination buffer to store \
> >                 intermediate scanline.
> > . Line 1684:                       lineNo += isBottomUp ? -yOffset : yOffset;
> > 
> > 3.b. Clearing of intermediate scanline buffer before starting to decode new \
> >                 row/line.
> > . Ensures the previous row's decoded pixels do not appear on current row
> > 3.c. Minor change to the condition in the while loop to ensure sufficient data is \
> > available before every iteration. 3.d. Removal of main/othervm from the jtreg \
> > comments in test file. 
> > 4. The build with changes was run through Jreg and JCK tests. No regressions were \
> >                 seen.
> > . In addition, the build works well for all input RLE4 
> > bitmap images from the test suite
> > 
> > The changes are available for review in the link:
> > http://cr.openjdk.java.net/~pnarayanan/8167278/webrev.02/
> > 
> > Kindly review the changes at your convenience&  share your feedback.
> > 
> > Thank you once again
> > Have a good day
> > 
> > Prahalad N.
> > 
> > --- Original message ---
> > From: Brian Burkhalter
> > Sent: Saturday, January 21, 2017 6:05 AM
> > To: Philip Race
> > Cc: 2d-dev@openjdk.java.net; Prahalad Kumar Narayanan
> > Subject: Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278:
> > ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) 
> > with RLE4 BMP
> > 
> > On Jan 20, 2017, at 4:30 PM, Brian Burkhalter<brian.burkhalter@oracle.com>  \
> > wrote: 
> > 
> > That is worrying me since I don't follow these lines are part of
> > that:-
> > 
> > 1684                     // Move to the position (xoff, yoff). Since l-is used
> > 1685                     // to index into the scanline buffer, the calculation
> > 1686                     // must be limited by the size
> > 1687                     l += xoff + yoff*width;
> > 1688                     l %= width;
> > 1687 was already there but 1688 and the comment are new and 1688 
> > looks wrong to me as it would seem to throw away the y it just added in ...
> > 
> > Indeed, if xoff is in the half-closed interval [0,width), then (xoff + \
> > yoff*width) % width == xoff. 
> > This does not however account for the accumulation into "l" which might negate my \
> > observation. 
> > Brian


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