From b966254322aed055e12dec02a64a5ac97534779c Mon Sep 17 00:00:00 2001 From: Harald Kuhr Date: Fri, 19 Apr 2013 16:17:01 +0200 Subject: [PATCH] TMI-JPEG: More lenient segment parsing, now allows 0xFF padding between segments + fixed an NPE in JPEGImageReader if the parsing fails. --- .../imageio/plugins/jpeg/JPEGImageReader.java | 14 +++++++++-- .../jpeg/JPEGSegmentImageInputStream.java | 6 +++++ .../plugins/jpeg/JPEGImageReaderTest.java | 3 ++- .../jpeg/JPEGSegmentImageInputStreamTest.java | 23 ++++++++++++++++++ .../resources/jpeg/jfif-padded-segments.jpg | Bin 0 -> 4326 bytes .../metadata/jpeg/JPEGSegmentUtil.java | 17 +++++++++---- .../metadata/jpeg/JPEGSegmentUtilTest.java | 13 ++++++++++ .../resources/jpeg/jfif-padded-segments.jpg | Bin 0 -> 4326 bytes 8 files changed, 68 insertions(+), 8 deletions(-) create mode 100755 imageio/imageio-jpeg/src/test/resources/jpeg/jfif-padded-segments.jpg create mode 100755 imageio/imageio-metadata/src/test/resources/jpeg/jfif-padded-segments.jpg diff --git a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java index 6a674242..8128f6f3 100644 --- a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java +++ b/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 getAppSegments(final int marker, final String identifier) throws IOException { diff --git a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStream.java b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStream.java index 87dd6004..ff92bda1 100644 --- a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStream.java +++ b/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 diff --git a/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReaderTest.java b/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReaderTest.java index 6fa34246..c2b7ba31 100644 --- a/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReaderTest.java +++ b/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReaderTest.java @@ -75,7 +75,8 @@ public class JPEGImageReaderTest extends ImageReaderAbstractTestCase 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 + } } diff --git a/imageio/imageio-jpeg/src/test/resources/jpeg/jfif-padded-segments.jpg b/imageio/imageio-jpeg/src/test/resources/jpeg/jfif-padded-segments.jpg new file mode 100755 index 0000000000000000000000000000000000000000..b7cfd8e6d7771b6c77e87374c7e415aa85405529 GIT binary patch literal 4326 zcmb_f2UJs8w?6mY^a7-Sg7jVl=}nPdLXi%NsD#i$Cv>bRqJkreNLLgcil~f*B4Yzl zEQlf^HXM*aKpY2D5CzS;@&OKlD*>`7u`~ep8_-!06p4*T>mlbzh9t*geGzTfIQIU;E+U442mzIBa0KyO9B8f zMg7XDN$~<4h+-xWEfB@=0vx`8vjjMJ0T<85_;}f(F=qfEP}po$dBXpJ+3a9pPBuGSh!?n^ccNcx z$4!inW~a}8`2W1bL?@&Dnm1YWN=c0Jb`i{I&(b(-d+$XY5y}$m*}OkK$;N9DbCXy; zi#Q_L$#)SaNBLSW;;1+mVSH@x8uvv%C(#z|{X))+2p|7N91?10zleEpUc&gq6mOwF zBf@sgVoW64S+L*pwXu09`WFt3b`*YZe3FN-&e-TRLM~xE2VqQT;!nOw5k7+bo6niV z^AW~`aUEET{s^9v&>tT?f2QZ-dC6YFK7_{l3VRsBwiEUicmsW42y}rS@J|X&6PyoV z8yBC>ACLs$r!N8Ri`gIO89n>c z7lfYa#v=eoRQ&X*C;%Y3AAsfoPBJfLAs>ZJdg^ufJ~4La=}4x1eAgba1zvjv)}?~0#`sQxDD=uN1zwH1Os3g zjDra<4QA0l9C(NdF(64u4pM=%AbrRbS`FDlu8v6gmOb zLg%5&P%Cs7>V}>}gU}fC5&8xrFa>79GO#MV0yc%MVHem3-T;Tg32+9S1s{Nm;Y#=n z+z7YA_u*c603L^@5P*;o2}A+WMobY~#2pDh!Vn&^4aq?YkxJwo(t>m#J;(s^9{GyF zVHg;Bj5cN^#tGw#;b0Ol+c5hvrI;E_Bc>hGgBiq3V18h!SZS;l)*Q>iuEmC9Q?c3D zLTnYb0ecJEiyg*J<8U|$oEpvq=Y(5_i^Ofl?Z+L()#Gm9x^Y9eDLfu8iPyqg;N9_T zd=fqzUxGh_zm9*5AI8rR$OJipKEZ(yK!_#mBoq=(6Rr^+6GjNLL@}ZY(VXZ`3?Zfy z^N6R2EyRb!Vd5-_Mp7eLkbFpyq)bvV=^W`6=@sb{nM_tBn~^=qT=EWbG5I(09r7Ug zD}_$cq}WmdDan)rlxoTi%5%ymDwV2AwW0=4lc)!%wbYx`*VGvihKP;`OC&^OyGWTx zqsU{C528d-6;W%^jiMQ%g`yWlABw&gBZ{et*^05nwuzOAT^8#To1!sjdNg-h3~fK{ zH0>^Jl#ZvX((UOX^ep;G`c3)}17RpJY#AKJF2*UwEyget$5dmon32p}=2>PJb3&Xh zZYb_2o+e%j3SA1bifW3!iaQnS75kM)N~TI-N(D-7N)yU*%5KVAlV{wP^(jWrA}41R8LT^RDYs@(=gSD(m1N|NE6mH z(u~k7(|o7}YZ+^CwT^0aFUKx7TOPZ-a(SONMcZ0CMY~RWP)9Z5`epi045$VU2HOpq4JHk>3_}f%89p;& z7`YkkF={jVZfs)AGd^cLW}<8oWKwR@XUa77Fx_u@#|&#`W47JwirH6lWAh~Q3+8{X z)Lt32^7P8lRcfn3SDjilWT9xmwm4xiuv&g~(CXu>2P_pV*_M@-Z>*H8LaeH+Myxfg zBdyO^f3VTD;n_6Ue6cmR-D-Qo4zY8z%dzXSr`!A5m)gH}P;>})sB@TbG;-YRc*6dB+ao4#pmQddTunO+GC@BY)_y+u_Cn`GTB+&qp>Lxm##hSW!eO zN-i2Gb}eo$Q7$=HGFKW=+FNE*R$ne%zOVet(U7B$k69f%S0Pi8Tk-8U_xQ6Djwc!` zRVxcm;!dWVe0$38RL8HTzt&bsRpnNFua2!AsPU?4uQjQyJuQ7YuMVzDt{XcOc;?Yr zyR*&bw9lQWm#EME4g8k;+q?5Y=bv71xzKje^y0ZoDwoO|7!5g%P-A-IL{mi5K=Zoh z?#s@X+get(H2kjfd-WBCD`i*3ujXANUCX-0zn*b@>PGyH_pRZrLv5SdUfx`L^GUmB zd)FS~FBtKPuHvHT+_ zf+dC-=+7gF+=NP#LO%Z!stxfP0DSM`^Jkm*{O?t$)_DQIjp+q_Er=Gs34kPBfZE^a z?-0!YG(LY2?66PcqU7A%fF`;`09c3)1eE7VK~ks)TA<<#EfZodNc~?iFaUeF=nt8N z7=dsVm_Q^7=r7Fhaab6^JdOylwjh-M64(M5f&_0EeWQk85rN?_7zB&Qp{f9t4R`{X zh$oN;I2@5mB$3G!3I$K3icl#cC?pG#2<}DZ@5Pb{I0E{Q{(ouw8-PYYH69az6ah?w z5E{g92XZJAEUMB_6O=132u3hiv?8<)G@wBYEgwN(v{)<#Z3MIxT?ZH%mad>@jbnJS z@rnsd{merr2ue2RTgAP)CzK6>c{_jf_o9&CG4>>>V7PSkB%) zzJC6wKIMdjhJ{CPBNLO7Q&Q8?Gj?U|&fc?kUrzqvf+K}R#U-VcCr|xaRb5ki`ohIa z4UJ9Bms{Fyw%@wlap&&6$30JapY}a_{$lXW(4WI^M@GlqO@93Jd20I0%-3%MUJyVQ zXbGA98!sBl3r6<9S(10cQ{$V7}tS+U!*x#(a@blj@T9&Qj zx$8@k`xBD#l(+0lnoKXkcZ*BsZ)>iPnDLzKah@5U@OPaoYAcZ)ckJzKBNkIKr61dj zfQq4Cb_ezz2rxS7<(E5}V)9z)QqKp9KV4?11~IK{t_ptdlF$0{@avpiH8Ue`{Quf8 ztGxD)nc$(;M_Wv;7T#Z*^-#QV7cnfyF758aqOShI%MYDr%La{p?5>+z)l)qahdm=D zUFm+&F)mhquc+RxU|k>?(@|~qFk`M@Vq}iy{buizjiTq*bG1aZl2zsul=&bvI{N0M z__y>~@j11=Yxr%QuO%m%+WYO^wY)9V+5Moq{K4C}mQtUucO=`Ka(ni38lo)nw6G8U zlIgv~2TB*8tfF7+H+b%(=rF>SY@MF2Y#Ke2=d(9JT~7V>_JBi6f(N|1t*}f_D>?r2 F{{k(OZz})* literal 0 HcmV?d00001 diff --git a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/jpeg/JPEGSegmentUtil.java b/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/jpeg/JPEGSegmentUtil.java index 818716f5..5ebce5b7 100644 --- a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/jpeg/JPEGSegmentUtil.java +++ b/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> segmentIdentifiers) throws IOException { + static JPEGSegment readSegment(final ImageInputStream stream, final Map> 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 get(Object key) { + public List 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 get(Object key) { + public List get(final Object key) { return containsKey(key) ? ALL_IDS : null; } diff --git a/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/jpeg/JPEGSegmentUtilTest.java b/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/jpeg/JPEGSegmentUtilTest.java index 443341dd..8d961e05 100644 --- a/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/jpeg/JPEGSegmentUtilTest.java +++ b/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 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()); + } } diff --git a/imageio/imageio-metadata/src/test/resources/jpeg/jfif-padded-segments.jpg b/imageio/imageio-metadata/src/test/resources/jpeg/jfif-padded-segments.jpg new file mode 100755 index 0000000000000000000000000000000000000000..b7cfd8e6d7771b6c77e87374c7e415aa85405529 GIT binary patch literal 4326 zcmb_f2UJs8w?6mY^a7-Sg7jVl=}nPdLXi%NsD#i$Cv>bRqJkreNLLgcil~f*B4Yzl zEQlf^HXM*aKpY2D5CzS;@&OKlD*>`7u`~ep8_-!06p4*T>mlbzh9t*geGzTfIQIU;E+U442mzIBa0KyO9B8f zMg7XDN$~<4h+-xWEfB@=0vx`8vjjMJ0T<85_;}f(F=qfEP}po$dBXpJ+3a9pPBuGSh!?n^ccNcx z$4!inW~a}8`2W1bL?@&Dnm1YWN=c0Jb`i{I&(b(-d+$XY5y}$m*}OkK$;N9DbCXy; zi#Q_L$#)SaNBLSW;;1+mVSH@x8uvv%C(#z|{X))+2p|7N91?10zleEpUc&gq6mOwF zBf@sgVoW64S+L*pwXu09`WFt3b`*YZe3FN-&e-TRLM~xE2VqQT;!nOw5k7+bo6niV z^AW~`aUEET{s^9v&>tT?f2QZ-dC6YFK7_{l3VRsBwiEUicmsW42y}rS@J|X&6PyoV z8yBC>ACLs$r!N8Ri`gIO89n>c z7lfYa#v=eoRQ&X*C;%Y3AAsfoPBJfLAs>ZJdg^ufJ~4La=}4x1eAgba1zvjv)}?~0#`sQxDD=uN1zwH1Os3g zjDra<4QA0l9C(NdF(64u4pM=%AbrRbS`FDlu8v6gmOb zLg%5&P%Cs7>V}>}gU}fC5&8xrFa>79GO#MV0yc%MVHem3-T;Tg32+9S1s{Nm;Y#=n z+z7YA_u*c603L^@5P*;o2}A+WMobY~#2pDh!Vn&^4aq?YkxJwo(t>m#J;(s^9{GyF zVHg;Bj5cN^#tGw#;b0Ol+c5hvrI;E_Bc>hGgBiq3V18h!SZS;l)*Q>iuEmC9Q?c3D zLTnYb0ecJEiyg*J<8U|$oEpvq=Y(5_i^Ofl?Z+L()#Gm9x^Y9eDLfu8iPyqg;N9_T zd=fqzUxGh_zm9*5AI8rR$OJipKEZ(yK!_#mBoq=(6Rr^+6GjNLL@}ZY(VXZ`3?Zfy z^N6R2EyRb!Vd5-_Mp7eLkbFpyq)bvV=^W`6=@sb{nM_tBn~^=qT=EWbG5I(09r7Ug zD}_$cq}WmdDan)rlxoTi%5%ymDwV2AwW0=4lc)!%wbYx`*VGvihKP;`OC&^OyGWTx zqsU{C528d-6;W%^jiMQ%g`yWlABw&gBZ{et*^05nwuzOAT^8#To1!sjdNg-h3~fK{ zH0>^Jl#ZvX((UOX^ep;G`c3)}17RpJY#AKJF2*UwEyget$5dmon32p}=2>PJb3&Xh zZYb_2o+e%j3SA1bifW3!iaQnS75kM)N~TI-N(D-7N)yU*%5KVAlV{wP^(jWrA}41R8LT^RDYs@(=gSD(m1N|NE6mH z(u~k7(|o7}YZ+^CwT^0aFUKx7TOPZ-a(SONMcZ0CMY~RWP)9Z5`epi045$VU2HOpq4JHk>3_}f%89p;& z7`YkkF={jVZfs)AGd^cLW}<8oWKwR@XUa77Fx_u@#|&#`W47JwirH6lWAh~Q3+8{X z)Lt32^7P8lRcfn3SDjilWT9xmwm4xiuv&g~(CXu>2P_pV*_M@-Z>*H8LaeH+Myxfg zBdyO^f3VTD;n_6Ue6cmR-D-Qo4zY8z%dzXSr`!A5m)gH}P;>})sB@TbG;-YRc*6dB+ao4#pmQddTunO+GC@BY)_y+u_Cn`GTB+&qp>Lxm##hSW!eO zN-i2Gb}eo$Q7$=HGFKW=+FNE*R$ne%zOVet(U7B$k69f%S0Pi8Tk-8U_xQ6Djwc!` zRVxcm;!dWVe0$38RL8HTzt&bsRpnNFua2!AsPU?4uQjQyJuQ7YuMVzDt{XcOc;?Yr zyR*&bw9lQWm#EME4g8k;+q?5Y=bv71xzKje^y0ZoDwoO|7!5g%P-A-IL{mi5K=Zoh z?#s@X+get(H2kjfd-WBCD`i*3ujXANUCX-0zn*b@>PGyH_pRZrLv5SdUfx`L^GUmB zd)FS~FBtKPuHvHT+_ zf+dC-=+7gF+=NP#LO%Z!stxfP0DSM`^Jkm*{O?t$)_DQIjp+q_Er=Gs34kPBfZE^a z?-0!YG(LY2?66PcqU7A%fF`;`09c3)1eE7VK~ks)TA<<#EfZodNc~?iFaUeF=nt8N z7=dsVm_Q^7=r7Fhaab6^JdOylwjh-M64(M5f&_0EeWQk85rN?_7zB&Qp{f9t4R`{X zh$oN;I2@5mB$3G!3I$K3icl#cC?pG#2<}DZ@5Pb{I0E{Q{(ouw8-PYYH69az6ah?w z5E{g92XZJAEUMB_6O=132u3hiv?8<)G@wBYEgwN(v{)<#Z3MIxT?ZH%mad>@jbnJS z@rnsd{merr2ue2RTgAP)CzK6>c{_jf_o9&CG4>>>V7PSkB%) zzJC6wKIMdjhJ{CPBNLO7Q&Q8?Gj?U|&fc?kUrzqvf+K}R#U-VcCr|xaRb5ki`ohIa z4UJ9Bms{Fyw%@wlap&&6$30JapY}a_{$lXW(4WI^M@GlqO@93Jd20I0%-3%MUJyVQ zXbGA98!sBl3r6<9S(10cQ{$V7}tS+U!*x#(a@blj@T9&Qj zx$8@k`xBD#l(+0lnoKXkcZ*BsZ)>iPnDLzKah@5U@OPaoYAcZ)ckJzKBNkIKr61dj zfQq4Cb_ezz2rxS7<(E5}V)9z)QqKp9KV4?11~IK{t_ptdlF$0{@avpiH8Ue`{Quf8 ztGxD)nc$(;M_Wv;7T#Z*^-#QV7cnfyF758aqOShI%MYDr%La{p?5>+z)l)qahdm=D zUFm+&F)mhquc+RxU|k>?(@|~qFk`M@Vq}iy{buizjiTq*bG1aZl2zsul=&bvI{N0M z__y>~@j11=Yxr%QuO%m%+WYO^wY)9V+5Moq{K4C}mQtUucO=`Ka(ni38lo)nw6G8U zlIgv~2TB*8tfF7+H+b%(=rF>SY@MF2Y#Ke2=d(9JT~7V>_JBi6f(N|1t*}f_D>?r2 F{{k(OZz})* literal 0 HcmV?d00001