Fix bug #45290 - Support odd files where the POIFS header block comes after the data...

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

diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml
index ff277d20dda0a12d0f3490bc75da184d04ef508b..a0e36343a71ee73d33b5259bae364f20232cd56c 100644 (file)
--- a/src/documentation/content/xdocs/changes.xml
+++ b/src/documentation/content/xdocs/changes.xml
@@ -37,6 +37,7 @@
 
                <!-- Don't forget to update status.xml too! -->
         <release version="3.5-beta4" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45290 - Support odd files where the POIFS header block comes after the data blocks, and is on the data blocks list</header>
            <action dev="POI-DEVELOPERS" type="fix">46184 - More odd escaped date formats</action>
            <action dev="POI-DEVELOPERS" type="add">Include the sheet number in the output of XLS2CSVmra</action>
            <action dev="POI-DEVELOPERS" type="fix">46043 - correctly write out HPSF properties with HWPF</action>

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 18922b442b68e93af6300f5cab821ffa23fe6835..4d2b1a99741e9f013590545b167098ce54b27e12 100644 (file)
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,6 +34,7 @@
        <!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.5-beta4" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45290 - Support odd files where the POIFS header block comes after the data blocks, and is on the data blocks list</header>
            <action dev="POI-DEVELOPERS" type="fix">46184 - More odd escaped date formats</action>
            <action dev="POI-DEVELOPERS" type="add">Include the sheet number in the output of XLS2CSVmra</action>
            <action dev="POI-DEVELOPERS" type="fix">46043 - correctly write out HPSF properties with HWPF</action>

diff --git a/src/java/org/apache/poi/poifs/eventfilesystem/POIFSReader.java b/src/java/org/apache/poi/poifs/eventfilesystem/POIFSReader.java
index 73911e6b0e05c9e27cb7f04dbf55cdd865cfabb0..6a1423ea48e45aade1959c32869201a8bf2ad503 100644 (file)
--- a/src/java/org/apache/poi/poifs/eventfilesystem/POIFSReader.java
+++ b/src/java/org/apache/poi/poifs/eventfilesystem/POIFSReader.java
@@ -245,13 +245,13 @@ public class POIFSReader
                     {
                         document =
                             new POIFSDocument(name, small_blocks
-                                .fetchBlocks(startBlock), size);
+                                .fetchBlocks(startBlock, -1), size);
                     }
                     else
                     {
                         document =
                             new POIFSDocument(name, big_blocks
-                                .fetchBlocks(startBlock), size);
+                                .fetchBlocks(startBlock, -1), size);
                     }
                     while (listeners.hasNext())
                     {
@@ -270,11 +270,11 @@ public class POIFSReader
                     // consume the document's data and discard it
                     if (property.shouldUseSmallBlocks())
                     {
-                        small_blocks.fetchBlocks(startBlock);
+                        small_blocks.fetchBlocks(startBlock, -1);
                     }
                     else
                     {
-                        big_blocks.fetchBlocks(startBlock);
+                        big_blocks.fetchBlocks(startBlock, -1);
                     }
                 }
             }

diff --git a/src/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java b/src/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java
index 92348ceaffc22eb7f1770144c6169708ec6054c4..6ea37ea3879deefc4861498773b645ea1019b273 100644 (file)
--- a/src/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java
+++ b/src/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java
@@ -173,11 +173,16 @@ public class POIFSFileSystem
                               data_blocks);
 
         // init documents
-        processProperties(SmallBlockTableReader
-            .getSmallDocumentBlocks(data_blocks, properties
-                .getRoot(), header_block_reader
-                    .getSBATStart()), data_blocks, properties.getRoot()
-                        .getChildren(), null);
+        processProperties(
+                       SmallBlockTableReader.getSmallDocumentBlocks(
+                                       data_blocks, properties.getRoot(), 
+                                       header_block_reader.getSBATStart()
+                       ), 
+                       data_blocks, 
+                       properties.getRoot().getChildren(), 
+                       null,
+                       header_block_reader.getPropertyStart()
+        );
     }
     /**
      * @param stream the stream to be closed
@@ -491,7 +496,8 @@ public class POIFSFileSystem
     private void processProperties(final BlockList small_blocks,
                                    final BlockList big_blocks,
                                    final Iterator properties,
-                                   final DirectoryNode dir)
+                                   final DirectoryNode dir,
+                                   final int headerPropertiesStartAt)
         throws IOException
     {
         while (properties.hasNext())
@@ -511,7 +517,8 @@ public class POIFSFileSystem
 
                 processProperties(
                     small_blocks, big_blocks,
-                    (( DirectoryProperty ) property).getChildren(), new_dir);
+                    (( DirectoryProperty ) property).getChildren(), 
+                    new_dir, headerPropertiesStartAt);
             }
             else
             {
@@ -522,14 +529,15 @@ public class POIFSFileSystem
                 if (property.shouldUseSmallBlocks())
                 {
                     document =
-                        new POIFSDocument(name, small_blocks
-                            .fetchBlocks(startBlock), size);
+                        new POIFSDocument(name, 
+                                          small_blocks.fetchBlocks(startBlock, headerPropertiesStartAt), 
+                                          size);
                 }
                 else
                 {
                     document =
                         new POIFSDocument(name,
-                                          big_blocks.fetchBlocks(startBlock),
+                                          big_blocks.fetchBlocks(startBlock, headerPropertiesStartAt),
                                           size);
                 }
                 parent.createDocument(document);

diff --git a/src/java/org/apache/poi/poifs/property/PropertyTable.java b/src/java/org/apache/poi/poifs/property/PropertyTable.java
index 11c56e886df3745f8cc52957e200a44b5dc3c8fc..00b306b7923577f06e35ffe445987f44e0d09098 100644 (file)
--- a/src/java/org/apache/poi/poifs/property/PropertyTable.java
+++ b/src/java/org/apache/poi/poifs/property/PropertyTable.java
@@ -78,7 +78,7 @@ public class PropertyTable
         _blocks      = null;
         _properties  =
             PropertyFactory
-                .convertToProperties(blockList.fetchBlocks(startBlock));
+                .convertToProperties(blockList.fetchBlocks(startBlock, -1));
         populatePropertyTree(( DirectoryProperty ) _properties.get(0));
     }
 

diff --git a/src/java/org/apache/poi/poifs/storage/BlockAllocationTableReader.java b/src/java/org/apache/poi/poifs/storage/BlockAllocationTableReader.java
index b167d2fca1cd7cf680d21da5cc028019f5bcde26..1016db9c46341833dc33fd67d1afab564def4d76 100644 (file)
--- a/src/java/org/apache/poi/poifs/storage/BlockAllocationTableReader.java
+++ b/src/java/org/apache/poi/poifs/storage/BlockAllocationTableReader.java
@@ -179,17 +179,34 @@ public class BlockAllocationTableReader
      */
 
     ListManagedBlock [] fetchBlocks(final int startBlock,
+                                    final int headerPropertiesStartBlock,
                                     final BlockList blockList)
         throws IOException
     {
         List blocks       = new ArrayList();
         int  currentBlock = startBlock;
-
-        while (currentBlock != POIFSConstants.END_OF_CHAIN)
-        {
-            blocks.add(blockList.remove(currentBlock));
-            currentBlock = _entries.get(currentBlock);
+        boolean firstPass = true;
+        
+        // Process the chain from the start to the end
+        // Normally we have header, data, end
+        // Sometimes we have data, header, end
+        // For those cases, stop at the header, not the end
+        while (currentBlock != POIFSConstants.END_OF_CHAIN) {
+               try {
+                       blocks.add(blockList.remove(currentBlock));
+                currentBlock = _entries.get(currentBlock);
+               } catch(IOException e) {
+                       if(currentBlock == headerPropertiesStartBlock) {
+                               // Special case where things are in the wrong order
+                               System.err.println("Warning, header block comes after data blocks in POIFS block listing");
+                               currentBlock = POIFSConstants.END_OF_CHAIN;
+                       } else {
+                               // Ripple up
+                               throw e;
+                       }
+               }
         }
+        
         return ( ListManagedBlock [] ) blocks
             .toArray(new ListManagedBlock[ 0 ]);
     }

diff --git a/src/java/org/apache/poi/poifs/storage/BlockList.java b/src/java/org/apache/poi/poifs/storage/BlockList.java
index 2352d3a2c1e303547e2b44032716f9a97d3bddb8..a5eb7df2173465f0d7eb40e6dca1a6dd24d82c80 100644 (file)
--- a/src/java/org/apache/poi/poifs/storage/BlockList.java
+++ b/src/java/org/apache/poi/poifs/storage/BlockList.java
@@ -59,13 +59,14 @@ public interface BlockList
      * blocks are removed from the list.
      *
      * @param startBlock the index of the first block in the stream
+     * @param headerPropertiesStartBlock the index of the first header block in the stream
      *
      * @return the stream as an array of correctly ordered blocks
      *
      * @exception IOException if blocks are missing
      */
 
-    public ListManagedBlock [] fetchBlocks(final int startBlock)
+    public ListManagedBlock [] fetchBlocks(final int startBlock, final int headerPropertiesStartBlock)
         throws IOException;
 
     /**

diff --git a/src/java/org/apache/poi/poifs/storage/BlockListImpl.java b/src/java/org/apache/poi/poifs/storage/BlockListImpl.java
index 7e44fda3f35b31978c07fae340256f7d130ec872..f315b5d515c0291a451886aae713b23bcda10279 100644 (file)
--- a/src/java/org/apache/poi/poifs/storage/BlockListImpl.java
+++ b/src/java/org/apache/poi/poifs/storage/BlockListImpl.java
@@ -94,8 +94,10 @@ class BlockListImpl
             result = _blocks[ index ];
             if (result == null)
             {
-                throw new IOException("block[ " + index
-                                      + " ] already removed");
+                throw new IOException(
+                               "block[ " + index + " ] already removed - " +
+                               "does your POIFS have circular or duplicate block references?"
+                );
             }
             _blocks[ index ] = null;
         }
@@ -119,7 +121,7 @@ class BlockListImpl
      * @exception IOException if blocks are missing
      */
 
-    public ListManagedBlock [] fetchBlocks(final int startBlock)
+    public ListManagedBlock [] fetchBlocks(final int startBlock, final int headerPropertiesStartBlock)
         throws IOException
     {
         if (_bat == null)
@@ -127,7 +129,7 @@ class BlockListImpl
             throw new IOException(
                 "Improperly initialized list: no block allocation table provided");
         }
-        return _bat.fetchBlocks(startBlock, this);
+        return _bat.fetchBlocks(startBlock, headerPropertiesStartBlock, this);
     }
 
     /**

diff --git a/src/java/org/apache/poi/poifs/storage/SmallBlockTableReader.java b/src/java/org/apache/poi/poifs/storage/SmallBlockTableReader.java
index e70774aa16d4e66f74bf2b0cb311d9e465b0dc37..4da79e71f1e23071990504214e090c114c7e494b 100644 (file)
--- a/src/java/org/apache/poi/poifs/storage/SmallBlockTableReader.java
+++ b/src/java/org/apache/poi/poifs/storage/SmallBlockTableReader.java
@@ -56,9 +56,9 @@ public class SmallBlockTableReader
     {
         BlockList list =
             new SmallDocumentBlockList(SmallDocumentBlock
-                .extract(blockList.fetchBlocks(root.getStartBlock())));
+                .extract(blockList.fetchBlocks(root.getStartBlock(), -1)));
 
-        new BlockAllocationTableReader(blockList.fetchBlocks(sbatStart),
+        new BlockAllocationTableReader(blockList.fetchBlocks(sbatStart, -1),
                                        list);
         return list;
     }

diff --git a/src/testcases/org/apache/poi/hssf/data/45290.xls b/src/testcases/org/apache/poi/hssf/data/45290.xls
new file mode 100644 (file)
index 0000000..dd06449
Binary files /dev/null and b/src/testcases/org/apache/poi/hssf/data/45290.xls differ

diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
index 024f94e5b785cab31d725ab37fea2a3c2ef05cab..402ee797a91976cee5d9b0a20bd04efb4ef296fe 100644 (file)
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
@@ -27,9 +27,6 @@ import java.util.List;
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
-import org.apache.poi.ss.util.Region;
-import org.apache.poi.ss.util.CellRangeAddress;
-
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.model.Workbook;
 import org.apache.poi.hssf.record.CellValueRecordInterface;
@@ -38,6 +35,7 @@ import org.apache.poi.hssf.record.NameRecord;
 import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate;
 import org.apache.poi.hssf.record.formula.DeletedArea3DPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.ss.util.CellRangeAddress;
 import org.apache.poi.util.TempFile;
 
 /**
@@ -1533,4 +1531,13 @@ public final class TestBugs extends TestCase {
         assertEquals(7, wb.getNumberOfSheets());
         wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
     }
+    
+    /**
+     * Odd POIFS blocks issue:
+     * block[ 44 ] already removed from org.apache.poi.poifs.storage.BlockListImpl.remove
+     */
+    public void test45290() {
+        HSSFWorkbook wb = openSample("45290.xls");
+        assertEquals(1, wb.getNumberOfSheets());
+    }
 }

diff --git a/src/testcases/org/apache/poi/poifs/storage/TestBlockAllocationTableReader.java b/src/testcases/org/apache/poi/poifs/storage/TestBlockAllocationTableReader.java
index a209de6c8d6231e35b50b9e9c2f4140434394478..d688ce14597380b06323d7f1f4df1e8a6228a572 100644 (file)
--- a/src/testcases/org/apache/poi/poifs/storage/TestBlockAllocationTableReader.java
+++ b/src/testcases/org/apache/poi/poifs/storage/TestBlockAllocationTableReader.java
@@ -1325,7 +1325,7 @@ public class TestBlockAllocationTableReader
             try
             {
                 ListManagedBlock[] dataBlocks =
-                    table.fetchBlocks(start_blocks[ j ], list);
+                    table.fetchBlocks(start_blocks[ j ], -1, list);
 
                 if (expected_length[ j ] == -1)
                 {

diff --git a/src/testcases/org/apache/poi/poifs/storage/TestBlockListImpl.java b/src/testcases/org/apache/poi/poifs/storage/TestBlockListImpl.java
index 6b8d091a5096047dfa974da67d4bb31d7d3ab6b2..194a951a1d793af50d21b714d202f86087d48976 100644 (file)
--- a/src/testcases/org/apache/poi/poifs/storage/TestBlockListImpl.java
+++ b/src/testcases/org/apache/poi/poifs/storage/TestBlockListImpl.java
@@ -285,7 +285,7 @@ public class TestBlockListImpl
             try
             {
                 ListManagedBlock[] dataBlocks =
-                    list.fetchBlocks(start_blocks[ j ]);
+                    list.fetchBlocks(start_blocks[ j ], -1);
 
                 if (expected_length[ j ] == -1)
                 {