[OpenJDK 2D-Dev] Review Request for JDK-8152672 : Exception while getting second image properties for JPEG with embedded thumbnail

[Jim Graham]

It looks like JPEG files have protection for scanning for an FF and 
assuming it is a marker by making sure that all FF bytes that appear in 
data are followed by a harmless byte, so a brute force search is 
probably fine. But it still seems wasteful when we know we are at a tag 
and we know the sizes of all of the tags, we should be able to skip 
around the file looking for the SOI directly...

			...jim

On 6/2/2016 5:10 AM, Jayathirth D V wrote:
> Fixed typo.
>
>
>
> *From:*Jayathirth D V
> *Sent:* Thursday, June 02, 2016 5:08 PM
> *To:* Philip Race
> *Cc:* Jim Graham; 2d-dev at openjdk.java.net
> *Subject:* RE: Review Request for JDK-8152672 : Exception while getting
> second image properties for JPEG with embedded thumbnail
>
>
>
> Hi Phil,
>
>
>
> We have two kind of images with which we are able to reproduce the issue:
>
> 1)      sample.jpg present in JBS bug(We can’t use this image because it
> is licensed ).
>
> 2)      JpegEmbedThumbnail.jpg taken using Prasanta’s camera and used in
> webrev.
>
>
>
> _ _
>
> _sample.jpg : _
>
> _ _
>
> If we do getNumImages() it will return 2. getNumImages() follows the
> same logic of skipping markers with length and registering SOI to get
> number of images.
>
> sample.jpg has markers as follows :
>
> SOI -> APP1 - > SOI -> EOI -> APP1 End -> EOI -> SOI -> EOI
>
>
>
> I have dumped first image its SOI is first one in the above list and for
> second image it is third one in the list. getNumImages() counts first
> and third SOI for number of images. But in case of skipImage() we are
> getting inside APP1 marker and considering its SOI.
>
>
>
> _JpegEmbedThumbnail.jpg :_
>
> _ _
>
> If we do getNumImages() it will return 1.
>
> JpegEmbedThumbnail.jpg has markers as follows :
>
> SOI -> APP1 -> SOI -> EOI -> APP1 End -> APP2 -> SOI -> APP2 End -> APP2
> -> EOI -> APP2 End -> EOI
>
>
>
> getNumImages() counts only first SOI for number of images. But in case
> of skipImage() we will are getting inside APP1 and APP2 markers also.
>
>
>
> Thanks,
>
> Jay
>
> *From:*Phil Race
> *Sent:* Thursday, June 02, 2016 4:05 AM
> *To:* Jayathirth D V
> *Cc:* Jim Graham; 2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>
> *Subject:* Re: Review Request for JDK-8152672 : Exception while getting
> second image properties for JPEG with embedded thumbnail
>
>
>
> I am bit doubtful about this. Are you sure we are correct in reporting
> two images to begin with ?
> Thumbnails should not get counted ..
>
>
> -phil.
>
> On 06/01/2016 01:06 AM, Jayathirth D V wrote:
>
>     Updated bug title in JBS as it was misleading.
>
>
>
>     *From:* Jayathirth D V
>     *Sent:* Wednesday, June 01, 2016 12:48 PM
>     *To:* Philip Race; Jim Graham
>     *Cc:* 2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>
>     *Subject:* Review Request for JDK-8152672 : Exception getting
>     thumbnail size for JPEG with embedded thumbnail
>
>
>
>     Hi,
>
>
>
>     _Please review the following fix in JDK9:_
>
>
>
>     Bug : https://bugs.openjdk.java.net/browse/JDK-8152672
>
>
>
>     Webrev : http://cr.openjdk.java.net/~jdv/8152672/webrev.00/
>     <http://cr.openjdk.java.net/%7Ejdv/8152672/webrev.00/>
>
>
>
>     Issue : When we are trying to get properties related to second image
>     in JPEG file we are getting IIOException mentioning that it is not a
>     JPEG file.
>
>
>
>     Root cause : When we are skipping first image to reach second image
>     header, we are just trying to find next available EOI marker. But if
>     first image has embedded thumbnail in APP1 marker, we will reach to
>     EOI of this thumbnail and not EOI of first image. So after we reach
>     EOI of embedded thumbnail we try to access second image SOI marker
>     which will fail.
>
>
>
>     Solution : We have to change the logic of how we skip to consecutive
>     images in JPEG file. We know that application markers, comments or
>     other markers can contain data same as SOI & EOI. Instead of just
>     checking for EOI marker serially, we should read length of these
>     markers and skip them.
>
>
>
>     Thanks,
>
>     Jay
>
>
>
>
>