From a95235b42279c6fcc77a64f1b441657725139d84 Mon Sep 17 00:00:00 2001 From: Harald Kuhr Date: Tue, 7 Nov 2023 14:04:28 +0100 Subject: [PATCH] #860: Fix regression in reading broken PackBits stream. --- .../java/com/twelvemonkeys/io/SubStream.java | 11 +- .../com/twelvemonkeys/io/SubStreamTest.java | 114 ++++++++++++++++++ .../plugins/psd/PSDImageReaderTest.java | 25 ++++ .../resources/broken-psd/short-packbits.psd | Bin 0 -> 10803 bytes 4 files changed, 145 insertions(+), 5 deletions(-) create mode 100644 common/common-io/src/test/java/com/twelvemonkeys/io/SubStreamTest.java create mode 100644 imageio/imageio-psd/src/test/resources/broken-psd/short-packbits.psd diff --git a/common/common-io/src/main/java/com/twelvemonkeys/io/SubStream.java b/common/common-io/src/main/java/com/twelvemonkeys/io/SubStream.java index 2b4cc2b0..6cfb389e 100755 --- a/common/common-io/src/main/java/com/twelvemonkeys/io/SubStream.java +++ b/common/common-io/src/main/java/com/twelvemonkeys/io/SubStream.java @@ -54,7 +54,7 @@ public final class SubStream extends FilterInputStream { */ public SubStream(final InputStream stream, final long length) { super(Validate.notNull(stream, "stream")); - bytesLeft = length; + bytesLeft = Validate.isTrue(length >= 0, length, "length < 0: %s"); } /** @@ -63,10 +63,11 @@ public final class SubStream extends FilterInputStream { */ @Override public void close() throws IOException { - // NOTE: Do not close the underlying stream + // NOTE: Do not close the underlying stream, but consume it while (bytesLeft > 0) { - //noinspection ResultOfMethodCallIgnored - skip(bytesLeft); + if (skip(bytesLeft) <= 0 && read() < 0) { + break; + } } } @@ -115,7 +116,7 @@ public final class SubStream extends FilterInputStream { @Override public long skip(long length) throws IOException { - long skipped = super.skip(findMaxLen(length));// Skips 0 or more, never -1 + long skipped = super.skip(findMaxLen(length)); // Skips 0 or more, never -1 bytesLeft -= skipped; return skipped; diff --git a/common/common-io/src/test/java/com/twelvemonkeys/io/SubStreamTest.java b/common/common-io/src/test/java/com/twelvemonkeys/io/SubStreamTest.java new file mode 100644 index 00000000..75a51360 --- /dev/null +++ b/common/common-io/src/test/java/com/twelvemonkeys/io/SubStreamTest.java @@ -0,0 +1,114 @@ +package com.twelvemonkeys.io; + +import org.junit.Test; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.Arrays; +import java.util.Random; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; + +/** + * SubStreamTest. + * + * @author Harald Kuhr + * @author last modified by $Author: haraldk$ + * @version $Id: SubStreamTest.java,v 1.0 07/11/2023 haraldk Exp$ + */ +public class SubStreamTest { + + private final Random rng = new Random(2918475687L); + + @SuppressWarnings("resource") + @Test(expected = IllegalArgumentException.class) + public void testCreateNullStream() { + new SubStream(null, 42); + } + + @Test(expected = IllegalArgumentException.class) + public void testCreateNegativeLength() { + new SubStream(new ByteArrayInputStream(new byte[1]), -1); + } + + @Test + public void testReadAll() throws IOException { + byte[] buf = new byte[128]; + rng.nextBytes(buf); + + try (InputStream stream = new SubStream(new ByteArrayInputStream(buf), buf.length)) { + for (byte b : buf) { + assertEquals(b, (byte) stream.read()); + } + + assertEquals(-1, stream.read()); + } + } + + @Test + public void testReadAllArray() throws IOException { + byte[] buf = new byte[128]; + rng.nextBytes(buf); + + try (InputStream stream = new SubStream(new ByteArrayInputStream(buf), buf.length)) { + byte[] temp = new byte[buf.length / 4]; + for (int i = 0; i < 4; i++) { + assertEquals(temp.length, stream.read(temp)); // Depends on ByteArrayInputStream specifics... + assertArrayEquals(Arrays.copyOfRange(buf, i * temp.length, (i + 1) * temp.length), temp); + } + + assertEquals(-1, stream.read()); + } + } + + @Test + public void testSkipAll() throws IOException { + byte[] buf = new byte[128]; + + try (InputStream stream = new SubStream(new ByteArrayInputStream(buf), buf.length)) { + assertEquals(128, stream.skip(buf.length)); // Depends on ByteArrayInputStream specifics... + assertEquals(-1, stream.read()); + } + } + + @SuppressWarnings("EmptyTryBlock") + @Test + public void testCloseConsumesAll() throws IOException { + ByteArrayInputStream stream = new ByteArrayInputStream(new byte[128]); + + try (InputStream ignore = new SubStream(stream, 128)) { + // Nothing here... + } + + assertEquals(0, stream.available()); + assertEquals(-1, stream.read()); + } + + @SuppressWarnings("EmptyTryBlock") + @Test + public void testCloseConsumesAllLongStream() throws IOException { + ByteArrayInputStream stream = new ByteArrayInputStream(new byte[256]); + + try (InputStream ignore = new SubStream(stream, 128)) { + // Nothing here... + } + + assertEquals(128, stream.available()); + assertEquals(0, stream.read()); + } + + @SuppressWarnings("EmptyTryBlock") + @Test(timeout = 500L) + public void testCloseConsumesAllShortStream() throws IOException { + ByteArrayInputStream stream = new ByteArrayInputStream(new byte[13]); + + try (InputStream ignore = new SubStream(stream, 42)) { + // Nothing here... + } + + assertEquals(0, stream.available()); + assertEquals(-1, stream.read()); + } +} \ No newline at end of file diff --git a/imageio/imageio-psd/src/test/java/com/twelvemonkeys/imageio/plugins/psd/PSDImageReaderTest.java b/imageio/imageio-psd/src/test/java/com/twelvemonkeys/imageio/plugins/psd/PSDImageReaderTest.java index 0e842816..9f6ecbb3 100755 --- a/imageio/imageio-psd/src/test/java/com/twelvemonkeys/imageio/plugins/psd/PSDImageReaderTest.java +++ b/imageio/imageio-psd/src/test/java/com/twelvemonkeys/imageio/plugins/psd/PSDImageReaderTest.java @@ -47,6 +47,7 @@ import javax.imageio.stream.ImageInputStream; import java.awt.*; import java.awt.color.*; import java.awt.image.*; +import java.io.EOFException; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -695,6 +696,30 @@ public class PSDImageReaderTest extends ImageReaderAbstractTest } } + @Test(timeout = 1000) + public void testBrokenPackBitsThrowsEOFException() throws IOException { + PSDImageReader imageReader = createReader(); + + try (ImageInputStream stream = ImageIO.createImageInputStream(getClassLoaderResource("/broken-psd/short-packbits.psd"))) { + imageReader.setInput(stream); + + assertEquals(1, imageReader.getNumImages(true)); + + assertEquals(427, imageReader.getWidth(0)); + assertEquals(107, imageReader.getHeight(0)); + + try { + imageReader.read(0); + + fail("Expected EOFException, is the test broken?"); + } + catch (EOFException expected) { + assertTrue(expected.getMessage().contains("PackBits")); + } + } + } + + final static class FakeCMYKColorSpace extends ColorSpace { FakeCMYKColorSpace() { super(ColorSpace.TYPE_CMYK, 4); diff --git a/imageio/imageio-psd/src/test/resources/broken-psd/short-packbits.psd b/imageio/imageio-psd/src/test/resources/broken-psd/short-packbits.psd new file mode 100644 index 0000000000000000000000000000000000000000..06af4b4f4f3501d463ff348b9668723b7464b2e8 GIT binary patch literal 10803 zcmeHrca&6Bw)d^-3{7lAK#)c<2#QKnlpqKq10rG=N5Q5E5+#}g!!RS}gaN~dj*2=4 zRGKC<9qCj#=bUq{TXpX}``fo#(3$n#`@S`A%^z=luWEJO-6!nd-uvux?moX+3s&Am z38FS1m;OCW>IpRytEbJLJ*#?2?f$K`d-hCOxvpl*x?St57w*`$bKUmGDXPb3eik*5 zk8L#udB(KaGiFS=ZYqZV(&xYIyfo^+7WlR88IioRux5h6qF>v7o%?Isj$;%xD~+*v z?bo)4-=nCyKT*^W!>?^){z_5(4pP)f>2LfQ%;(FdJ$rV}x#pVv`}a@XTw61h59p=O z|8>Bnk^i~y8@#DJ-lf=8-&(t2-QKNxs`;SS?AW?v@2=|IJJ;3JR!{loB>tBt{>H4| z*m3o$+6}e4YPTcQ8qBiI+c)BJZ(qN8&*mN5t2b}|&sO+foc0?Vc(Et1QK)Z5hi)W_6k)N$%t>I79sou!(n3sg5Hqtuj4pcmK$enCQz7ZwRCg@c46gyV$Q2xkfB3zrF33;!V8D121-gz%v7 z72&(WW5REQCxuPIZlPLe69$FpBC4o&(cq#{MN^7q6)h}UQFMRNhN4|X&lDXlI#P73 z=-Z;xMeRk(B3n_YC|6usTvc3MJf--?;w8m*7uOU&TKr7$OU3UOe_4F8xV2bWY%h)# zlaf9q!%HTY%qdw~a&O7TlKmxzN{*C#Uh;EETZy{FU6Ls+Egf7szI0~klG6K1A1QsZ z^rg}dOTQ~^ER~l!OVed#Wkbs*m(49(QMRsZPuZce_sYI0J71ckkX;^`71P&fc4QKiB)+-rx7`=xy(vs~lK4sd8cE1C@I# zU#a|}vawQMne5Z2&$vGG`rO}VPoG!%9PiW8$I^%HTh(`J-=%%m_kFhS(Y`19D*8tH zRrDL%Z+^cA`#sU`NWUNZN&AKRSM(p(e_{W1{h#T7w10hnZU6LussYmmtQfFuz)J(Z z8qhhwH?VBrxPglXt{-@C;HLvy20E*Xszz5Wtg5N{W7TI>ZB_0;rGv%~S~}>FK`##a z`yk1n*kuDRyWz4mmpyjb(aX+VW*b~Ic--KngSQTTb?}M7n!)+YM_fMt^4iN^xcuA8 z6_;m+Trp(cklG=KhkQ3gJtRN0dg!8|TZX;pw9WEN49&zP}+eYjh@!p7*5uqywUpeo}N3MM9%7!cb zBdbQ{H8K}OjdVclR>UXPKt7D^vk6JQn@2F2kNk%bOjlb&ds}5ZC-Bsq% zy++R(yK8QVTKJ8t~Ad&d2FT-`X|_#xw$ zj(=kO*W*nS`b@ZG!lM(8O;AlNoj7aamWdxuluZ&&nm%dMq@$ChlZBIKOx`^C!^!fi zORk=M_4cceU9Fu`Ic5HoeN(=kVxKyA>K#)LOs$(5xn|5Y_h0k+HSO1sYo}kk_1e#_ zHBK8i?e=L0rk$RaxNg#QHP;=zPIY~s>la`D?DeOvkKZu)hT0oGxig3 z<;JgW^xZW6rrMi6yU98Cs=4dtelpi~bM?(@Z~p7ewp&KsvhJ2+w>ajFp0|G9=kvVt zC(eIl{x|a@3$9(TYr)S8vJ2-de0pKytwp!qcI)9=C5r|wx_i<4i!6(;TD)=b*NbDf zO~38Q+ZvaYELpbX^(ES+BbL@IJ-#${`^?*)zP)u><+8h$y}!(H$D})U-En%kaQW@a z-&}6`-Pqr4|6Sb*YQ@qOZ>}(}9KZ6>m1piOy>sQANAL8knzrhxRqc0G-SyC2U)_~i zyuY=N!!Z2fLq`L?y&>bDQvzGZvsjw^ROwnM&i^3Fp$ZI8}*^zBDu zyO!@8*3+_tE>;?Emqx0gr8eO!WAq$6t8d`^4fW z{`O??lQmB^KQ-#9=by4aJ^$%Xo)J9r@H5TNj(+ywv+n0^d+y8Udp*D9dGUd354>?8 z^~Zbuc>3UogU=mw{^_(`M|}L^$N9f*{;Tej+desYY}~OUpO${Q?^DlbYd&lL+pNDG|9sfzFMiH_vHc6% zm#e;PJwEgJ@vnw|_3GEculIiK|K@>jWPiW)@29?<^6e+z4gT)M@2T(iejoT@?GKt0 z%T6@^IP1r6e;WJK2R~Q+eE6i`md`R!Q5kwvG#nFSNAJZ$IBLx8rQ* zoX+~LSzUGAGrCWTri*?SPZ$3znISnTohdyfn=Lynze(PpxJA*Vyj6KYwN%xu{+(K` zxm%;tKA^Sf*6V!wt@^lOpMfzxXDTrrHupClu?)8yvre>rXPaR=XTQ}ha@^%GI%}Lk z*KQZhSt~exK zN)Anak-8z(oL-r>WVU6P>|uH+{bg=eu04NG-plM~%h-3hiQH+jjF?~>0KWgr`68h3 z5up6<`BqW`sVk^aSUo&es2d1H1OkdsS?UONli+E=(};UHHHKPCJ%qK!gIJMNQcI~- z_+3feNsa$ES@qxd^;fT6#%k!ISDF9eUJbyts;2hgS{f_~t)_3CPpPU)kz^E1K)(b4OcQkb7t<7pxgB?Orm8!u4p?WJ($~6~Sb6`H# zd_Kr_G^>@Zbx}|@GXUat$Tw<;@m#Yi0@fy#@~i`_4b4g~w8#L`r>)>S+j7p4S18pj z-JBFOP(%pJCP6?*2m~yvfntsjj$yGS{}L3(&zv~vPeT>D6gm|HS$&XAobv)0n^;0% zrmBM^yZlh-^@H_=i@QJ zamrvaOV7{{)vH=t84mi;R+Y5Q1(v!LkS-B{k^Xc4Y!haNquyjPt52n%TS=hTC7XWo z5hzJpl-*}yQs@hi3g#KYBye$~97Dq}HWWplvq73AOe&4tIq;hDIfjj-2oxnl41*7= zD;^_IT^NO9a}3D?uxu^|Oeo25Y&H)0V4CC5`E)QtU_>NBfTg23K4Eb}7FcOe$IvVn zO#qih=RH6D(46V|;RhoPedCR7I^#JXh%t`l(>kIpH?y|0QnR=%PoQtArA2Q%h3=n2 zEv<3_gF_8TQ2xV`p!>#pPFrlu6}+#y&Ufg5-P2}Lzjc) zNr^$DBtD5z*+zHDV=kFd6c1Q^9(}h_f?1$2N;@$+wl4rJ9Oh1=%tNbRfeP*S zC+oduemc=ba?n?GCI?x}MLqz|Gc*TfxwGh!xfT7NXgrIZXL|f6-EC-&{d5v^=kq{1 zI)6<`?`fI=K{w`ORcm2`U_PSvMBKXQX(H*j+B%n9%K0yFr`+xDdkO z*uiDpUE#3QWwLM;PhJqA z)pWBcl}o1CmJT}E)Xb*D0r%+$ZTQgz<}+CUU!9STUBF%G`^l7Y)S>t1)c{=`@qDgX zj1$oeSxYPj+S9(I=6hVRIy6dqCPAy}FaUzEhCqcRBLi(1Ag0cNK+F*kWMsHQ6K zq)}?KS`%g;FcO`EZM9i#E(TXWXf=j_mS+?%Lun?O2QHS%LIvH`*rw|8fX+bRilnHy zrNsrlrshtaG#{z6vC_s?6`o42E}Fm`cXM;M>3k9xO+%B~s0K!C!I(6icSEAPsinz* zXLZwgC!|FUZC05LBJFWpUes^sj!I!bzB@rYXYqs%$QV{ChcL^+UY&)@yI4txMCvmV zh7sX8uk~BpPE8Eb@_cuSIM4AJ;KU4IB|#Fc&nSI)?^#9?Cz_xG7nx)meEKk`oFsJy zqe^!rNi-Bu5@N`i@NhMqVTn8jAqSBJ^Z|<@0JJnCI{K! z4)TP{u)w9#1Uone&(jh*nU6Fi36y8jgy2ccu(*zN3MGuuo|h5KGe651?KT=JK{6uuMC4PzkV)`|17EaCDy`aiF-8nUQ9E^$-I;?)8 zVjSMA23)S3niwODiqm$9PP0~jMhm(`R!t1?s1b~jxE_@ywM3f^Sg|8)MwF<5sB>;7 zXtF*V;-`WZPQ@1Roo-Om9_K|Eiyu4eJdE=aj5(Clfz_WzmC3Az=;M(B464i~@vzVM z5hfWjgCQ0*B1}wAG+D15y$o1USqQyk9EgJWc@^>69!EDV0qn8)co=k(7^2u?3MX`6 z2^3&52ovMojK&PW;}cC9J9NpQ8F+l6Oh*xh_V6%%9wun!5XQs9xOf;lqL_mPl#B)( zcxWSv1cUFOV_tBk6CPq?Qf{y@p#a!(5kF@mF)y)k{xG%$LyQeV0oDdlA7|ryQDS5L z;k+G!LB>Yh3D!oushk^pQQ8iUOx^~rbkYs(MB0hUG7dzEB1)!!l63uwg8112Ou~&S zgCVfx3osEL#)~kVKaxWj9^aKj7*8VO#FV5_Wd=K(1(;+SRq`-)I_krYxCdORBx=kT z@FV_$m#8t0Ua|p1VM1s)MEsAVo8H((m_QE!Z!=n`96A1C~N%oYM7w;g=SnlCQ=dc(iUcTn5FI1jJ z+$w%3%G=yN>@wpqdvOa1T|siXFeUEA?6*6~PCQBG(3cWlJ))u+bEy*_gqW|u^ zSbxf5kQq@=U{cC` zdeRXBd6Z-%iL|sm-5oIbz@#CH3$Ako^bnd*JtGn#X*ADM*JKsU<+Mzzl( z55yZa?(}(CN*;{0={+vB%I+3B9A14`no{dcK{eeRz?VE7mxrPb8bZ)v!iu_B=YfP6HKrOOHfumEV%x#-Blxtyc9B5xLN58uF9xGV zA_+AbI#pQt+iU<5w>T{}s?VgN1U2R~onf0ygBs0VNXt+otr4}ya#BT;DBIo*Aw3YR zAnf&7$%WL#01sA%8@(ng7IFFhpk$?7?%3lMKMu_2NilF zK$}sar(GQD?nnil{F?^iEolKHJA&3`9d?zspeBuNy0n{&d z&x;%Jf1m#zQsDphxFQk4x>Gz6lF3~nLfj?>iIBVoOI?oTi4f)z5km4@Fix4t}*xli*C)t(T|3sly9~)r4;+H}R=; zNVVWZhbjaCjm{ATJ)TFjA^?}^CD1YK4=8sqdL3t})L3&Lt5HkS%nkZ32*f<*joyTc!1 zkp;;GoL0OoAO?gKUccX_%0tTIFc|Pa>XQ#RZ1M;Md&x{0gY&jnRBztw8|0DN(3eHvRWeVO`UGiR$ z#P!q|z3v|GZBm2uLatdIS7>zYTAl?F>D3D26&vKubcZzRl<2!+ev8ko>okabV38Rl z9hec;7K6+TI7-o+^NI}e)-1kdy$-i0jnkV$`gTvW+n{Pf=4Xr*c@!^)B#VkQx7sB% z=z>A1Nfsc|pgv?bwHqXET7@i#TOwCPl`^TBXF(h)26C8-d;r2K+=^w43OBls_kW@n zJM+pO|B1zj=7caWJsM0&llj+_j4JcUg79l!Whm?p%tzjPtCm1oox-(Gb>Zf2q+7sd z#4MH~3nIe{TRPu~{6=G@Et~7&ASv-#JWjbC5{i@vL)V0GCIxV!o>6h+LUN5632qCU z(}jxyT|Siyx)G-pDbXYnCtTLm5ekc)Ml)tJ)5W&9!tOQ(y$@P6M#$nF7*jv3m04Jq zTp2?a#IH=}-O?CX&}v3!p_wer=}q~pPRG)=cvP9ndnFO@sqo;6$-VisL6wD=)JKPK zNf-?lL>5zqW%M?jh}+es&oJOp#Ii1N5XYloZdE4llw$w{A!Io!L})!8KoC_T3({R+ zK_s|CWGM+7L;A3{*o6Vm#37{!VdQ#=kOi^0fRm(flu#DLLSvvPjREl3oDK=Df!U{z zkZ#nV!2tNRSU(sf#4;?78~`KII@l(w#p=u#{5K#Ag8m~5lEbSwmqvRl@)oVZX^Dc{ zi!4aiqSNam5Yy_+ZX1h*EN9c|o#r@%Em(`piRg3|pC${O3r|F!6Ig2iW1__ya|S2! zEKE|T2}91JHUzMcOd0X8&Y^y<1-&1T?@SPPeO3g1DZ`4T5XNJ1ug1*fovbKGB6S%N z!*s=oQ{y+g9hi(MX}&W_9A_{C%nsnXSuu_}nUQ;Qp3{stMpOYiE;7lU^&zqnS&&m` z0Ns%wkziO3M4vNwcy5v<(kKM&L>$ls%z9)HC}1>v%|pONUmzoCTA<`5^JMM4fp z4&O&KO>k{EWHn0iSr&IZk{~%6d2W2)U`GWuVpT?Gu?hMzJ^vuHgfn?gyM$Mc$Vh_3 zrBaS8