Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-30 Thread Jayathirth D V 
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- li

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-24 Thread Philip Race 

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/~pnarayana

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-24 Thread Prahalad Kumar Narayanan 
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

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-23 Thread Prahalad Kumar Narayanan 
Hello Brian

First, I should thank you for sharing the link. 

. Since the code changes pertain to one particular format of BMP, I ' did not ' 
use the entire BMP image set to test.
. I used the 3 types of RLE4 compressed images present in the suite- valid, 
questionable & corrupt.
  . Having put through this test, I believe, the code is stable now

Thank you
Have a good day

Prahalad N.

--- Original Message ---
From: Brian Burkhalter 
Sent: Tuesday, January 24, 2017 1:16 AM
To: Prahalad Kumar Narayanan
Cc: Philip Race; 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: 
ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 
BMP

Hi Prahalad,

Out of curiosity, did you test against all BMP images in the suite, or only the 
ones which are RLE4-compressed?

Thanks,

Brian

On Jan 23, 2017, at 6:24 AM, Prahalad Kumar Narayanan 
<prahalad.kumar.naraya...@oracle.com> wrote:


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

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-23 Thread Phil Race 

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.burkhal...@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 add

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-23 Thread Brian Burkhalter 
Hi Prahalad,

Out of curiosity, did you test against all BMP images in the suite, or only the 
ones which are RLE4-compressed?

Thanks,

Brian

On Jan 23, 2017, at 6:24 AM, Prahalad Kumar Narayanan 
 wrote:

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

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-23 Thread Prahalad Kumar Narayanan 
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.burkhal...@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

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-20 Thread Brian Burkhalter 
On Jan 20, 2017, at 3:04 PM, Philip Race  wrote:
> . Note: I couldn't create a suitable image with Delta 
> encoding in image buffer. Hence this small change could not be tested.
Have you looked at image test suites such as 
http://bmptestsuite.sourceforge.net/?

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

Brian

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-20 Thread Philip Race 

 ii. Handling of Delta encoding (0x00 0x02 xOffset yOffset)
. The scanline of decoded image should be copied to 
destination if yOff shifts next pixel's location to other line
. Secondly, the scanline index should be limited by 
boundary value after x and y offsets are added.
. Note: I couldn't create a suitable image with Delta 
encoding in image buffer. Hence this small change could not be tested.

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


Why "othervm" for the test ? I don't see anything the test does that 
requires this

and it just slows down the test harness.

-phil.

On 1/20/17, 1:09 AM, Prahalad Kumar Narayanan wrote:

Hello Jay

Thank you for your time in review.

I 've incorporated review suggestions and the modified code is available for 
review under:
Link: http://cr.openjdk.java.net/~pnarayanan/8167278/webrev.01/

Quick info on the changes from previous revision:
1. Line stride calculation: This has been moved into the 'else if' section as 
suggested..
2. Typo error: At Line 1670 has been corrected
3. Un-used argument value- 'padding', in decodeRLE4 function()


Not using this padding parameter will cause any problems while decoding?

   Line 1547: mentions - If width is not 32bit aligned then while 
un-compressing each scanline will have padding bytes.
   The above comment (and thus padding value) is not applicable to ' 
current ' RLE4 decoding logic because,
 . The logic creates an intermediate scanline array exactly of 
size -width.
 . Besides, when the intermediate scanline is flushed to the 
destination, the logic assumes the destination is also of type- 
MultiPixelPackedSampleModel.


If it is not needed we can remove "padding" parameter in decodeRLE4() and its 
calculation in readRLE4().
If it is not under the scope of this bug you can raise a new bug for the same.

   Yes - The padding is not applicable for current logic, it could be 
removed.
   I have not removed the variable because it could help in fixing this 
bug- JDK-8172696 in future
[JDK-8172696]  ClassCastException is thrown while decoding 
RLE4 Bitmap with a destination BufferedImage set in ImageReadParams

Kindly review the new changes at your convenience.

Thank you for your time in review
Have a good day

Prahalad N.


-Original Message-
From: Jayathirth D V
Sent: Thursday, January 19, 2017 12:36 PM
To: Prahalad Kumar Narayanan; 2d-dev@openjdk.java.net
Subject: RE: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: 
ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 
BMP

Hi Prahalad,

Please find my observations :

1) Since you have moved calculation of "lineStride" to different function I think we can optimize 
it more by moving the calculation of lineStride inside the "else if ((lineNo - sourceRegion.y) % scaleY 
== 0)" because it is not used in "if (noTransform)" case.

2) Also there is small typo at line 1670 "// REL4 decoding" and please remove 
trailing spaces in test case before pushing.

Apart from these things changes are working fine.

Also I noted that we are not using "padding" parameter  passed to decodeRLE4() 
function.
Not using this padding parameter will cause any problems while decoding?

If it is not needed we can remove "padding" parameter in decodeRLE4() and its 
calculation in readRLE4().If it is not under the scope of this bug you can raise a new 
bug for the same.

Thanks,
Jay

-Original Message-
From: Prahalad Kumar Narayanan
Sent: Friday, January 13, 2017 2:54 PM
To: 2d-dev@openjdk.java.net
Subject: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException 
when calling ImageIO.read(InputStream) with RLE4 BMP

Hello Everyone

Good day to you.

Request your time in reviewing the fix for bug
 . [JDK-8167278] : ArrayIndexOutOfBoundsException when calling 
ImageIO.read(InputStream) with RLE4 BMP

Root Cause
 . The issue seems to stem from a mal-formed RLE4 Encoded Bitmap Image
 . Specifically - the width as mentioned in header and encoded image data 
do not match.
 . Unfortunately, the decoder logic doesn't contain guard checks to prevent 
out of bounds array access.

Fix Details:
 . Necessary guard conditions have been put to the RLE4 bitmap decoding 
logic.
 . Besides, two new issues were observed in same function block. They have 
been

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-20 Thread Jayathirth D V 
Hi Prahalad,

Changes are fine.

Thanks,
Jay

-Original Message-
From: Prahalad Kumar Narayanan 
Sent: Friday, January 20, 2017 2:39 PM
To: Jayathirth D V; 2d-dev@openjdk.java.net
Subject: RE: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: 
ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 
BMP

Hello Jay

Thank you for your time in review.

I 've incorporated review suggestions and the modified code is available for 
review under:
Link: http://cr.openjdk.java.net/~pnarayanan/8167278/webrev.01/

Quick info on the changes from previous revision:
1. Line stride calculation: This has been moved into the 'else if' section as 
suggested..
2. Typo error: At Line 1670 has been corrected 3. Un-used argument value- 
'padding', in decodeRLE4 function()

> Not using this padding parameter will cause any problems while decoding?
  Line 1547: mentions - If width is not 32bit aligned then while 
un-compressing each scanline will have padding bytes.
  The above comment (and thus padding value) is not applicable to ' 
current ' RLE4 decoding logic because, 
. The logic creates an intermediate scanline array exactly of 
size -width. 
. Besides, when the intermediate scanline is flushed to the 
destination, the logic assumes the destination is also of type- 
MultiPixelPackedSampleModel.

> If it is not needed we can remove "padding" parameter in decodeRLE4() and its 
> calculation in readRLE4().
> If it is not under the scope of this bug you can raise a new bug for the same.
  Yes - The padding is not applicable for current logic, it could be 
removed.
  I have not removed the variable because it could help in fixing this 
bug- JDK-8172696 in future
   [JDK-8172696]  ClassCastException is thrown while decoding 
RLE4 Bitmap with a destination BufferedImage set in ImageReadParams

Kindly review the new changes at your convenience.

Thank you for your time in review
Have a good day

Prahalad N.


-Original Message-
From: Jayathirth D V
Sent: Thursday, January 19, 2017 12:36 PM
To: Prahalad Kumar Narayanan; 2d-dev@openjdk.java.net
Subject: RE: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: 
ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 
BMP

Hi Prahalad,

Please find my observations :

1) Since you have moved calculation of "lineStride" to different function I 
think we can optimize it more by moving the calculation of lineStride inside 
the "else if ((lineNo - sourceRegion.y) % scaleY == 0)" because it is not used 
in "if (noTransform)" case.

2) Also there is small typo at line 1670 "// REL4 decoding" and please remove 
trailing spaces in test case before pushing.

Apart from these things changes are working fine.

Also I noted that we are not using "padding" parameter  passed to decodeRLE4() 
function. 
Not using this padding parameter will cause any problems while decoding?

If it is not needed we can remove "padding" parameter in decodeRLE4() and its 
calculation in readRLE4().If it is not under the scope of this bug you can 
raise a new bug for the same.

Thanks,
Jay

-Original Message-
From: Prahalad Kumar Narayanan
Sent: Friday, January 13, 2017 2:54 PM
To: 2d-dev@openjdk.java.net
Subject: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException 
when calling ImageIO.read(InputStream) with RLE4 BMP

Hello Everyone

Good day to you.

Request your time in reviewing the fix for bug
. [JDK-8167278] : ArrayIndexOutOfBoundsException when calling 
ImageIO.read(InputStream) with RLE4 BMP 

Root Cause
. The issue seems to stem from a mal-formed RLE4 Encoded Bitmap Image
. Specifically - the width as mentioned in header and encoded image data do 
not match.
. Unfortunately, the decoder logic doesn't contain guard checks to prevent 
out of bounds array access.

Fix Details:
. Necessary guard conditions have been put to the RLE4 bitmap decoding 
logic.
. Besides, two new issues were observed in same function block. They have 
been addressed as well.
 i. The last scanline of decoded image is missed in destination 
image (when EoF sequence arrives without EoL)
. This was observed with a sample image created with gimp 
tool.
 ii. Handling of Delta encoding (0x00 0x02 xOffset yOffset)
. The scanline of decoded image should be copied to 
destination if yOff shifts next pixel's location to other line
. Secondly, the scanline index should be limited by 
boundary value after x and y offsets are added.
. Note: I couldn't create a suitable image with Delta 
encoding in image buffer. Hence this small change could not be tested.

Other Details:
. The fix was run through both Jtreg and JCK test suites. No regressions 
were seen.

The changes are available for your

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-20 Thread Prahalad Kumar Narayanan 
Hello Jay

Thank you for your time in review.

I 've incorporated review suggestions and the modified code is available for 
review under:
Link: http://cr.openjdk.java.net/~pnarayanan/8167278/webrev.01/

Quick info on the changes from previous revision:
1. Line stride calculation: This has been moved into the 'else if' section as 
suggested..
2. Typo error: At Line 1670 has been corrected
3. Un-used argument value- 'padding', in decodeRLE4 function()

> Not using this padding parameter will cause any problems while decoding?
  Line 1547: mentions - If width is not 32bit aligned then while 
un-compressing each scanline will have padding bytes.
  The above comment (and thus padding value) is not applicable to ' 
current ' RLE4 decoding logic because, 
. The logic creates an intermediate scanline array exactly of 
size -width. 
. Besides, when the intermediate scanline is flushed to the 
destination, the logic assumes the destination is also of type- 
MultiPixelPackedSampleModel.

> If it is not needed we can remove "padding" parameter in decodeRLE4() and its 
> calculation in readRLE4().
> If it is not under the scope of this bug you can raise a new bug for the same.
  Yes - The padding is not applicable for current logic, it could be 
removed.
  I have not removed the variable because it could help in fixing this 
bug- JDK-8172696 in future
   [JDK-8172696]  ClassCastException is thrown while decoding 
RLE4 Bitmap with a destination BufferedImage set in ImageReadParams

Kindly review the new changes at your convenience.

Thank you for your time in review
Have a good day

Prahalad N.


-Original Message-
From: Jayathirth D V 
Sent: Thursday, January 19, 2017 12:36 PM
To: Prahalad Kumar Narayanan; 2d-dev@openjdk.java.net
Subject: RE: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: 
ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 
BMP

Hi Prahalad,

Please find my observations :

1) Since you have moved calculation of "lineStride" to different function I 
think we can optimize it more by moving the calculation of lineStride inside 
the "else if ((lineNo - sourceRegion.y) % scaleY == 0)" because it is not used 
in "if (noTransform)" case.

2) Also there is small typo at line 1670 "// REL4 decoding" and please remove 
trailing spaces in test case before pushing.

Apart from these things changes are working fine.

Also I noted that we are not using "padding" parameter  passed to decodeRLE4() 
function. 
Not using this padding parameter will cause any problems while decoding?

If it is not needed we can remove "padding" parameter in decodeRLE4() and its 
calculation in readRLE4().If it is not under the scope of this bug you can 
raise a new bug for the same.

Thanks,
Jay

-Original Message-
From: Prahalad Kumar Narayanan 
Sent: Friday, January 13, 2017 2:54 PM
To: 2d-dev@openjdk.java.net
Subject: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException 
when calling ImageIO.read(InputStream) with RLE4 BMP

Hello Everyone

Good day to you.

Request your time in reviewing the fix for bug
. [JDK-8167278] : ArrayIndexOutOfBoundsException when calling 
ImageIO.read(InputStream) with RLE4 BMP 

Root Cause
. The issue seems to stem from a mal-formed RLE4 Encoded Bitmap Image
. Specifically - the width as mentioned in header and encoded image data do 
not match.
. Unfortunately, the decoder logic doesn't contain guard checks to prevent 
out of bounds array access.

Fix Details:
. Necessary guard conditions have been put to the RLE4 bitmap decoding 
logic.
. Besides, two new issues were observed in same function block. They have 
been addressed as well.
 i. The last scanline of decoded image is missed in destination 
image (when EoF sequence arrives without EoL)
. This was observed with a sample image created with gimp 
tool.
 ii. Handling of Delta encoding (0x00 0x02 xOffset yOffset)
. The scanline of decoded image should be copied to 
destination if yOff shifts next pixel's location to other line
. Secondly, the scanline index should be limited by 
boundary value after x and y offsets are added.
. Note: I couldn't create a suitable image with Delta 
encoding in image buffer. Hence this small change could not be tested.

Other Details:
. The fix was run through both Jtreg and JCK test suites. No regressions 
were seen.

The changes are available for your review under: 
Link: http://cr.openjdk.java.net/~pnarayanan/8167278/webrev.00/

Kindly review the changes at your convenience and share your feedback.

Thank you for your time in review
Have a good day

Prahalad N.

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException when calling ImageIO.read(InputStream) with RLE4 BMP

2017-01-18 Thread Jayathirth D V 
Hi Prahalad,

Please find my observations :

1) Since you have moved calculation of "lineStride" to different function I 
think we can optimize it more by moving the calculation of lineStride inside 
the "else if ((lineNo - sourceRegion.y) % scaleY == 0)" because it is not used 
in "if (noTransform)" case.

2) Also there is small typo at line 1670 "// REL4 decoding" and please remove 
trailing spaces in test case before pushing.

Apart from these things changes are working fine.

Also I noted that we are not using "padding" parameter  passed to decodeRLE4() 
function. 
Not using this padding parameter will cause any problems while decoding?

If it is not needed we can remove "padding" parameter in decodeRLE4() and its 
calculation in readRLE4().If it is not under the scope of this bug you can 
raise a new bug for the same.

Thanks,
Jay
-Original Message-
From: Prahalad Kumar Narayanan 
Sent: Friday, January 13, 2017 2:54 PM
To: 2d-dev@openjdk.java.net
Subject: [OpenJDK 2D-Dev] [9] RFR: JDK-8167278: ArrayIndexOutOfBoundsException 
when calling ImageIO.read(InputStream) with RLE4 BMP

Hello Everyone

Good day to you.

Request your time in reviewing the fix for bug
. [JDK-8167278] : ArrayIndexOutOfBoundsException when calling 
ImageIO.read(InputStream) with RLE4 BMP 

Root Cause
. The issue seems to stem from a mal-formed RLE4 Encoded Bitmap Image
. Specifically - the width as mentioned in header and encoded image data do 
not match.
. Unfortunately, the decoder logic doesn't contain guard checks to prevent 
out of bounds array access.

Fix Details:
. Necessary guard conditions have been put to the RLE4 bitmap decoding 
logic.
. Besides, two new issues were observed in same function block. They have 
been addressed as well.
 i. The last scanline of decoded image is missed in destination 
image (when EoF sequence arrives without EoL)
. This was observed with a sample image created with gimp 
tool.
 ii. Handling of Delta encoding (0x00 0x02 xOffset yOffset)
. The scanline of decoded image should be copied to 
destination if yOff shifts next pixel's location to other line
. Secondly, the scanline index should be limited by 
boundary value after x and y offsets are added.
. Note: I couldn't create a suitable image with Delta 
encoding in image buffer. Hence this small change could not be tested.

Other Details:
. The fix was run through both Jtreg and JCK test suites. No regressions 
were seen.

The changes are available for your review under: 
Link: http://cr.openjdk.java.net/~pnarayanan/8167278/webrev.00/

Kindly review the changes at your convenience and share your feedback.

Thank you for your time in review
Have a good day

Prahalad N.