From 7e2a5dacee9ec0a842190db6541ae97f084b49ae Mon Sep 17 00:00:00 2001 From: Yegor Kozlov Date: Thu, 20 Aug 2009 13:49:33 +0000 Subject: [PATCH] Avoid exception when reading ClipboardData packet in OLE property sets, see bugzilla 45583 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@806172 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/status.xml | 2 +- src/java/org/apache/poi/POIDocument.java | 23 +++++++++++------- src/java/org/apache/poi/hpsf/Section.java | 10 -------- .../org/apache/poi/hpsf/VariantSupport.java | 17 ++++++++++--- .../extractor/HPSFPropertiesExtractor.java | 2 +- .../src/org/apache/poi/hdgf/HDGFDiagram.java | 3 --- .../org/apache/poi/hslf/HSLFSlideShow.java | 3 --- .../src/org/apache/poi/hwpf/HWPFDocument.java | 1 - .../TestHPSFPropertiesExtractor.java | 10 ++++++++ .../org/apache/poi/hssf/data/42726.xls | Bin 0 -> 98816 bytes 10 files changed, 40 insertions(+), 31 deletions(-) create mode 100755 src/testcases/org/apache/poi/hssf/data/42726.xls diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 480b0a1a2..bded2bc4a 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -33,7 +33,7 @@ - 47668 - Improved parsing of OOXML documents + 45583 - Avoid exception when reading ClipboardData packet in OLE property sets 47652 - Added support for reading encrypted workbooks 47604 - Implementation of an XML to XLSX Importer using Custom XML Mapping 47620 - Avoid FormulaParseException in XSSFWorkbook.setRepeatingRowsAndColumns when removing repeated rows and columns diff --git a/src/java/org/apache/poi/POIDocument.java b/src/java/org/apache/poi/POIDocument.java index 4d7e50c0f..15571601b 100644 --- a/src/java/org/apache/poi/POIDocument.java +++ b/src/java/org/apache/poi/POIDocument.java @@ -47,19 +47,19 @@ import org.apache.poi.util.POILogger; */ public abstract class POIDocument { /** Holds metadata on our document */ - protected SummaryInformation sInf; + private SummaryInformation sInf; /** Holds further metadata on our document */ - protected DocumentSummaryInformation dsInf; + private DocumentSummaryInformation dsInf; /** The open POIFS FileSystem that contains our document */ protected POIFSFileSystem filesystem; /** The directory that our document lives in */ protected DirectoryNode directory; /** For our own logging use */ - protected POILogger logger = POILogFactory.getLogger(this.getClass()); + private final static POILogger logger = POILogFactory.getLogger(POIDocument.class); /* Have the property streams been read yet? (Only done on-demand) */ - protected boolean initialized = false; + private boolean initialized = false; protected POIDocument(DirectoryNode dir, POIFSFileSystem fs) { @@ -120,7 +120,10 @@ public abstract class POIDocument { * if it wasn't found */ protected PropertySet getPropertySet(String setName) { - DocumentInputStream dis; + //directory can be null when creating new documents + if(directory == null) return null; + + DocumentInputStream dis; try { // Find the entry, and get an input stream for it dis = directory.createDocumentInputStream(setName); @@ -157,14 +160,16 @@ public abstract class POIDocument { * @param writtenEntries a list of POIFS entries to add the property names too */ protected void writeProperties(POIFSFileSystem outFS, List writtenEntries) throws IOException { - if(sInf != null) { - writePropertySet(SummaryInformation.DEFAULT_STREAM_NAME,sInf,outFS); + SummaryInformation si = getSummaryInformation(); + if(si != null) { + writePropertySet(SummaryInformation.DEFAULT_STREAM_NAME, si, outFS); if(writtenEntries != null) { writtenEntries.add(SummaryInformation.DEFAULT_STREAM_NAME); } } - if(dsInf != null) { - writePropertySet(DocumentSummaryInformation.DEFAULT_STREAM_NAME,dsInf,outFS); + DocumentSummaryInformation dsi = getDocumentSummaryInformation(); + if(dsi != null) { + writePropertySet(DocumentSummaryInformation.DEFAULT_STREAM_NAME, dsi, outFS); if(writtenEntries != null) { writtenEntries.add(DocumentSummaryInformation.DEFAULT_STREAM_NAME); } diff --git a/src/java/org/apache/poi/hpsf/Section.java b/src/java/org/apache/poi/hpsf/Section.java index a9ea717c0..525450e79 100644 --- a/src/java/org/apache/poi/hpsf/Section.java +++ b/src/java/org/apache/poi/hpsf/Section.java @@ -240,16 +240,6 @@ public class Section { ple = propertyList.get(propertyCount - 1); ple.length = size - ple.offset; - if (ple.length <= 0) - { - final StringBuffer b = new StringBuffer(); - b.append("The property set claims to have a size of "); - b.append(size); - b.append(" bytes. However, it exceeds "); - b.append(ple.offset); - b.append(" bytes."); - throw new IllegalPropertySetDataException(b.toString()); - } } /* Look for the codepage. */ diff --git a/src/java/org/apache/poi/hpsf/VariantSupport.java b/src/java/org/apache/poi/hpsf/VariantSupport.java index 1b06e239a..e96e6739d 100644 --- a/src/java/org/apache/poi/hpsf/VariantSupport.java +++ b/src/java/org/apache/poi/hpsf/VariantSupport.java @@ -272,9 +272,20 @@ public class VariantSupport extends Variant } case Variant.VT_CF: { + if(l1 < 0) { + /** + * YK: reading the ClipboardData packet (VT_CF) is not quite correct. + * The size of the data is determined by the first four bytes of the packet + * while the current implementation calculates it in the Section constructor. + * Test files in Bugzilla 42726 and 45583 clearly show that this approach does not always work. + * The workaround below attempts to gracefully handle such cases instead of throwing exceptions. + * + * August 20, 2009 + */ + l1 = LittleEndian.getInt(src, o1); o1 += LittleEndian.INT_SIZE; + } final byte[] v = new byte[l1]; - for (int i = 0; i < l1; i++) - v[i] = src[(o1 + i)]; + System.arraycopy(src, o1, v, 0, v.length); value = v; break; } @@ -509,7 +520,7 @@ public class VariantSupport extends Variant } case Variant.VT_CF: { - final byte[] b = (byte[]) value; + final byte[] b = (byte[]) value; out.write(b); length = b.length; break; diff --git a/src/java/org/apache/poi/hpsf/extractor/HPSFPropertiesExtractor.java b/src/java/org/apache/poi/hpsf/extractor/HPSFPropertiesExtractor.java index 1a6b12614..450e8d3b4 100644 --- a/src/java/org/apache/poi/hpsf/extractor/HPSFPropertiesExtractor.java +++ b/src/java/org/apache/poi/hpsf/extractor/HPSFPropertiesExtractor.java @@ -55,7 +55,7 @@ public class HPSFPropertiesExtractor extends POITextExtractor { text.append( getPropertiesText(dsi) ); // Now custom ones - CustomProperties cps = dsi.getCustomProperties(); + CustomProperties cps = dsi == null ? null : dsi.getCustomProperties(); if(cps != null) { Iterator keys = cps.keySet().iterator(); while(keys.hasNext()) { diff --git a/src/scratchpad/src/org/apache/poi/hdgf/HDGFDiagram.java b/src/scratchpad/src/org/apache/poi/hdgf/HDGFDiagram.java index 483762484..aa15ce1b1 100644 --- a/src/scratchpad/src/org/apache/poi/hdgf/HDGFDiagram.java +++ b/src/scratchpad/src/org/apache/poi/hdgf/HDGFDiagram.java @@ -68,9 +68,6 @@ public final class HDGFDiagram extends POIDocument { _docstream = new byte[docProps.getSize()]; dir.createDocumentInputStream("VisioDocument").read(_docstream); - // Read in the common POI streams - readProperties(); - // Check it's really visio String typeString = new String(_docstream, 0, 20); if(! typeString.equals(VISIO_HEADER)) { diff --git a/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java b/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java index 90b2cfd3e..df4d291f5 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java +++ b/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java @@ -146,9 +146,6 @@ public final class HSLFSlideShow extends POIDocument { // Now, build records based on the PowerPoint stream buildRecords(); - // Look for Property Streams: - readProperties(); - // Look for any other streams readOtherStreams(); diff --git a/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocument.java b/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocument.java index 8b6d2fdae..db4acdd09 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocument.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocument.java @@ -170,7 +170,6 @@ public final class HWPFDocument extends POIDocument { // Sort out the hpsf properties super(directory, pfilesystem); - readProperties(); // read in the main stream. DocumentEntry documentProps = (DocumentEntry) diff --git a/src/testcases/org/apache/poi/hpsf/extractor/TestHPSFPropertiesExtractor.java b/src/testcases/org/apache/poi/hpsf/extractor/TestHPSFPropertiesExtractor.java index e05cbc6ba..2bdc3c08d 100644 --- a/src/testcases/org/apache/poi/hpsf/extractor/TestHPSFPropertiesExtractor.java +++ b/src/testcases/org/apache/poi/hpsf/extractor/TestHPSFPropertiesExtractor.java @@ -25,6 +25,7 @@ import junit.framework.TestCase; import org.apache.poi.hssf.extractor.ExcelExtractor; import org.apache.poi.hssf.usermodel.HSSFWorkbook; +import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.poifs.filesystem.POIFSFileSystem; public final class TestHPSFPropertiesExtractor extends TestCase { @@ -117,4 +118,13 @@ public final class TestHPSFPropertiesExtractor extends TestCase { assertTrue(fsText.indexOf("AUTHOR = marshall") > -1); assertTrue(fsText.indexOf("TITLE = Titel: \u00c4h") > -1); } + + public void test42726() throws Exception { + HPSFPropertiesExtractor ex = new HPSFPropertiesExtractor(HSSFTestDataSamples.openSampleWorkbook("42726.xls")); + String txt = ex.getText(); + assertTrue(txt.indexOf("PID_AUTHOR") != -1); + assertTrue(txt.indexOf("PID_EDITTIME") != -1); + assertTrue(txt.indexOf("PID_REVNUMBER") != -1); + assertTrue(txt.indexOf("PID_THUMBNAIL") != -1); + } } diff --git a/src/testcases/org/apache/poi/hssf/data/42726.xls b/src/testcases/org/apache/poi/hssf/data/42726.xls new file mode 100755 index 0000000000000000000000000000000000000000..e492c5d3d92bb8c7978b5f6e87d562deb5d40cbc GIT binary patch literal 98816 zcmeI5TX0=f8OPT?x%BixPnwphAoWloy`{Am6)Kb_y;plPt=0<)NpsSKrb$gwq$rZM z)C(2`tAbdCYQ0pHiwLNIlqY?_k#WR9ADH6!Afpe#(Q%lejsI`$v$rS7$;tV?oZ|)m zJM-Ur?X~{jT5JE-+Iz2k`p{>8HSNrahc0;04co4874F>NL|2`YpFsL&(Rqz?avnhE z1_uYD?0zJ0%I$xIz}aj+Weh7Zc2$s!`2;8lO@yRuGLBQA^Ps6v3c?g{)1X>tIy3`1 zADRih4thQG2IvClLguLoptaCCXg#z6+6cW9 zdKa__+6--hwnE#WcSB9kc4!Cm9_YQ$PUwBm`=Jj&AB3)kJ_LOjx(2!yYKB^%T~I5O zf!d(mP&?ECT?g%fI-$K#7t{^ygFXWFK)p~Ov>&=2`Y3b*bO5>$`WWZ3@nGfCeqQ_mc~|2}gSoM~ zxpP(_2s+tj$}jDM4!bblXAqm^hootAv*n9u)Parttk`TV2hORf3*uigBUV49Z8 z%We_p1tEul;ra7M$*&(JfBq=>6G!A9OW-c+cY|`Q>96p|Y5mpySleIekJI}n_~VTJ zs=;e9cLR3>Gx-^LbmQLL-k!mOvv;%4St>bq%Tg}#EIW!jI>6tbvARe(H_#%5 zu6jV$Tmcq(QlY#PL^9{HH-tY%dCT17fm)P3g_UB`9SqPw@^sLEeS@3zGM+24T1=v{ zBvymTe_+A*D%Moe4G~tFJK8&X)0l}Hdr^HOmYv)8^<aEMG>}3Weo7dIf#;{E9d)&1%7|XduL!}dI^@OtLtWO>Db${zc<~~y|=k5y`_73 zdQ*KB9@3 zKeX`qftB-e(kJjx3vi(_q=7r+>Vx_qijR%~7N-e-y2%4`!GXybc-a!Npbv*S9GYZ8 z22au}WZW_ODJOYyGCWU^=H{LCLlJ%;I2WD;7yyY4&k}w(qO%H|$%G2aDsd(Q5S~@x z48xs0tHv3oLiTI|&SZ{<&Lqx~IC~O{d}%3krhH#gnEa9>Q#Iv3PE!7pB;`LuQXnV! zb=W6;yj)Sre`KWmr$x$tP^A24M9P0Wr2HpC%6}-N{O3W+e-tF$U+__Y$u_V7bzq_m zNS{1HTokTzV6m^eGQ7>)#DS1AGIKVJk&hD)a`jgwYTcu9;jTe;+3g~!FX1m*-i=5l zc@KbfbPps<_drC~4H?OF?nq)t)gy_RcqDxj4cK4lKltR6IJ^Up<);dc9LX|`wqcBVj~N#>s*Vf;&JD_}9a)9U`P}hE7uFoRGwBx8 zzjD7k{N#ZrWgr-!NXzSaP4o2~t?3P!o}P}b-8h$g z@yPFc4N$Z-NFw>$KVf0z^ZQ@;&88KP{9IO@asGNqwglD^J`zNTc0{piD9WRTUj4jl3F=VXcWpR_Sg_0-!O$IId-V+%E13>oWqS=?l7p{9!=V;wJxn~W{g zbTMSC<7IJ^v4xs0hKzN*EN(KkP}9Ybv5uF;O~w{#x)?In@v^wd*g{PgL&iE@7B?AN zsOe(JSjWraCSwaVT?`rPcv;+JY@w!$A!8jci<^us)O0aqtm9>Ild*-GE{2SCyew`q zwouc>kg<-J#ZAT*YPuLQ*735q$=E_o7emH6UKTeQTd3(`$XLhA;wEDYHC+rD>v&n* zWNe|Piy>njFN>RuE!1=|WUS+5ag(uynl6Tnb-XNYGPY3D#gMU%m&HxS7HYZ}GS>03 zxXIW;O&3GPI$jnx8C$67V#rv>%i<0-!O z$IId-V+%E13>oWqS=?l7p{9!=V;wJxn~W{gbTMSC<7IJ^v4xs0hKzN*EN(KkP}9Yb zv5uF;O~w{#x)?In@v^wd*g{PgL&iE@7B?ANsOe(JSjWraCSwaVT?`rPcv;+JY@w!$ zA!8jci<^us)O0aqtm9>Ild*-GE{2SCyew`qwouc>kg<-J#ZAT*YPuLQ*735q$=E_o z7emH6UKTeQTd3(`$XLhA;wEDYHC+rD>v&n*WNe|Piy>njFN>RuE!1=|WUS+5ag(uy znl6Tnb-XNYGPY3D#gMU%m&HxS7HYZ}GS>03xXIW;O&3GPI$jnx8C$67V#rv>%i<$}kwl&W#8|wL`ZN4toYF4uj)_y{@5=afNYWXu0t$tgWk7MCjt-*-QRT&UM&m1Mmo*(J)?4xpqr#l6*luWis9s}Nc*{|9nd%G~ zK>{Q|0wh2JBtQZr zKmsH{0%aj^7N0DKQ1fBv7U&4{N$6A1r=ib4w?dzVjzYIVpMyRR-45LWeE~WKeG&Q+ z^kwKy=q~6h(B05iq2tikpsz#sK=(o?pl?9mgib==g1!xX2l_5_3i=*&AM}0b2hjb{ z1JHxeL(mVQhoK)qk3f$?k3m0%eggdzdK`KJdJ;N^Ykn$w9M2cqYatdGxRC$}kN^pg z011!)36KB@kN^pg011!)36KB@kN^pg011!)36KB@kN^pg011!)36KB@kN^pg011!) z36KB@kN^pg011!)36KB@kN^pg011!)36KB@kN^pg011!)36KB@kN^pg011!)36KB@ zkN^pg011!)36KB@kN^pg011!)36KB@kN^pg011!)36KB@kN^pg011!)36KB@kN^pg z011!)36KB@kN^pg011!)3FHuHa^3jrb7{BMb>Z0K4&;=Mot){aT=f572e8*96F%s4 z7`iSz4Bc1Y^z@W4Zzo#saeG{g@5i2@#v;yYT>^j3RYKB_B0{{Sb#mwo+-kJE*X?s# z(TD4b4Uc`&zFbelATB77ScI~T+42DGcDhUfcj2N*Xg_MZ*y~@6DraP?t#aGleq8@v z*X+lBBW6LH9}~&xa~()`73jj;Z2ML46x;qJc>mZcv&CWdtU(LAd>=A?2I~5Ax#9Yk Ypxq6=_0sIL{N?|jFHOr~E#qnbpTt|bQvd(} literal 0 HcmV?d00001