Bug 58156: Possible data corruption in hasPOIFSHeader and hasOOXMLHeader

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1691948 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
index c27ba3cd1404782c70083ad55f58f1cfb8e28cf3..9817c3d8186a8ff6cace1207d6beb416276bad53 100644 (file)
--- a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
+++ b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
@@ -354,7 +354,11 @@ public class NPOIFSFileSystem extends BlockStore
      *  has a POIFS (OLE2) header at the start of it.
      * If your InputStream does not support mark / reset,
      *  then wrap it in a PushBackInputStream, then be
-     *  sure to always use that, and not the original!
+     *  sure to always use that and not the original!
+     *  
+     *  After the method call, the InputStream is at the
+     *  same position as of the time of entering the method.
+     *  
      * @param inp An InputStream which supports either mark/reset, or is a PushbackInputStream
      */
     public static boolean hasPOIFSHeader(InputStream inp) throws IOException {
@@ -362,13 +366,13 @@ public class NPOIFSFileSystem extends BlockStore
         inp.mark(8);
 
         byte[] header = new byte[8];
-        IOUtils.readFully(inp, header);
+        int bytesRead = IOUtils.readFully(inp, header);
         LongField signature = new LongField(HeaderBlockConstants._signature_offset, header);
 
         // Wind back those 8 bytes
         if(inp instanceof PushbackInputStream) {
             PushbackInputStream pin = (PushbackInputStream)inp;
-            pin.unread(header);
+            pin.unread(header, 0, bytesRead);
         } else {
             inp.reset();
         }

diff --git a/src/java/org/apache/poi/util/IOUtils.java b/src/java/org/apache/poi/util/IOUtils.java
index f1d5a2378d77ef901e3936dc5170203660da62db..b5f22a1a1a2b558b2319a1a85a465132cddd073b 100644 (file)
--- a/src/java/org/apache/poi/util/IOUtils.java
+++ b/src/java/org/apache/poi/util/IOUtils.java
@@ -57,7 +57,7 @@ public final class IOUtils {
         // Wind back those 8 bytes
         if(stream instanceof PushbackInputStream) {
             PushbackInputStream pin = (PushbackInputStream)stream;
-            pin.unread(header);
+            pin.unread(header, 0, read);
         } else {
             stream.reset();
         }

diff --git a/src/ooxml/java/org/apache/poi/POIXMLDocument.java b/src/ooxml/java/org/apache/poi/POIXMLDocument.java
index 0352d5c8741290c0ad528c75ea1f1966091d8192..b0b5d4454f0e99999e228246419a233c41eb1d1b 100644 (file)
--- a/src/ooxml/java/org/apache/poi/POIXMLDocument.java
+++ b/src/ooxml/java/org/apache/poi/POIXMLDocument.java
@@ -127,18 +127,19 @@ public abstract class POIXMLDocument extends POIXMLDocumentPart implements Close
         inp.mark(4);
 
         byte[] header = new byte[4];
-        IOUtils.readFully(inp, header);
+        int bytesRead = IOUtils.readFully(inp, header);
 
         // Wind back those 4 bytes
         if(inp instanceof PushbackInputStream) {
             PushbackInputStream pin = (PushbackInputStream)inp;
-            pin.unread(header);
+            pin.unread(header, 0, bytesRead);
         } else {
             inp.reset();
         }
 
         // Did it match the ooxml zip signature?
         return (
+                bytesRead == 4 &&
                 header[0] == POIFSConstants.OOXML_FILE_HEADER[0] &&
                 header[1] == POIFSConstants.OOXML_FILE_HEADER[1] &&
                 header[2] == POIFSConstants.OOXML_FILE_HEADER[2] &&

diff --git a/src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java b/src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java
index 4ea22134188be96de552d2139ce10ce95eec8619..ff4f5dccd8c947ac0a675987bb11e7071652c037 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java
+++ b/src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java
@@ -19,8 +19,10 @@
 
 package org.apache.poi;
 
+import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 import java.io.PushbackInputStream;
+import java.util.Arrays;
 
 import junit.framework.TestCase;
 
@@ -62,4 +64,22 @@ public class TestDetectAsOOXML extends TestCase
                assertFalse(POIXMLDocument.hasOOXMLHeader(in));
                in.close();
        }
+    
+    public void testFileCorruption() throws Exception {
+           
+           // create test InputStream
+           byte[] testData = { (byte)1, (byte)2, (byte)3 };
+        ByteArrayInputStream testInput = new ByteArrayInputStream(testData);
+        
+        // detect header
+        InputStream in = new PushbackInputStream(testInput, 10);
+        assertFalse(POIXMLDocument.hasOOXMLHeader(in));
+        
+        // check if InputStream is still intact
+        byte[] test = new byte[3];
+        in.read(test);
+        assertTrue(Arrays.equals(testData, test));
+        assertEquals(-1, in.read());
+       }
+
 }

diff --git a/src/testcases/org/apache/poi/poifs/filesystem/TestOffice2007XMLException.java b/src/testcases/org/apache/poi/poifs/filesystem/TestOffice2007XMLException.java
index d475b7aad0e27372e0c47bd0c4115da7aa4eaed4..d5d92bd6bb64ebe2d4182ac8c3d8cfefe2aee3b6 100644 (file)
--- a/src/testcases/org/apache/poi/poifs/filesystem/TestOffice2007XMLException.java
+++ b/src/testcases/org/apache/poi/poifs/filesystem/TestOffice2007XMLException.java
@@ -17,12 +17,16 @@
 
 package org.apache.poi.poifs.filesystem;
 
-import junit.framework.TestCase;
-
-import java.io.*;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PushbackInputStream;
+import java.util.Arrays;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
 
+import junit.framework.TestCase;
+
 /**
  * Class to test that POIFS complains when given an Office 2007 XML document
  *
@@ -38,7 +42,7 @@ public class TestOffice2007XMLException extends TestCase {
                InputStream in = openSampleStream("sample.xlsx");
 
                try {
-                       new POIFSFileSystem(in);
+                       new POIFSFileSystem(in).close();
                        fail("expected exception was not thrown");
                } catch(OfficeXmlFileException e) {
                        // expected during successful test
@@ -72,4 +76,39 @@ public class TestOffice2007XMLException extends TestCase {
                    in.close();
                }
        }
+    
+    public void testFileCorruption() throws Exception {
+        
+        // create test InputStream
+        byte[] testData = { (byte)1, (byte)2, (byte)3 };
+        InputStream testInput = new ByteArrayInputStream(testData);
+        
+        // detect header
+        InputStream in = new PushbackInputStream(testInput, 10);
+        assertFalse(POIFSFileSystem.hasPOIFSHeader(in));
+        
+        // check if InputStream is still intact
+        byte[] test = new byte[3];
+        in.read(test);
+        assertTrue(Arrays.equals(testData, test));
+        assertEquals(-1, in.read());
+    }
+
+
+    public void testFileCorruptionOPOIFS() throws Exception {
+        
+        // create test InputStream
+        byte[] testData = { (byte)1, (byte)2, (byte)3 };
+        InputStream testInput = new ByteArrayInputStream(testData);
+        
+        // detect header
+        InputStream in = new PushbackInputStream(testInput, 10);
+        assertFalse(OPOIFSFileSystem.hasPOIFSHeader(in));
+
+        // check if InputStream is still intact
+        byte[] test = new byte[3];
+        in.read(test);
+        assertTrue(Arrays.equals(testData, test));
+        assertEquals(-1, in.read());
+    }
 }