TMI-20: Made the EXIFReader more lenient about bad directory entry count.

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

@@ -86,23 +86,34 @@ public final class EXIFReader extends MetadataReader {
     private Directory readDirectory(final ImageInputStream pInput, final long pOffset) throws IOException {
         List<IFD> ifds = new ArrayList<IFD>();
         List<Entry> entries = new ArrayList<Entry>();
+
         pInput.seek(pOffset);
+        long nextOffset = -1;
         int entryCount = pInput.readUnsignedShort();
 
         for (int i = 0; i < entryCount; i++) {
-            entries.add(readEntry(pInput));
+            EXIFEntry entry = readEntry(pInput);
+
+            if (entry == null) {
+                // TODO: Log warning?
+                nextOffset = 0;
+                break;
+            }
+
+            entries.add(entry);
+        }
+
+        if (nextOffset == -1) {
+            nextOffset = pInput.readUnsignedInt();
         }
 
-        long nextOffset = pInput.readUnsignedInt();
-        
         // Read linked IFDs
         if (nextOffset != 0) {
             // TODO: This is probably not okay anymore.. Replace recursion with while loop
-            Directory next = readDirectory(pInput, nextOffset);
-            ifds.add((IFD) ((AbstractCompoundDirectory) next).getDirectory(0));
-//            for (Entry entry : next) {
-//                entries.add(entry);
-//            }
+            AbstractCompoundDirectory next = (AbstractCompoundDirectory) readDirectory(pInput, nextOffset);
+            for (int i = 0; i < next.directoryCount(); i++) {
+                ifds.add((IFD) next.getDirectory(i));
+            }
         }
 
         // TODO: Make what sub-IFDs to parse optional? Or leave this to client code? At least skip the non-TIFF data?
@@ -209,8 +220,13 @@ public final class EXIFReader extends MetadataReader {
     private EXIFEntry readEntry(final ImageInputStream pInput) throws IOException {
         // TODO: BigTiff entries are different
         int tagId = pInput.readUnsignedShort();
-
         short type = pInput.readShort();
+
+        // This isn't really an entry, and the directory entry count was wront
+        if (tagId == 0 && type == 0) {
+            return null;
+        }
+
         int count = pInput.readInt(); // Number of values
 
         // It's probably a spec violation to have count 0, but we'll be lenient about it
@@ -218,11 +234,9 @@ public final class EXIFReader extends MetadataReader {
             throw new IIOException(String.format("Illegal count %d for tag %s type %s @%08x", count, tagId, type, pInput.getStreamPosition()));
         }
 
-        int valueLength = getValueLength(type, count);
-
-        if (type < 0 || type > 13) {
+        if (type <= 0 || type > 13) {
             // Invalid tag, this is just for debugging
-            System.err.printf("offset: %08x%n", pInput.getStreamPosition() - 8l);
+            System.err.printf("Bad EXIF data at offset: %08x\n", pInput.getStreamPosition() - 8l);
             System.err.println("tagId: " + tagId);
             System.err.println("type: " + type + " (INVALID)");
             System.err.println("count: " + count);
@@ -231,17 +245,19 @@ public final class EXIFReader extends MetadataReader {
             pInput.seek(pInput.getStreamPosition() - 8);
 
             try {
-                byte[] bytes = new byte[8 + Math.max(20, valueLength)];
-                pInput.readFully(bytes);
+                byte[] bytes = new byte[8 + Math.max(20, count)];
+                int len = pInput.read(bytes);
 
-                System.err.print("data: " + HexDump.dump(bytes));
-                System.err.println(bytes.length < valueLength ? "..." : "");
+                System.err.print("data: " + HexDump.dump(bytes, 0, len));
+                System.err.println(len < count ? "[...]" : "");
             }
             finally {
                 pInput.reset();
             }
         }
 
+        int valueLength = getValueLength(type, count);
+
         Object value;
         // TODO: For BigTiff allow size > 4 && <= 8 in addition
         if (valueLength > 0 && valueLength <= 4) {

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

@@ -161,4 +161,18 @@ public class EXIFReaderTest extends MetadataReaderAbstractTest {
         assertNotNull(entry);
         assertEquals(Rational.NaN, entry.getValue());
     }
+
+    @Test
+    public void testReadBadDirectoryCount() 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-bad-directory-entry-count.jpg"));
+        stream.seek(4424 + 10);
+
+        Directory directory = createReader().read(new SubImageInputStream(stream, 214 - 6));
+        assertEquals(7, directory.size()); // TIFF structure says 8, but the last entry isn't there
+
+        Directory exif = (Directory) directory.getEntryById(TIFF.TAG_EXIF_IFD).getValue();
+        assertNotNull(exif);
+        assertEquals(3, exif.size());
+    }
 }