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