Made EXIFReader more lenient while parsing.

- Now supports empty strings encoded with value count 0.
 - Added Rational.NaN constant to handle bad EXIF data.
Fixed a bug in the JPEGImageReader's raw EXIF thumbnail decoding.
Added test cases.

imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java

@@ -70,7 +70,7 @@ import java.util.List;
  */
 public class JPEGImageReader extends ImageReaderBase {
     // TODO: Fix the (stream) metadata inconsistency issues.
-    // - Sun JPEGMetadata class does not (and can not be made to) support CMYK data.. We need to create all new metadata classes
+    // - Sun JPEGMetadata class does not (and can not be made to) support CMYK data.. We need to create all new metadata classes.. :-/
 
     private final static boolean DEBUG = "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.plugins.jpeg.debug"));
 
@@ -543,7 +543,7 @@ public class JPEGImageReader extends ImageReaderBase {
             ImageInputStream stream = ImageIO.createImageInputStream(data);
 
             CompoundDirectory exifMetadata = (CompoundDirectory) new EXIFReader().read(stream);
-            /**/
+
             if (exifMetadata.directoryCount() == 2) {
                 Directory ifd1 = exifMetadata.getDirectory(1);
                 Entry compression = ifd1.getEntryById(TIFF.TAG_COMPRESSION);
@@ -556,7 +556,7 @@ public class JPEGImageReader extends ImageReaderBase {
                     if (width == null || height == null) {
                         throw new IIOException("Missing dimensions for RAW EXIF thumbnail");
                     }
-                    
+
                     Entry bitsPerSample = ifd1.getEntryById(TIFF.TAG_BITS_PER_SAMPLE);
                     Entry samplesPerPixel = ifd1.getEntryById(TIFF.TAG_SAMPLES_PER_PIXELS);
                     Entry photometricInterpretation = ifd1.getEntryById(TIFF.TAG_PHOTOMETRIC_INTERPRETATION);
@@ -578,41 +578,49 @@ public class JPEGImageReader extends ImageReaderBase {
                     
                     int interpretation = photometricInterpretation != null ? ((Number) photometricInterpretation.getValue()).intValue() : 2;
 
-                    // Read raw image data, either RGB or YCbCr
+                    // IFD1 should contain strip offsets for uncompressed images
-                    byte[] thumbData = readFully(stream, w * h * 3);
+                    Entry offset = ifd1.getEntryById(TIFF.TAG_STRIP_OFFSETS);
-                    DataBuffer buffer = new DataBufferByte(thumbData, thumbData.length);
+                    if (offset != null) {
-                    WritableRaster raster = Raster.createInterleavedRaster(buffer, w, h, w * 3, 3, new int[] {0, 1, 2}, null);
+                        stream.seek(((Number) offset.getValue()).longValue());
 
-                    switch (interpretation) {
+                        // Read raw image data, either RGB or YCbCr
-                        case 2:
+                        int thumbSize = w * h * 3;
-                            // RGB
+                        byte[] thumbData = readFully(stream, thumbSize);
-                            break;
+
-                        case 6:
+                        switch (interpretation) {
-                            // YCbCr
+                            case 2:
-                            YCbCrConverter.convertYCbCr2RGB(raster);
+                                // RGB
-                            break;
+                                break;
-                        default:
+                            case 6:
-                            throw new IIOException("Unknown photometric interpretation for RAW EXIF thumbail: " + interpretation);
+                                // YCbCr
+                                for (int i = 0, thumbDataLength = thumbData.length; i < thumbDataLength; i++) {
+                                    YCbCrConverter.convertYCbCr2RGB(thumbData, thumbData, i);
+                                }
+                                break;
+                            default:
+                                throw new IIOException("Unknown photometric interpretation for RAW EXIF thumbail: " + interpretation);
+                        }
+
+                        thumbnails.add(readRawThumbnail(thumbData, thumbData.length, 0, w, h));
                     }
-
-                    ColorModel cm = new ComponentColorModel(ColorSpace.getInstance(ColorSpace.CS_sRGB),false, false, Transparency.OPAQUE, DataBuffer.TYPE_BYTE);
-
-                    thumbnails.add(new BufferedImage(cm, raster, cm.isAlphaPremultiplied(), null));
                 }
                 else if (compression == null || compression.getValue().equals(6)) {
                     Entry jpegOffset = ifd1.getEntryById(TIFF.TAG_JPEG_INTERCHANGE_FORMAT);
+
+                    // IFD1 should contain jpeg offset for JPEG thumbnail
                     if (jpegOffset != null) {
-                        stream.seek((Long) jpegOffset.getValue());
+                        stream.seek(((Number) jpegOffset.getValue()).longValue());
                         InputStream adapter = IIOUtil.createStreamAdapter(stream);
                         BufferedImage exifThumb = ImageIO.read(adapter);
+
                         if (exifThumb != null) {
                             thumbnails.add(exifThumb);
                         }
+
                         adapter.close();
                     }
                 }
             }
-            //*/
 
             return exifMetadata;
         }
@@ -841,8 +849,7 @@ public class JPEGImageReader extends ImageReaderBase {
 
             JFIF jfif = getJFIF();
             if (jfif != null && jfif.thumbnail != null) {
-                // TODO: Actually decode jfif
+                thumbnails.add(readRawThumbnail(jfif.thumbnail, jfif.thumbnail.length, 0, jfif.xThumbnail, jfif.yThumbnail));
-                thumbnails.add(new BufferedImage(jfif.xThumbnail, jfif.yThumbnail, BufferedImage.TYPE_3BYTE_BGR));
             }
 
             JFXX jfxx = getJFXX();
@@ -861,10 +868,10 @@ public class JPEGImageReader extends ImageReaderBase {
                         
                         int[] rgbs = new int[256];
                         for (int i = 0; i < rgbs.length; i++) {
-                            int rgb = (jfxx.thumbnail[3 * i] & 0xff) << 16|
+                            int rgb = (jfxx.thumbnail[3 * i] & 0xff) << 16
-                                    (jfxx.thumbnail[3 * i] & 0xff) << 8 |
+                                    | (jfxx.thumbnail[3 * i] & 0xff) << 8
-                                    (jfxx.thumbnail[3 * i] & 0xff);
+                                    | (jfxx.thumbnail[3 * i] & 0xff);
-                            
+
                             rgbs[i] = rgb;
                         }
 
@@ -881,19 +888,13 @@ public class JPEGImageReader extends ImageReaderBase {
                         w = jfxx.thumbnail[0] & 0xff;
                         h = jfxx.thumbnail[1] & 0xff;
 
-                        buffer = new DataBufferByte(jfxx.thumbnail, jfxx.thumbnail.length - 2, 2);
+                        thumbnails.add(readRawThumbnail(jfxx.thumbnail, jfxx.thumbnail.length - 2, 2, w, h));
-                        raster = Raster.createInterleavedRaster(buffer, w, h, w * 3, 3, new int[] {0, 1, 2}, null);
-                        ColorModel cm = new ComponentColorModel(ColorSpace.getInstance(ColorSpace.CS_sRGB),false, false, Transparency.OPAQUE, DataBuffer.TYPE_BYTE);
-
-                        thumbnails.add(new BufferedImage(cm, raster, cm.isAlphaPremultiplied(), null));
                         break;
-
                     default:
                         processWarningOccurred("Unknown JFXX extension code: " + jfxx.extensionCode);
                 }
             }
 
-            
             // TODO: Ideally we want to decode image data in getThumbnail, less ideally here, but at least not in getEXIFMetadata()
             CompoundDirectory exifMetadata = getEXIFMetadata();
 //            System.err.println("exifMetadata: " + exifMetadata);
@@ -905,6 +906,16 @@ public class JPEGImageReader extends ImageReaderBase {
         }
     }
 
+    // TODO: Candidate for util method
+    private BufferedImage readRawThumbnail(final byte[] thumbnail, final int size, final int offset, int w, int h) {
+        DataBufferByte buffer;WritableRaster raster;
+        buffer = new DataBufferByte(thumbnail, size, offset);
+        raster = Raster.createInterleavedRaster(buffer, w, h, w * 3, 3, new int[] {0, 1, 2}, null);
+        ColorModel cm = new ComponentColorModel(ColorSpace.getInstance(ColorSpace.CS_sRGB),false, false, Transparency.OPAQUE, DataBuffer.TYPE_BYTE);
+
+        return new BufferedImage(cm, raster, cm.isAlphaPremultiplied(), null);
+    }
+
     @Override
     public int getNumThumbnails(final int imageIndex) throws IOException {
         readThumbnailMetadata(imageIndex);
@@ -935,12 +946,10 @@ public class JPEGImageReader extends ImageReaderBase {
     public BufferedImage readThumbnail(int imageIndex, int thumbnailIndex) throws IOException {
         checkThumbnailBounds(imageIndex, thumbnailIndex);
 
-        // TODO: Thumbnail progress listeners...
-
-        BufferedImage thumbnail = thumbnails.get(thumbnailIndex);
         processThumbnailStarted(imageIndex, thumbnailIndex);
         // For now: Clone. TODO: Do the actual decoding/reading here.
-        thumbnail = new BufferedImage(thumbnail.getColorModel(), thumbnail.copyData(null), thumbnail.getColorModel().isAlphaPremultiplied(), null);
+        BufferedImage cached = thumbnails.get(thumbnailIndex);
+        BufferedImage thumbnail = new BufferedImage(cached.getColorModel(), cached.copyData(null), cached.getColorModel().isAlphaPremultiplied(), null);
         processThumbnailProgress(100f);
         processThumbnailComplete();
 

imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStream.java

@@ -208,6 +208,12 @@ final class JPEGSegmentImageInputStream extends ImageInputStreamImpl {
         return total;
     }
 
+    @SuppressWarnings({"FinalizeDoesntCallSuperFinalize"})
+    @Override
+    protected void finalize() throws Throwable {
+        // Empty finalizer (for improved performance; no need to call super.finalize() in this case)
+    }
+
     static class Segment {
         private final int marker;
 

imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReaderTest.java

@@ -303,7 +303,7 @@ public class JPEGImageReaderTest extends ImageReaderAbstractTestCase<JPEGImageRe
     }
 
     @Test
-    public void testEXIFRAWThumbnail() throws IOException {
+    public void testEXIFRawThumbnail() throws IOException {
         JPEGImageReader reader = createReader();
         reader.setInput(ImageIO.createImageInputStream(getClassLoaderResource("/jpeg/exif-rgb-thumbnail-sony-d700.jpg")));
 
@@ -317,4 +317,20 @@ public class JPEGImageReaderTest extends ImageReaderAbstractTestCase<JPEGImageRe
         assertEquals(80, thumbnail.getWidth());
         assertEquals(60, thumbnail.getHeight());
     }
+
+    @Test
+    public void testBadEXIFRawThumbnail() throws IOException {
+        JPEGImageReader reader = createReader();
+        reader.setInput(ImageIO.createImageInputStream(getClassLoaderResource("/jpeg/exif-rgb-thumbnail-bad-exif-kodak-dc210.jpg")));
+
+        assertTrue(reader.hasThumbnails(0));
+        assertEquals(1, reader.getNumThumbnails(0));
+        assertEquals(96, reader.getThumbnailWidth(0, 0));
+        assertEquals(72, reader.getThumbnailHeight(0, 0));
+
+        BufferedImage thumbnail = reader.readThumbnail(0, 0);
+        assertNotNull(thumbnail);
+        assertEquals(96, thumbnail.getWidth());
+        assertEquals(72, thumbnail.getHeight());
+    }
 }

imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/exif/EXIFEntry.java

@@ -80,6 +80,8 @@ final class EXIFEntry extends AbstractEntry {
                 return "PhotometricInterpretation";
             case TIFF.TAG_IMAGE_DESCRIPTION:
                 return "ImageDescription";
+            case TIFF.TAG_STRIP_OFFSETS:
+                return "StripOffsets";
             case TIFF.TAG_ORIENTATION:
                 return "Orientation";
             case TIFF.TAG_SAMPLES_PER_PIXELS:

imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReader.java

@@ -213,12 +213,11 @@ public final class EXIFReader extends MetadataReader {
         short type = pInput.readShort();
         int count = pInput.readInt(); // Number of values
 
-        // TODO: Read up what the spec says about value-count == 0, it makes no sense.
+        // It's probably a spec violation to have count 0, but we'll be lenient about it
         if (count < 0) {
             throw new IIOException(String.format("Illegal count %d for tag %s type %s @%08x", count, tagId, type, pInput.getStreamPosition()));
         }
 
-        Object value;
         int valueLength = getValueLength(type, count);
 
         if (type < 0 || type > 13) {
@@ -243,7 +242,8 @@ public final class EXIFReader extends MetadataReader {
             }
         }
 
-        // TODO: For BigTiff allow size > 4 && <= 8
+        Object value;
+        // TODO: For BigTiff allow size > 4 && <= 8 in addition
         if (valueLength > 0 && valueLength <= 4) {
             value = readValueInLine(pInput, type, count);
             pInput.skipBytes(4 - valueLength);
@@ -278,8 +278,11 @@ public final class EXIFReader extends MetadataReader {
 
         switch (pType) {
             case 2: // ASCII
-                // TODO: This might be UTF-8 or ISO-8859-x, even though spec says ASCII
+                // TODO: This might be UTF-8 or ISO-8859-x, even though spec says NULL-terminated 7 bit ASCII
                 // TODO: Fail if unknown chars, try parsing with ISO-8859-1 or file.encoding
+                if (pCount == 0) {
+                    return "";
+                }
                 byte[] ascii = new byte[pCount];
                 pInput.readFully(ascii);
                 int len = ascii[ascii.length - 1] == 0 ? ascii.length - 1 : ascii.length;
@@ -364,23 +367,23 @@ public final class EXIFReader extends MetadataReader {
 
             case 5: // RATIONAL
                 if (pCount == 1) {
-                    return new Rational(pInput.readUnsignedInt(), pInput.readUnsignedInt());
+                    return createSafeRational(pInput.readUnsignedInt(), pInput.readUnsignedInt());
                 }
 
                 Rational[] rationals = new Rational[pCount];
                 for (int i = 0; i < rationals.length; i++) {
-                    rationals[i] = new Rational(pInput.readUnsignedInt(), pInput.readUnsignedInt());
+                    rationals[i] = createSafeRational(pInput.readUnsignedInt(), pInput.readUnsignedInt());
                 }
 
                 return rationals;
             case 10: // SRATIONAL
                 if (pCount == 1) {
-                    return new Rational(pInput.readInt(), pInput.readInt());
+                    return createSafeRational(pInput.readInt(), pInput.readInt());
                 }
 
                 Rational[] srationals = new Rational[pCount];
                 for (int i = 0; i < srationals.length; i++) {
-                    srationals[i] = new Rational(pInput.readInt(), pInput.readInt());
+                    srationals[i] = createSafeRational(pInput.readInt(), pInput.readInt());
                 }
 
                 return srationals;
@@ -412,6 +415,15 @@ public final class EXIFReader extends MetadataReader {
         }
     }
 
+    private static Rational createSafeRational(final long numerator, final long denominator) throws IOException {
+        if (denominator == 0) {
+            // Bad data.
+            return Rational.NaN;
+        }
+
+        return new Rational(numerator, denominator);
+    }
+
     private int getValueLength(final int pType, final int pCount) {
         if (pType > 0 && pType <= TIFF.TYPE_LENGTHS.length) {
             return TIFF.TYPE_LENGTHS[pType - 1] * pCount;

imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/exif/Rational.java

@@ -53,10 +53,16 @@ public final class Rational extends Number implements Comparable<Rational> {
     // TODO: Move to com.tm.lang?
     // Inspired by http://www.cs.princeton.edu/introcs/92symbolic/Rational.java.html and java.lang.Integer
     static final Rational ZERO = new Rational(0, 1);
+    static final Rational NaN = new Rational(); // TODO: This field needs thoughts/tests/spec/consistency check, see Float.NaN
 
     private final long numerator;
     private final long denominator;
 
+    private Rational() {
+        numerator = 0;
+        denominator = 0;
+    }
+
     public Rational(final long pNumber) {
         this(pNumber, 1);
     }
@@ -121,6 +127,10 @@ public final class Rational extends Number implements Comparable<Rational> {
 
     @Override
     public double doubleValue() {
+        if (this == NaN) {
+            return Double.NaN;
+        }
+
         return numerator / (double) denominator;
     }
 
@@ -147,6 +157,10 @@ public final class Rational extends Number implements Comparable<Rational> {
 
     @Override
     public String toString() {
+        if (this == NaN) {
+            return "NaN";
+        }
+
         return denominator == 1 ? Long.toString(numerator) : String.format("%s/%s", numerator, denominator);
     }
 

imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReaderTest.java

@@ -30,10 +30,13 @@ package com.twelvemonkeys.imageio.metadata.exif;
 
 import com.twelvemonkeys.imageio.metadata.CompoundDirectory;
 import com.twelvemonkeys.imageio.metadata.Directory;
+import com.twelvemonkeys.imageio.metadata.Entry;
 import com.twelvemonkeys.imageio.metadata.MetadataReaderAbstractTest;
+import com.twelvemonkeys.imageio.stream.SubImageInputStream;
 import org.junit.Test;
 
 import javax.imageio.ImageIO;
+import javax.imageio.stream.ImageInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 
@@ -131,4 +134,31 @@ public class EXIFReaderTest extends MetadataReaderAbstractTest {
         assertNull(ifd1.getEntryById(TIFF.TAG_IMAGE_WIDTH));
         assertNull(ifd1.getEntryById(TIFF.TAG_IMAGE_HEIGHT));
     }
+
+    @Test
+    public void testReadBadDataZeroCount() throws IOException {
+        // This image seems to contain bad Exif. But as other tools are able to read, so should we..
+        ImageInputStream stream = ImageIO.createImageInputStream(getResource("/jpeg/exif-rgb-thumbnail-bad-exif-kodak-dc210.jpg"));
+        stream.seek(12);
+        Directory directory = createReader().read(new SubImageInputStream(stream, 21674));
+
+        assertEquals(22, directory.size());
+
+        // Special case: Ascii string with count == 0, not ok according to spec (?), but we'll let it pass
+        assertEquals("", directory.getEntryById(TIFF.TAG_IMAGE_DESCRIPTION).getValue());
+    }
+
+    @Test
+    public void testReadBadDataRationalZeroDenominator() throws IOException {
+        // This image seems to contain bad Exif. But as other tools are able to read, so should we..
+        ImageInputStream stream = ImageIO.createImageInputStream(getResource("/jpeg/exif-rgb-thumbnail-bad-exif-kodak-dc210.jpg"));
+        stream.seek(12);
+        Directory directory = createReader().read(new SubImageInputStream(stream, 21674));
+
+        // Special case: Rational with zero-denominator inside EXIF data
+        Directory exif = (Directory) directory.getEntryById(TIFF.TAG_EXIF_IFD).getValue();
+        Entry entry = exif.getEntryById(EXIF.TAG_COMPRESSED_BITS_PER_PIXEL);
+        assertNotNull(entry);
+        assertEquals(Rational.NaN, entry.getValue());
+    }
 }