From 4aa8334e3b695cc34a2bfb035b9cc35dc9271e73 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 3 Jan 2020 13:11:41 +0000 Subject: [PATCH] Fix some additional file-handle-leaks reported by unit-tests via an enhanced version of the file-leak-detector Also add a test for Bug 64045 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1872287 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/poi/TestAllFiles.java | 1 + .../org/apache/poi/xslf/util/EMFHandler.java | 6 +- .../apache/poi/xssf/XSSFMemoryLeakTests.java | 25 ++++++++ .../TestXSSFBEventBasedExcelExtractor.java | 16 ++--- .../xssf/model/TestSharedStringsTable.java | 6 +- .../poi/xssf/usermodel/TestXSSFBugs.java | 8 +++ .../apache/poi/hssf/usermodel/TestBugs.java | 57 +++++++++--------- test-data/spreadsheet/xlsx-corrupted.xlsx | Bin 0 -> 6842 bytes 8 files changed, 76 insertions(+), 43 deletions(-) create mode 100644 test-data/spreadsheet/xlsx-corrupted.xlsx diff --git a/src/integrationtest/org/apache/poi/TestAllFiles.java b/src/integrationtest/org/apache/poi/TestAllFiles.java index 2374c9e6d4..8f9ba82a4c 100644 --- a/src/integrationtest/org/apache/poi/TestAllFiles.java +++ b/src/integrationtest/org/apache/poi/TestAllFiles.java @@ -317,6 +317,7 @@ public class TestAllFiles { "spreadsheet/poc-shared-strings.xlsx", // contains shared-string-entity-expansion "document/61612a.docx", "document/word2.doc", + "spreadsheet/xlsx-corrupted.xlsx", // old Excel files, which we only support simple text extraction of "spreadsheet/testEXCEL_2.xls", diff --git a/src/ooxml/java/org/apache/poi/xslf/util/EMFHandler.java b/src/ooxml/java/org/apache/poi/xslf/util/EMFHandler.java index 94fb64447e..9d7e620bb4 100644 --- a/src/ooxml/java/org/apache/poi/xslf/util/EMFHandler.java +++ b/src/ooxml/java/org/apache/poi/xslf/util/EMFHandler.java @@ -43,8 +43,9 @@ class EMFHandler extends MFProxy { @Override public void parse(File file) throws IOException { - // stream needs to be kept open - parse(file.toURI().toURL().openStream()); + // stream needs to be kept open until the instance is closed + is = file.toURI().toURL().openStream(); + parse(is); } @Override @@ -66,7 +67,6 @@ class EMFHandler extends MFProxy { // } } } - } protected String getContentType() { diff --git a/src/ooxml/testcases/org/apache/poi/xssf/XSSFMemoryLeakTests.java b/src/ooxml/testcases/org/apache/poi/xssf/XSSFMemoryLeakTests.java index 958e54554c..c987e4a91b 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/XSSFMemoryLeakTests.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/XSSFMemoryLeakTests.java @@ -17,6 +17,9 @@ package org.apache.poi.xssf; +import org.apache.poi.ooxml.POIXMLException; +import org.apache.poi.openxml4j.exceptions.InvalidFormatException; +import org.apache.poi.openxml4j.opc.OPCPackage; import org.apache.poi.util.MemoryLeakVerifier; import org.apache.poi.xssf.usermodel.XSSFCell; import org.apache.poi.xssf.usermodel.XSSFRow; @@ -27,12 +30,14 @@ import org.junit.Test; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCell; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.IOException; import java.io.OutputStream; import java.util.ArrayList; import java.util.List; import static org.junit.Assert.assertSame; +import static org.junit.Assert.fail; /** * A test which uses {@link MemoryLeakVerifier} to ensure that certain @@ -141,4 +146,24 @@ public class XSSFMemoryLeakTests { wb1.close(); } + + @Test(expected = POIXMLException.class) + public void testFileLeak() throws IOException, InvalidFormatException { + File file = XSSFTestDataSamples.getSampleFile("xlsx-corrupted.xlsx"); + verifier.addObject(file); + try (XSSFWorkbook ignored = new XSSFWorkbook(file)) { + fail("Should catch exception as the file is corrupted"); + } + } + + @Test(expected = POIXMLException.class) + public void testFileLeak2() throws IOException, InvalidFormatException { + File file = XSSFTestDataSamples.getSampleFile("xlsx-corrupted.xlsx"); + verifier.addObject(file); + try (OPCPackage pkg = OPCPackage.open(file)) { + try (XSSFWorkbook ignored = new XSSFWorkbook(pkg)) { + fail("Should catch exception as the file is corrupted"); + } + } + } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/extractor/TestXSSFBEventBasedExcelExtractor.java b/src/ooxml/testcases/org/apache/poi/xssf/extractor/TestXSSFBEventBasedExcelExtractor.java index 226a76ead6..025cf49dc8 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/extractor/TestXSSFBEventBasedExcelExtractor.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/extractor/TestXSSFBEventBasedExcelExtractor.java @@ -113,17 +113,17 @@ public class TestXSSFBEventBasedExcelExtractor { extractor.setIncludeCellComments(true); String[] rows = extractor.getText().split("[\r\n]+"); assertEquals(283, rows.length); - BufferedReader reader = Files.newBufferedReader(XSSFTestDataSamples.getSampleFile("62815.xlsb.txt").toPath(), - StandardCharsets.UTF_8); - String line = reader.readLine(); - for (String row : rows) { - assertEquals(line, row); - line = reader.readLine(); - while (line != null && line.startsWith("#")) { + try (BufferedReader reader = Files.newBufferedReader(XSSFTestDataSamples.getSampleFile("62815.xlsb.txt").toPath(), + StandardCharsets.UTF_8)) { + String line = reader.readLine(); + for (String row : rows) { + assertEquals(line, row); line = reader.readLine(); + while (line != null && line.startsWith("#")) { + line = reader.readLine(); + } } } } } - } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/model/TestSharedStringsTable.java b/src/ooxml/testcases/org/apache/poi/xssf/model/TestSharedStringsTable.java index f78af2f044..0963a27798 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/model/TestSharedStringsTable.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/model/TestSharedStringsTable.java @@ -27,6 +27,7 @@ import java.nio.file.Path; import java.util.List; import java.util.function.Predicate; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.ss.usermodel.Workbook; @@ -233,11 +234,12 @@ public final class TestSharedStringsTable { Path path = XSSFTestDataSamples.getSampleFile("48936-strings.txt").toPath(); - List lst = Files - .lines(path, StandardCharsets.UTF_8) + Stream lines = Files.lines(path, StandardCharsets.UTF_8); + List lst = lines .map(String::trim) .filter(((Predicate)String::isEmpty).negate()) .collect(Collectors.toList()); + lines.close(); for (String str : lst) { s.createRow(i++).createCell(0).setCellValue(str); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java index 9eb2ed9dc7..65eaa7f173 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -3455,4 +3455,12 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { }*/ } } + + @Test(expected = POIXMLException.class) + public void test64045() throws IOException, InvalidFormatException { + File file = XSSFTestDataSamples.getSampleFile("xlsx-corrupted.xlsx"); + try (XSSFWorkbook ignored = new XSSFWorkbook(file)) { + fail("Should catch exception as the file is corrupted"); + } + } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index 29f6e22f19..060f665bc7 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java @@ -2914,43 +2914,40 @@ public final class TestBugs extends BaseTestBugzillaIssues { @Test public void test46515() throws IOException { - Workbook wb = HSSFTestDataSamples.openSampleWorkbook("46515.xls"); + try (Workbook wb = HSSFTestDataSamples.openSampleWorkbook("46515.xls")) { - // Get structure from webservice - String urlString = "http://poi.apache.org/components/spreadsheet/images/calendar.jpg"; - URL structURL = new URL(urlString); - BufferedImage bimage; - try { - bimage = ImageIO.read(structURL); - } catch (IOException e) { - Assume.assumeNoException("Downloading a jpg from poi.apache.org should work", e); - return; - } finally { - wb.close(); - } + // Get structure from webservice + String urlString = "http://poi.apache.org/components/spreadsheet/images/calendar.jpg"; + URL structURL = new URL(urlString); + BufferedImage bimage; + try { + bimage = ImageIO.read(structURL); + } catch (IOException e) { + Assume.assumeNoException("Downloading a jpg from poi.apache.org should work", e); + return; + } - // Convert BufferedImage to byte[] - ByteArrayOutputStream imageBAOS = new ByteArrayOutputStream(); - ImageIO.write(bimage, "jpeg", imageBAOS); - imageBAOS.flush(); - byte[] imageBytes = imageBAOS.toByteArray(); - imageBAOS.close(); + // Convert BufferedImage to byte[] + ByteArrayOutputStream imageBAOS = new ByteArrayOutputStream(); + ImageIO.write(bimage, "jpeg", imageBAOS); + imageBAOS.flush(); + byte[] imageBytes = imageBAOS.toByteArray(); + imageBAOS.close(); - // Pop structure into Structure HSSFSheet - int pict = wb.addPicture(imageBytes, HSSFWorkbook.PICTURE_TYPE_JPEG); - Sheet sheet = wb.getSheet("Structure"); - assertNotNull("Did not find sheet", sheet); - HSSFPatriarch patriarch = (HSSFPatriarch) sheet.createDrawingPatriarch(); - HSSFClientAnchor anchor = new HSSFClientAnchor(0, 0, 0, 0, (short) 1, 1, (short) 10, 22); - anchor.setAnchorType(AnchorType.MOVE_DONT_RESIZE); - patriarch.createPicture(anchor, pict); + // Pop structure into Structure HSSFSheet + int pict = wb.addPicture(imageBytes, HSSFWorkbook.PICTURE_TYPE_JPEG); + Sheet sheet = wb.getSheet("Structure"); + assertNotNull("Did not find sheet", sheet); + HSSFPatriarch patriarch = (HSSFPatriarch) sheet.createDrawingPatriarch(); + HSSFClientAnchor anchor = new HSSFClientAnchor(0, 0, 0, 0, (short) 1, 1, (short) 10, 22); + anchor.setAnchorType(AnchorType.MOVE_DONT_RESIZE); + patriarch.createPicture(anchor, pict); - // Write out destination file + // Write out destination file // FileOutputStream fileOut = new FileOutputStream("/tmp/46515.xls"); // wb.write(fileOut); // fileOut.close(); - - wb.close(); + } } @Test diff --git a/test-data/spreadsheet/xlsx-corrupted.xlsx b/test-data/spreadsheet/xlsx-corrupted.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..0e363e7fed99619549c856f3fce0f16fd73ee370 GIT binary patch literal 6842 zcma)=1z3~q_s2(fhe&rfh)75(C<!OOskO-xKic>j zV)i`GaQa@DMNex-c1+Oy@^wN-h5Gs=U6SL*@@vY&*i6LIHsgIBQyxnd3*gDh_qb8T zF7Z?p3emnpaGmOcj;+48LY86xppXyudQ|mbxnscg}w~gVcWV^@WEC^YyP5iuQW^=t3Qpz%Ke; zAwV1od8^70TJ4f22bldbtvBBH15MEkH+`a-{R-8wKXNg<Y7^Jc7%Sw}ONH$U|dhbh1g3kawwD!JLq^*SK=%qq$QmounZ=D@1AaWIOk*fdz z$l5alzkT@ef`eQIFw7Ka53_X;_?G*?H3Fjy!!v|?yz=%-U{ana8Ma+AdZ1s;9VWZ#jISEAs*}|m zJ{4=dZ~FTW-#_q_l+>qVf{=MCB@nPsxKdPXA|e`g`gf8wFXoO8ViM6^kz?1lYrm@z z6!liN1h;iDf|htLV6d2Q8Vn6t3l#~NXQfQq6%ET)|4cqS-@?)eDb5vSaV4mWxX*aN z0*%10JSMiRCvoGB=xuLk)d*AQz6h(!N zj>A+jrQ}?`+rCb{h0=2=g^GR#lno2T5CTs!~ z03bP=G*g&8!B>}9pyx(>z7A!k#>9wn9a zGQDU0V%m^Hb8_#V)54l8%|I7e>j)ip-fU5)g3nU(@+3lP zM7H0}2Sp@cr1QSh(r%P%719LyS765aOvjG+mAvW1l6J@|ek9FV!%5eDqw%r{X_XC4 zg`I%Y5!u%&PraS&LwV<-S)a1`G3*8E#7Auon2wy6g`aP>n>NYnlE+AW?~1fu=OnrC;bd(}E;Ame$lF-KkV<4&&u9B0FnwO1Url3}7K7xayJ zs0K~<7iG+*ehGyw;k-~VkqdU$9%B7NB@}#wMyJOgRPtSDr{{Ay8ORADRdRaZ{XC!l zl=4k#r+^E;L!@Mo`yit;#NNRI29;(O;umCQgg{MUU<>GdY34hc*ZIVmWv=2XsDVIt zfST77e+B*j)s2SyQ&;O>y8eS2&zC4_;!bfQvOEt+004sTCp$R0SwkHDtvuHf)nF5h zR4+FWnj99Yn^-BzqLzS+0=iM1o>wPNnAj}D&A2A)!{x^$Q|+TwE@4AlJv=7^$8De_$~ zaw$>dlCSU=^T}ZN-oWJ!x}we&|Cvogj~MTEDpuZbT+T};QYosy8D8RtH)xgY*(lqn z;|k&{?q$bD2{h#~_OVmBeF(qV`uYNQAv-OW9l5BF`0Kb2w(Mhgc3`8oWHSI9!x~n< z!`*nZv?!8R*M-hfvKT*ewDPUVWjQHJqI<4_lPhhx!FKRmi?BvwCAQK8%UT~y%jKS@ z4-HgwJR=L|j)_yPK;bWki~{R?>=@!yjmvi~jH_@$ShZt5BkUtY+6X#Rw{@j^w8B_a zMey#u`O+4z74Pe>Q-rQLeidr2w$3f+DSxHeL2^&2D#;7`t<$c1es>)TR+}5_Y1*); zvhEklOxliR3Y&*35@1F{y95=vYE)QdnP}-wYte!@vkfD6oSA@+GhX6ln#oj+9#W2j z0~7Gl_yg?(S6o;3ksJfRF}CRL3VN!hBzcj88cn~bRDzB>UzCSNz^$N9Aw-q@u)tY+M9Q59U(a9opCN1QHhC@Vit+x0E zWZk&422y6h#)C#yyQ>E)w&@Kg^{a>*aP+w{N#g$?pjl$r+9!FwU{inJR zE$QvbV$xqQkQII9c!44ivtDVoXjf&>@sMe1JKBt=Hb;(QW!G8O@bSX4JT|m(mb`Ii zO!J958P0(Zy^UWrG0;J?Jd%XEg0bJ&Cq{`34EmT%7x4>YZ7=N~#%#i(cUMUkMyK=R z7p_JwKk40(=^0)GL-{4BU^w+0nGKMh1Kx=#;iBsdYeoo^^VyPb$UK@tX3GwlE%G1x ziMg>o1gzm`kKE1vbz6xal-59FBF3DC-)*&q)#wz&&M1?Z7F~N1%!Zai+gEBZ)>v?2 z)T^U&MW;-vr{}S*caC~$scr9YyZwN#{Ff_=61$(vJQ|v?I%hd+gvfS32jg3LKoYOL z)0(BnjJC+Vgg}X%yOtKU+{^TMK8LVJeM|E#20bxb_+j1()X6?t_CP9jcuW!~(m52C zlHNvIJY)Wb4cr;Kb*a9jO{WiAE2`7H^tyMp%yj#Z+1_D;O&P)~C3HK$XxZzM#r1S9 z(Sew$6a(AYWwo)|M8quhDDnfL}h!U~5hiZWniC!KCd+fdCU3dC&wTR0_bezjwFUwe# zt5m|>;+|Xg-t2Jyh!Eq+oUM9p1)S%qC=`#esA_sf@cBamK<9=Gu3zb0yG)%>L6H;x z%uspoi^2CzsE_!9P`abXCP2dpd>f_C<;hC5O%n3nyoo5<1f{L*?j&cK zYS~)&QuFO>svOb@=S0-R_4Ztm7bgfX>|!9M=8Ad!)^zJL>$JHyf@X*bZoy(yAkYCwi+`a$_xRI5K*Ui`N1{Lr3A1xyl3;%MS8jI6Nrv$WCwzu!Je@o?aoN`%B65=UACbeHPT##)BypDC4JHDN7AyTS7HTKpUFOsh*48tYc~Dp z;;#4gTQU8)$tV0(Ob+G{h~t?~{|~ivz9<{CjZjjN1=)oS08stL`Gynzx8|Tx>$by$ z7;)pCwykYfAq87i8Cjso%jl=B3fL8BLVYCRSB&o6oE;FKp zb~a5`xA+gbOmSbNs}VyT_UmQWAhnA}BeQH~OlKHrZe))x)fD2E+r)624QS|ja3eB< z;dXazxS)?P9!O9BZf9w?ijlgAZ!(Vom#p^itmcNPhLvj4lxDYhlB&RL2<|ochdf6t zLK&QjecDl%DYV7V2Mj==ODUk?0C1sF&5$}%&cY#?;H~jM_BAXQYcWbjOrm&9BdO#n~E|riJI^;Vp)HTeLzS&YD-& zS%mM|8u)$nv1Z(yd*odc0MF=b>cEG&8Y7|-)~gJ@e6Zg|H5<|i=A6j4#43wyah0R7 zFeD7Zsw-y@T)0~&+I=dl+>}IWX&>{{v1UBx|$sLEc#$sl60J8Pns&S z9$tJj&}vkDi*b{oCYypxh26CxJ^FLs;2@6gYnlhsq({>8V*$4UaAz+N>xNWZJXynz zs}RekUkEP+dXe-dG(IB_v)eH=AkrIvMKpsW6lVDI>x0!wNFoM_EyYrL(TGqc^DD9v z%nAccbB;;?^rUoeJqT65V8c)3LSD)6knp(Ju79Q4@J*Stkpja8_oqp%rQwm%aeK-} z?!5h`x9++Z_et}+M^f=@^r#&#eu*vbFyp9gmT^f+ANF3i(=(K^bbR)xEM;8fo@Q1- zhBFa!1q1r0VL$mXE_ob1DgQD}es%3ydOAZ-+TyPM3$qz{=>>2+^aP{4yx5-6$6H$U z0bEAVlbYVxo@4*vjTYw7DSfU+F+vnvfMoKk#+b{OmX+S@Xi3d=y83s7S3D&i*U49| zzFoOBaRYd?%iK3^@f?TM-uUvksfhT~z?dQeVIL(~Jk}N&Q9O`;RWhz-QEJY?T&2m~ zo5qMhR+HoiMMTGctGuu!U)gW8~oxUR?z&oMDA>d2K2ixUo3#D(*6S40cmtw74X z1Lv=D7y5U(w?Zexi5oYxu_d;LE3NZY%Lvr8E8pqb$vK6KdL@6Mz1=hRmFBtD2AXHR6Ug#YJZ7^$C-uuA74Hl(~ z?9cYYp%}r}K5VwVmk>;zkf?>hsUK?|)eG8ZzmSU)JLXX{B*&hEadh#@qf{XapWT~P z{sdzJxl`3du1W!O&T$>;4&uSJj6|yOvI=5E&pEVvL;^qiJVzzW+oDaRCE^JwkLjgd zYS&A*aqf0^LuJko2qWIW5q-+U&ov1mE76Ee0FTBZ44(bJ6uk(s=#j*~9 zrPhmKN1ILWy4~8-UVJGNtzR#(CSgeJ-i$De_M}vW$JBbfQzXF*EA}YJX6lq&IW)ib zVBc@}UC=nGV|rw+?m>z|E;NpDQpQ&!9yKN6lbK8`TuCKca0CQzp=X_G>?sM4D;5bd zs!=wHd9x4W$N=Zsu~{k)z1HDv!hvp$B;fVQH7<`z@{OtGPdQ$XY->zezvgNG$Tns~ zGq&4YY)4`k?xn0^rb}uig+8ACG&u)L6ARWr5_X9!BzN|4YojdkkQIT@Acqxf%9a zxHcmbwQg&65M04b2UXnRJm}q4jWS*G*miZaa}%&Ckc2Hsslb&BS|`3PU{$yPy?FF zj^yvT(TW~89)WmOOG4e<(y{jDNudweypLaw3WOivzL!Z z>*#Wm4vf8A%sD0SbuWd1VtkKio2Ui7T6A)w^xpEC^s0Hoiu=$UyT*wFoe~H2D;=z@ z6RK2pU+<=^3Ic6fr>z_a0*m(m=@6oz(xd&&sii`;@BjePx&q|*o__kHZ;SEWxBbl# zIR*c@FTp`}6yG1jfHk=QB>#*RiNu`_Pk+Sy&4@f3XZKm$Ph0Z8QO`SdXQ{`?{`_Z< zzkIuY!v6Aueq8LgH_UU_{v2tD|8EjSvi|5S{mSF--qP6+H-55yjG_My{M|P?yD(r3 zz;_qvpRV%PlAQOD&PJB`9dy=F`X}UnDHG9C-mpw@K4NN zDLy|ooDH=6H|E(P;=j?)cPVG-SxDm&nc&kj{D}D5+mD{*Z*&aIQ%w>fpI?#UDWEw0 F_J3+7!<7I4 literal 0 HcmV?d00001