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 b10f55a4..78190a55 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 @@ -86,23 +86,34 @@ public final class EXIFReader extends MetadataReader { private Directory readDirectory(final ImageInputStream pInput, final long pOffset) throws IOException { List ifds = new ArrayList(); List entries = new ArrayList(); + 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) { 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 d454b308..e5738037 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 @@ -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()); + } } diff --git a/imageio/imageio-metadata/src/test/resources/jpeg/exif-bad-directory-entry-count.jpg b/imageio/imageio-metadata/src/test/resources/jpeg/exif-bad-directory-entry-count.jpg new file mode 100644 index 00000000..9a747e9c Binary files /dev/null and b/imageio/imageio-metadata/src/test/resources/jpeg/exif-bad-directory-entry-count.jpg differ