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; [email protected]
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: [email protected]
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.