[OpenJDK 2D-Dev] [15] RFR JDK-6532025: GIF reader throws misleading exception with truncated images

[Philip Race]

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.burkhalter at oracle.com <mailto:brian.burkhalter at 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.D.V at ORACLE.COM <mailto:JAYATHIRTH.D.V at 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.burkhalter at oracle.com <mailto:brian.burkhalter at 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.D.V at ORACLE.COM <mailto:JAYATHIRTH.D.V at 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.
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/2d-dev/attachments/20200305/56842638/attachment.htm>