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