[OpenJDK 2D-Dev] [15] RFR JDK-6532025: GIF reader throws misleading exception with truncated images
Brian Burkhalter
brian.burkhalter at oracle.com
Fri Mar 6 16:18:36 UTC 2020
+1
> On Mar 6, 2020, at 8:01 AM, Phil Race <philip.race at oracle.com> wrote:
>
> +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/ <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 <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/afd4754f/attachment-0001.htm>
More information about the 2d-dev
mailing list