Fix bug #45290 - Support odd files where the POIFS header block comes after the data blocks, and is on the data blocks list

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

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>

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>

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);
                     }
                 }
             }

src/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java

@@ -173,11 +173,16 @@ public class POIFSFileSystem
                               data_blocks);
 
         // init documents
-        processProperties(SmallBlockTableReader
+        processProperties(
-            .getSmallDocumentBlocks(data_blocks, properties
+        		SmallBlockTableReader.getSmallDocumentBlocks(
-                .getRoot(), header_block_reader
+        				data_blocks, properties.getRoot(), 
-                    .getSBATStart()), data_blocks, properties.getRoot()
+        				header_block_reader.getSBATStart()
-                        .getChildren(), null);
+        		), 
+        		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
+                        new POIFSDocument(name, 
-                            .fetchBlocks(startBlock), size);
+                                          small_blocks.fetchBlocks(startBlock, headerPropertiesStartAt), 
+                                          size);
                 }
                 else
                 {
                     document =
                         new POIFSDocument(name,
-                                          big_blocks.fetchBlocks(startBlock),
+                                          big_blocks.fetchBlocks(startBlock, headerPropertiesStartAt),
                                           size);
                 }
                 parent.createDocument(document);

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));
     }
 

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;
+        boolean firstPass = true;
         
-        while (currentBlock != POIFSConstants.END_OF_CHAIN)
+        // 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 ]);
     }

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;
 
     /**

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
+                throw new IOException(
-                                      + " ] already removed");
+                		"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);
     }
 
     /**

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;
     }

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());
+    }
 }

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)
                 {

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)
                 {