+1
-Phil.
On 3/5/20 7:47 PM, Jayathirth D v wrote:
Hi Phil,
Thanks for your inputs.
Yes we don’t need extra “left > 0” check, I have removed it and I have
updated the test case to print “Test passed”.
Please find updated webrev for review:
http://cr.openjdk.java.net/~jdv/6532025/webrev.01/
I ran all imageio jtreg tests and there are no failures with updated
change.
Thanks,
Jay
On 06-Mar-2020, at 8:22 AM, Philip Race <philip.r...@oracle.com
<mailto:philip.r...@oracle.com>> wrote:
I don't understand why you need to recheck that left > 0.
Nothing can change it between the while loop check and your check
while (left > 0) {
int nbytes = stream.read(block, off, left);
+ if (nbytes == -1 && left > 0) {
+ throw new IIOException("Invalid block
length for " +
+ "LZW encoded image data");
+ }
off += nbytes;
left -= nbytes;
}
Also in the test since you are printing
51 } catch (IIOException e) {
52 // do nothing we expect IIOException but we should not
53 // throw IndexOutOfBoundsException
54 System.out.println(e.toString());
55 System.out.println("Caught IIOException ignore it");
Maybe add "Test passed" to be extra clear !
-phil.
On 3/5/20, 6:15 PM, Jayathirth D v wrote:
Hi Brian,
Thanks for the clarification and approval.
@Others : Need one more review please take a look.
Regards,
Jay
On 06-Mar-2020, at 2:05 AM, Brian Burkhalter
<brian.burkhal...@oracle.com <mailto:brian.burkhal...@oracle.com>>
wrote:
Hi Jay,
I think the overall change is fine.
Regards,
Brian
On Mar 5, 2020, at 9:18 AM, Jayathirth D v
<jayathirth....@oracle.com <mailto:jayathirth....@oracle.com>> wrote:
Hello Brian,
Thanks for the review. GIF stream in regression test case would
match warn.gif stream behaviour that’s why it going into
GIFImageReader.getCode().
Are you okay with overall webrev.00 patch or have you just
approved GIFImageReader change? Please clarify.
Regards,
Jay
On 05-Mar-2020, at 10:20 PM, Brian Burkhalter
<brian.burkhal...@oracle.com
<mailto:brian.burkhal...@oracle.com>> wrote:
Hello Jay,
The source fix looks OK to me. I get the same exception as in the
bug description when I run the test against my unpatched local
JDK 15 build.
Thanks,
Brian
On Mar 5, 2020, at 2:12 AM, Jayathirth D v
<jayathirth....@oracle.com <mailto:jayathirth....@oracle.com>>
wrote:
Please review the following fix for JDK 15:
Bug : https://bugs.openjdk.java.net/browse/JDK-6532025
Webrev : http://cr.openjdk.java.net/~jdv/6532025/webrev.00/
<http://cr.openjdk.java.net/%7Ejdv/6532025/webrev.00/>
Root cause : When we have truncated GIF images, stream.read()
returns -1 but GIFImageReader doesn’t handle such conditions
properly and continues to read input stream data.
Solution : Handle cases where we reach EOF and throw appropriate
exception.
