diff --git a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReader.java b/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReader.java index 3cdb0253..7826f21e 100644 --- a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReader.java +++ b/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReader.java @@ -38,6 +38,7 @@ import com.twelvemonkeys.lang.Validate; import javax.imageio.IIOException; import javax.imageio.ImageIO; import javax.imageio.stream.ImageInputStream; +import java.io.EOFException; import java.io.File; import java.io.IOException; import java.nio.ByteOrder; @@ -52,6 +53,9 @@ import java.util.*; * @version $Id: EXIFReader.java,v 1.0 Nov 13, 2009 5:42:51 PM haraldk Exp$ */ public final class EXIFReader extends MetadataReader { + + final static boolean DEBUG = "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.metadata.exif.debug")); + static final Collection KNOWN_IFDS = Collections.unmodifiableCollection(Arrays.asList(TIFF.TAG_EXIF_IFD, TIFF.TAG_GPS_IFD, TIFF.TAG_INTEROP_IFD, TIFF.TAG_SUB_IFD)); @Override @@ -83,13 +87,22 @@ public final class EXIFReader extends MetadataReader { return readDirectory(input, directoryOffset, true); } + // TODO: Consider re-writing so that the linked IFD parsing is done externally to the method protected Directory readDirectory(final ImageInputStream pInput, final long pOffset, final boolean readLinked) throws IOException { List ifds = new ArrayList(); List entries = new ArrayList(); pInput.seek(pOffset); long nextOffset = -1; - int entryCount = pInput.readUnsignedShort(); + + int entryCount; + try { + entryCount = pInput.readUnsignedShort(); + } + catch (EOFException e) { + // Treat EOF here as empty Sub-IFD + entryCount = 0; + } for (int i = 0; i < entryCount; i++) { EXIFEntry entry = readEntry(pInput); @@ -119,15 +132,9 @@ public final class EXIFReader extends MetadataReader { } } - // TODO: Make what sub-IFDs to parse optional? Or leave this to client code? At least skip the non-TIFF data? - // TODO: Put it in the constructor? + // TODO: Consider leaving to client code what sub-IFDs to parse (but always parse TAG_SUB_IFD). readSubdirectories(pInput, entries, - Arrays.asList(TIFF.TAG_EXIF_IFD, TIFF.TAG_GPS_IFD, TIFF.TAG_INTEROP_IFD, TIFF.TAG_SUB_IFD -// , TIFF.TAG_IPTC, TIFF.TAG_XMP -// , TIFF.TAG_ICC_PROFILE -// , TIFF.TAG_PHOTOSHOP -// ,TIFF.TAG_MODI_OLE_PROPERTY_SET - ) + Arrays.asList(TIFF.TAG_EXIF_IFD, TIFF.TAG_GPS_IFD, TIFF.TAG_INTEROP_IFD, TIFF.TAG_SUB_IFD) ); ifds.add(0, new IFD(entries)); @@ -224,20 +231,24 @@ public final class EXIFReader extends MetadataReader { // Invalid tag, this is just for debugging long offset = pInput.getStreamPosition() - 8l; - System.err.printf("Bad EXIF"); - System.err.println("tagId: " + tagId + (tagId <= 0 ? " (INVALID)" : "")); - System.err.println("type: " + type + " (INVALID)"); - System.err.println("count: " + count); + if (DEBUG) { + System.err.printf("Bad EXIF data @%08x\n", pInput.getStreamPosition()); + System.err.println("tagId: " + tagId + (tagId <= 0 ? " (INVALID)" : "")); + System.err.println("type: " + type + " (INVALID)"); + System.err.println("count: " + count); + } pInput.mark(); pInput.seek(offset); try { - byte[] bytes = new byte[8 + Math.min(120, Math.max(20, count))]; + byte[] bytes = new byte[8 + Math.min(120, Math.max(24, count))]; int len = pInput.read(bytes); - System.err.print(HexDump.dump(offset, bytes, 0, len)); - System.err.println(len < count ? "[...]" : ""); + if (DEBUG) { + System.err.print(HexDump.dump(offset, bytes, 0, len)); + System.err.println(len < count ? "[...]" : ""); + } } finally { pInput.reset(); diff --git a/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReaderTest.java b/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReaderTest.java index b7770e11..1249de6c 100644 --- a/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReaderTest.java +++ b/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReaderTest.java @@ -192,17 +192,86 @@ public class EXIFReaderTest extends MetadataReaderAbstractTest { } @Test - public void testReadExifJPEGWithInteropSubDir() throws IOException { - ImageInputStream stream = ImageIO.createImageInputStream(getResource("/jpeg/exif-with-interop-subdir.jpg")); + public void testReadExifJPEGWithInteropSubDirR98() throws IOException { + ImageInputStream stream = ImageIO.createImageInputStream(getResource("/jpeg/exif-with-interop-subdir-R98.jpg")); stream.seek(30); - Directory directory = createReader().read(new SubImageInputStream(stream, 65535)); + CompoundDirectory directory = (CompoundDirectory) createReader().read(new SubImageInputStream(stream, 1360)); + assertEquals(17, directory.size()); + assertEquals(2, directory.directoryCount()); + + Directory exif = (Directory) directory.getEntryById(TIFF.TAG_EXIF_IFD).getValue(); + assertNotNull(exif); + assertEquals(23, exif.size()); + + // The interop IFD is empty (entry count is 0) + Directory interop = (Directory) exif.getEntryById(TIFF.TAG_INTEROP_IFD).getValue(); + assertNotNull(interop); + assertEquals(2, interop.size()); + + assertNotNull(interop.getEntryById(1)); // InteropIndex + assertEquals("ASCII", interop.getEntryById(1).getTypeName()); + assertEquals("R98", interop.getEntryById(1).getValue()); // Known values: R98, THM or R03 + + assertNotNull(interop.getEntryById(2)); // InteropVersion + assertEquals("UNDEFINED", interop.getEntryById(2).getTypeName()); + assertArrayEquals(new byte[] {'0', '1', '0', '0'}, (byte[]) interop.getEntryById(2).getValue()); + } + + @Test + public void testReadExifJPEGWithInteropSubDirEmpty() throws IOException { + ImageInputStream stream = ImageIO.createImageInputStream(getResource("/jpeg/exif-with-interop-subdir-empty.jpg")); + stream.seek(30); + + CompoundDirectory directory = (CompoundDirectory) createReader().read(new SubImageInputStream(stream, 1360)); assertEquals(11, directory.size()); + assertEquals(1, directory.directoryCount()); Directory exif = (Directory) directory.getEntryById(TIFF.TAG_EXIF_IFD).getValue(); assertNotNull(exif); assertEquals(24, exif.size()); + // The interop IFD is empty (entry count is 0) + Directory interop = (Directory) exif.getEntryById(TIFF.TAG_INTEROP_IFD).getValue(); + assertNotNull(interop); + assertEquals(0, interop.size()); + } + + @Test + public void testReadExifJPEGWithInteropSubDirEOF() throws IOException { + ImageInputStream stream = ImageIO.createImageInputStream(getResource("/jpeg/exif-with-interop-subdir-eof.jpg")); + stream.seek(30); + + CompoundDirectory directory = (CompoundDirectory) createReader().read(new SubImageInputStream(stream, 236)); + assertEquals(8, directory.size()); + assertEquals(1, directory.directoryCount()); + + Directory exif = (Directory) directory.getEntryById(TIFF.TAG_EXIF_IFD).getValue(); + assertNotNull(exif); + assertEquals(5, exif.size()); + + // The interop IFD isn't there (offset points to outside the TIFF structure)... + // Have double-checked using ExifTool, which says "Warning : Bad InteropOffset SubDirectory start" + Directory interop = (Directory) exif.getEntryById(TIFF.TAG_INTEROP_IFD).getValue(); + assertNotNull(interop); + assertEquals(0, interop.size()); + } + + @Test + public void testReadExifJPEGWithInteropSubDirBad() throws IOException { + ImageInputStream stream = ImageIO.createImageInputStream(getResource("/jpeg/exif-with-interop-subdir-bad.jpg")); + stream.seek(30); + + CompoundDirectory directory = (CompoundDirectory) createReader().read(new SubImageInputStream(stream, 12185)); + assertEquals(16, directory.size()); + assertEquals(2, directory.directoryCount()); + + Directory exif = (Directory) directory.getEntryById(TIFF.TAG_EXIF_IFD).getValue(); + assertNotNull(exif); + assertEquals(26, exif.size()); + + // JPEG starts at offset 1666 and length 10519, interop IFD points to offset 1900... + // Have double-checked using ExifTool, which says "Warning : Bad InteropIFD directory" Directory interop = (Directory) exif.getEntryById(TIFF.TAG_INTEROP_IFD).getValue(); assertNotNull(interop); assertEquals(0, interop.size()); diff --git a/imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir-R98.jpg b/imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir-R98.jpg new file mode 100644 index 00000000..cf0e4c43 Binary files /dev/null and b/imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir-R98.jpg differ diff --git a/imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir-bad.jpg b/imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir-bad.jpg new file mode 100644 index 00000000..dee98118 Binary files /dev/null and b/imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir-bad.jpg differ diff --git a/imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir.jpg b/imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir-empty.jpg similarity index 100% rename from imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir.jpg rename to imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir-empty.jpg diff --git a/imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir-eof.jpg b/imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir-eof.jpg new file mode 100644 index 00000000..9b8594b7 Binary files /dev/null and b/imageio/imageio-metadata/src/test/resources/jpeg/exif-with-interop-subdir-eof.jpg differ