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

10 files changed, 40 insertions, 31 deletions

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 480b0a1a29..bded2bc4a5 100644
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -33,7 +33,7 @@
 
     <changes>
         <release version="3.5-beta7" date="2009-??-??">
-           <action dev="POI-DEVELOPERS" type="add">47668 - Improved parsing of OOXML documents</action>
+           <action dev="POI-DEVELOPERS" type="add">45583 - Avoid exception when reading ClipboardData packet in OLE property sets</action>
            <action dev="POI-DEVELOPERS" type="add">47652 - Added support for reading encrypted workbooks</action>
            <action dev="POI-DEVELOPERS" type="add">47604 - Implementation of an XML to XLSX Importer using Custom XML Mapping</action>
            <action dev="POI-DEVELOPERS" type="fix">47620 - Avoid FormulaParseException in XSSFWorkbook.setRepeatingRowsAndColumns when removing repeated rows and columns</action>
diff --git a/src/java/org/apache/poi/POIDocument.java b/src/java/org/apache/poi/POIDocument.java
index 4d7e50c0fb..15571601b4 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 a9ea717c01..525450e799 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 1b06e239aa..e96e6739df 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 1a6b126145..450e8d3b4b 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 483762484d..aa15ce1b13 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 90b2cfd3ed..df4d291f51 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 8b6d2fdae7..db4acdd095 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 e05cbc6bad..2bdc3c08d2 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 0000000000..e492c5d3d9
--- /dev/null
+++ b/src/testcases/org/apache/poi/hssf/data/42726.xls