TMI-JPEG: More lenient segment parsing, now allows 0xFF padding between segments + fixed an NPE in JPEGImageReader if the parsing fails.

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

@@ -601,14 +601,24 @@ public class JPEGImageReader extends ImageReaderBase {
 
             segments = JPEGSegmentUtil.readSegments(imageInput, SEGMENT_IDENTIFIERS);
         }
-        catch (IOException ignore) {
+        catch (IIOException ignore) {
+            if (DEBUG) {
+                ignore.printStackTrace();
+            }
         }
         catch (IllegalArgumentException foo) {
-            foo.printStackTrace();
+            if (DEBUG) {
+                foo.printStackTrace();
+            }
         }
         finally {
             imageInput.reset();
         }
+
+        // In case of an exception, avoid NPE when referencing segments later
+        if (segments == null) {
+            segments = Collections.emptyList();
+        }
     }
 
     private List<JPEGSegment> getAppSegments(final int marker, final String identifier) throws IOException {

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

@@ -90,6 +90,12 @@ final class JPEGSegmentImageInputStream extends ImageInputStreamImpl {
                 long realPosition = stream.getStreamPosition();
                 int marker = stream.readUnsignedShort();
 
+                // Skip over 0xff padding between markers
+                while (marker == 0xffff) {
+                    realPosition++;
+                    marker = (marker & 0xff) << 8 | stream.readUnsignedByte();
+                }
+
                 // TODO: Refactor to make various segments optional, we probably only want the "Adobe" APP14 segment, 'Exif' APP1 and very few others
                 if (isAppSegmentMarker(marker) && marker != JPEG.APP0 && !(marker == JPEG.APP1 && isAppSegmentWithId("Exif", stream)) && marker != JPEG.APP14) {
                     int length = stream.readUnsignedShort(); // Length including length field itself

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

@@ -75,7 +75,8 @@ public class JPEGImageReaderTest extends ImageReaderAbstractTestCase<JPEGImageRe
                 new TestData(getClassLoaderResource("/jpeg/gray-sample.jpg"), new Dimension(386, 396)),
                 new TestData(getClassLoaderResource("/jpeg/cmyk-sample.jpg"), new Dimension(160, 227)),
                 new TestData(getClassLoaderResource("/jpeg/cmyk-sample-multiple-chunk-icc.jpg"), new Dimension(2707, 3804)),
-                new TestData(getClassLoaderResource("/jpeg/jfif-jfxx-thumbnail-olympus-d320l.jpg"), new Dimension(640, 480))
+                new TestData(getClassLoaderResource("/jpeg/jfif-jfxx-thumbnail-olympus-d320l.jpg"), new Dimension(640, 480)),
+                new TestData(getClassLoaderResource("/jpeg/jfif-padded-segments.jpg"), new Dimension(20, 45))
         );
 
         // More test data in specific tests below

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

@@ -135,4 +135,27 @@ public class JPEGSegmentImageInputStreamTest {
 
         assertEquals(9299l, length); // Sanity check: same as file size
     }
+
+    @Test
+    public void testReadPaddedSegmentsBug() throws IOException {
+        ImageInputStream stream = new JPEGSegmentImageInputStream(ImageIO.createImageInputStream(getClassLoaderResource("/jpeg/jfif-padded-segments.jpg")));
+
+        List<JPEGSegment> appSegments = JPEGSegmentUtil.readSegments(stream, JPEGSegmentUtil.APP_SEGMENTS);
+        assertEquals(2, appSegments.size());
+
+        assertEquals(JPEG.APP0, appSegments.get(0).marker());
+        assertEquals("JFIF", appSegments.get(0).identifier());
+
+        assertEquals(JPEG.APP1, appSegments.get(1).marker());
+        assertEquals("Exif", appSegments.get(1).identifier());
+
+        stream.seek(0l);
+
+        long length = 0;
+        while (stream.read() != -1) {
+            length++;
+        }
+
+        assertEquals(1079L, length); // Sanity check: same as file size, except padding and the filtered ICC_PROFILE segment
+    }
 }

imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/jpeg/JPEGSegmentUtil.java

@@ -153,11 +153,18 @@ public final class JPEGSegmentUtil {
         }
     }
 
-    static JPEGSegment readSegment(final ImageInputStream stream, Map<Integer, List<String>> segmentIdentifiers) throws IOException {
+    static JPEGSegment readSegment(final ImageInputStream stream, final Map<Integer, List<String>> segmentIdentifiers) throws IOException {
         int marker = stream.readUnsignedShort();
+
+        // Skip over 0xff padding between markers
+        while (marker == 0xffff) {
+            marker = (marker & 0xff) << 8 | stream.readUnsignedByte();
+        }
+
         if ((marker >> 8 & 0xff) != 0xff) {
             throw new IIOException(String.format("Bad marker: %04x", marker));
         }
+
         int length = stream.readUnsignedShort(); // Length including length field itself
 
         byte[] data;
@@ -196,7 +203,7 @@ public final class JPEGSegmentUtil {
         }
 
         @Override
-        public boolean contains(Object o) {
+        public boolean contains(final Object o) {
             return true;
         }
     }
@@ -208,13 +215,13 @@ public final class JPEGSegmentUtil {
         }
 
         @Override
-        public List<String> get(Object key) {
+        public List<String> get(final Object key) {
             return key instanceof Integer && JPEGSegment.isAppSegmentMarker((Integer) key) ? ALL_IDS : null;
 
         }
 
         @Override
-        public boolean containsKey(Object key) {
+        public boolean containsKey(final Object key) {
             return true;
         }
     }
@@ -226,7 +233,7 @@ public final class JPEGSegmentUtil {
         }
 
         @Override
-        public List<String> get(Object key) {
+        public List<String> get(final Object key) {
             return containsKey(key) ? ALL_IDS : null;
 
         }

imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/jpeg/JPEGSegmentUtilTest.java

@@ -196,4 +196,17 @@ public class JPEGSegmentUtilTest {
         assertEquals(JPEG.APP14, segments.get(21).marker());
         assertEquals("Adobe", segments.get(21).identifier());
     }
+
+    @Test
+    public void testReadPaddedSegments() throws IOException {
+        List<JPEGSegment> segments = JPEGSegmentUtil.readSegments(getData("/jpeg/jfif-padded-segments.jpg"), JPEGSegmentUtil.APP_SEGMENTS);
+        assertEquals(3, segments.size());
+
+        assertEquals(JPEG.APP0, segments.get(0).marker());
+        assertEquals("JFIF", segments.get(0).identifier());
+        assertEquals(JPEG.APP2, segments.get(1).marker());
+        assertEquals("ICC_PROFILE", segments.get(1).identifier());
+        assertEquals(JPEG.APP1, segments.get(2).marker());
+        assertEquals("Exif", segments.get(2).identifier());
+    }
 }