[OpenJDK 2D-Dev] Review Request for JDK-8160943 : [PIT] new failure of closed/javax/imageio/ReadAllThumbnailsTest.java

[Jayathirth D V]

Hi Jim,

I just ran the test case attached in https://bugs.openjdk.java.net/browse/JDK-4958271 .
It is actually generating an image and using it to do reader.readAll().

Thanks,
Jay
-----Original Message-----
From: Jim Graham 
Sent: Thursday, July 21, 2016 2:22 PM
To: Jayathirth D V; Philip Race
Cc: 2d-dev
Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-8160943 : [PIT] new failure of closed/javax/imageio/ReadAllThumbnailsTest.java

Hi Jay,

How did you generate that image with thumbnail?  With ImageIO?

			...jim

On 07/19/2016 12:18 PM, Jayathirth D V wrote:
> Hi Phil,
>
> I generated Jpeg image with thumbnail as given in test case attached for https://bugs.openjdk.java.net/browse/JDK-4958271 with JDK 9.
> The image has only two APP0 markers and after the second marker it has "00 FF" which we can consider as "X FF" as per https://www.w3.org/Graphics/JPEG/itu-t81.pdf. But it is just continuation of pattern "00 00 FF" which we see around the end of second APP0 marker.
>
> For this new image I don't see whether we are writing improperly in JDK9(If we consider "X FF" bits between markers).
> So we can just apply http://cr.openjdk.java.net/~jdv/8160943/webrev.00/ and open up the test case(after verifying whether it is the only test case which uses this particular image) with new image that I have created.
>
> Thanks,
> Jay
>
> -----Original Message-----
> From: Philip Race
> Sent: Wednesday, July 13, 2016 10:06 AM
> To: Jim Graham
> Cc: Jayathirth D V; 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-8160943 : [PIT] 
> new failure of closed/javax/imageio/ReadAllThumbnailsTest.java
>
> Whilst looking at the reason this test was originally created
> https://bugs.openjdk.java.net/browse/JDK-4958271
> it started to look likely that the image was (as I sort of suspected) was generated by IIO itself.
>
> I think there a couple of things we can do here
> 1) Open up the test (and image - although if moving it make sure no other tests in that location need the image).
> 2) Investigate to see if the bug such that the "bad padding" is written out is still reproducible with current IIO ... or if it was fixed sometime between when that image was created and now.
>
> -phil.
>
> On 7/12/16, 7:48 PM, Jim Graham wrote:
>> I think this is fine, but I noticed that some of the recently added 
>> comments have some grammar issues.  It might be nice to change the
>> following:
>>
>>  565                     // markers which do not contain length data
>> (doesn't => do not)
>>
>>  576                     // markers which contain length data
>> (contains => contain)
>>
>> I don't need to review those changes...
>>
>>             ...jim
>>
>> On 7/12/16 9:15 AM, Jayathirth D V wrote:
>>> Hi,
>>>
>>>
>>>
>>> Thanks for your input Phil.
>>>
>>> I have made changes just to parse "FF FF"(Like "FF 00" or any marker 
>>> without length)and not consider it as an invalid marker in
>>> skipImage() of JPEGImageReader.java.
>>>
>>> Also I have removed closed/javax/imageio/ReadAllThumbnailsTest.java
>>> from ProblemList.txt as part of fix.
>>>
>>>
>>>
>>> Bug : https://bugs.openjdk.java.net/browse/JDK-8160943
>>>
>>>
>>>
>>> Please find webrev for review for JDK9:
>>>
>>> http://cr.openjdk.java.net/~jdv/8160943/webrev.00/
>>>
>>>
>>>
>>> Thanks,
>>>
>>> Jay
>>>
>>>
>>>
>>> *From:*Phil Race
>>> *Sent:* Saturday, July 09, 2016 12:37 AM
>>> *To:* Jayathirth D V
>>> *Cc:* Jim Graham; 2d-dev
>>> *Subject:* Re: REG : JDK-8160943 : [PIT] new failure of 
>>> closed/javax/imageio/ReadAllThumbnailsTest.java
>>>
>>>
>>>
>>> On 07/08/2016 04:08 AM, Jayathirth D V wrote:
>>>
>>>     Hi,
>>>
>>>
>>>
>>>     In JDK-8152672 
>>> <https://bugs.openjdk.java.net/browse/JDK-8152672>
>>> we modified skipImage() in JpegImageReader.java
>>>     and added tighter checks for parsing Jpeg data.
>>>
>>>
>>>
>>>     We have to find any marker,0 or EOF after we find "FF" while 
>>> parsing JPEG data.
>>>
>>>     But in JDK-8160943
>>> <https://bugs.openjdk.java.net/browse/JDK-8160943> we have gap 
>>> between APP0 marker and DQT(FF DB)
>>>     marker which contains data "00 FF".
>>>
>>>
>>>
>>>     APP0_End -> 00 FF -> FF DB(DQT)
>>>
>>>
>>>
>>>     So after we skip APP0 marker we find two bytes of data which is 
>>> "FF FF". In the present code we consider this as
>>>     invalid marker.
>>>
>>>
>>> See https://www.w3.org/Graphics/JPEG/itu-t81.pdf
>>>
>>> B.1.1.2 Markers
>>> Markers serve to identify the various structural parts of the 
>>> compressed data formats.
>>> Most markers start marker segments containing a related group of 
>>> parameters; some markers stand alone. All markers are assigned 
>>> two-byte codes: an X'FF' byte followed by a byte which is not equal 
>>> to 0 or X'FF' (see Table B.1).
>>> Any marker may optionally be preceded by any number of fill bytes, 
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> which are bytes assigned code X'FF
>>> ^^^^^^^^^^^^^^^^^^^^^^
>>>
>>> -phil.
>>>
>>>
>>>     Because of this JDK-8160943
>>> <https://bugs.openjdk.java.net/browse/JDK-8160943> is failing.
>>>
>>>
>>>
>>>     Is the length of APP0 marker not valid in the image or we should 
>>> not consider "FF FF" as invalid maker?
>>>
>>>     Please let me know your input.
>>>
>>>
>>>
>>>     Thanks,
>>>
>>>     Jay
>>>
>>>
>>>