[OpenJDK 2D-Dev] [11] RFR JDK-8191073: JpegImageReader throws IndexOutOfBoundsException when trying to read image data from tables-only image

[Jayathirth D V]

Hello Brian,

 

Thanks for reviewing the changes.

 

I am posting updated webrev since I need one more review and approval  from 2d-dev list.

Updated webrev for review : http://cr.openjdk.java.net/~jdv/8191073/webrev.02/ 

 

Thanks,

Jay

 

From: Brian Burkhalter 
Sent: Tuesday, January 09, 2018 2:30 AM
To: Jayathirth D V
Cc: 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [11] RFR JDK-8191073: JpegImageReader throws IndexOutOfBoundsException when trying to read image data from tables-only image

 

Hi Jay,

 

Sorry but I have a few picky comments.

 

 377         // If imagePositions list doesn't contain any of the image stream

 378         // starting position(i.e tables-only image) we should not try to access

 379         // imagePositions.size() as it done below, because it will lead to

 380         // IndexOutOfBoundsException with index -1.

 

You might consider this verbiage instead:

 

If the image positions list is empty as in the case of a tables-only stream, then attempting to access the element at index

imagePositions.size() - 1 will cause an IndexOutOfBoundsException.

 

 381         if (seekForwardOnly && (!(imagePositions.isEmpty()))) {

 

I think this is more readable if some parentheses are eliminated:

 

 381         if (seekForwardOnly && !imagePositions.isEmpty()) {

 

 499         // We should not try to read image information from an input stream

 500         // which only contains tables-only(StreamMetadata) information.

 

You might consider this verbiage instead:

 

If the image positions list is empty as in the case of a tables-only stream, then no image data can be read.

 

No need to update the webrev.

 

Thanks,

 

Brian

 

On Jan 7, 2018, at 11:23 PM, Jayathirth D V <HYPERLINK "mailto:jayathirth.d.v at oracle.com"jayathirth.d.v at oracle.com> wrote:





I have removed the usage of tablesOnlyStream variable in code and updated the comment section.

Please find updated webrev for review:

http://cr.openjdk.java.net/~jdv/8191073/webrev.01/

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180109/0fcb510e/attachment-0001.html>