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

Phil Race philip.race at oracle.com
Fri Mar 6 16:01:35 UTC 2020


+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.race at oracle.com 
>> <mailto:philip.race at 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.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/20200306/5081f157/attachment.htm>


More information about the 2d-dev mailing list