dragon66 / icafe

Java library for reading, writing, converting and manipulating images and metadata
Eclipse Public License 1.0
204 stars 58 forks source link

IllegalArgumentException: Copy range out of array bounds - please don't kill me :-( #41

Closed sinedsem closed 7 years ago

sinedsem commented 7 years ago

Hi.. You know what happens, when about 400 users actively use your library? They find bugs...

Here I have an image: https://github.com/sinedsem/test/blob/master/5.jpg . Poor image, what happened with you? One evil user removed all metadata from you using XnView.

Now when I try to read metadata with icafe Metadata.readMetadata("5.jpg"); it throws IllegalArgumentException

java.lang.IllegalArgumentException: Copy range out of array bounds

    at com.icafe4j.util.ArrayUtils.subArray(Unknown Source)
    at com.icafe4j.image.meta.adobe.IRB.read(Unknown Source)
    at com.icafe4j.image.meta.Metadata.ensureDataRead(Unknown Source)
    at com.icafe4j.image.meta.adobe.IRB.get8BIM(Unknown Source)
    at com.icafe4j.image.jpeg.JPEGTweaker.readMetadata(Unknown Source)
    at com.icafe4j.image.meta.Metadata.readMetadata(Unknown Source)
    at com.icafe4j.image.meta.Metadata.readMetadata(Unknown Source)
    at com.icafe4j.image.meta.Metadata.readMetadata(Unknown Source)

Can this be related to https://github.com/dragon66/icafe/pull/38 ? I still think that subArray method should allow copying the full length.

dragon66 commented 7 years ago

@sinedsem subArray method is not the cause of the issue. The real issue is although the user is supposed to have removed all the metadata but actually only the data itself is removed while leaving an empty metadata container - a Photoshop IRB which contains a zero size 8BIM block.

When I wrote ICAFE, I first had "correctness" in mind but not "completeness". Now that we've got about 400 users, ICAFE is struggling to cope with all kinds of wired input. That's why it is complaining.

Anyway, I already know the issue and will fix it and let you know when I am done.

dragon66 commented 7 years ago

@sinedsem I checked in a one line fix, please see if that works.

sinedsem commented 7 years ago

@dragon66 I have no more files to check, but I I think I will release another version of my app, 0.0.7, tomorrow and let that user check it himself.

BTW, can you confirm that when icafe reads metadata, it doesn't read the whole image file, but only it's beginning?

dragon66 commented 7 years ago

@sinedsem You can say that but beginning of the image is a relative concept. For JPEG image, when reading metadata, icafe will read until SOS (start of scan) segment of the image as metadata can be scattered throughout the whole image and only until the SOS segment, can we be sure there are no more metadata come. The actual image data then follow. The rest of the image can be neglected. For TIFF image, metadata can be anywhere in the IFD (Image File Directory) and actual image data could even be at the beginning of the image.

But if we are going to insert or remove metadata, we will have to copy to the end of the image starting from SOS.

sinedsem commented 7 years ago

@dragon66, it seems I need to narrow my question. When it comes to 7000x7000 points jpeg, is it faster to read metadata and get thumbnail (if there is one) than read the whole image and resize it to make a thumb?

dragon66 commented 7 years ago

@sinedsem, Although it's hard to answer this question as they are completely different processes, generally speaking, resizing image will be slower and use way more memory because it will have to read the whole image and manipulate the raster data using different interpolation algorithm.

On the other hand, finding and extracting an existing thumbnail should be faster in most of the cases.

ICAFE itself can extract thumbnails embedded in different locations of JPEG, TIFF, etc and save them in either JPEG or TIFF format depending on the original compression.

If instead of saving thumbnails to stream/file, you want to use the thumbnail somewhere else, in most of the cases, you need to call getCompressedImage() on Thumbnail.java to retrieve a Byte[] representing the whole thumbnail in compressed format either JPEG or TIFF. In almost all cases, it would be in JPEG format.

sinedsem commented 7 years ago

This is fixed.