Fix for 45066 - sheet encoding size mismatch problems

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@659403 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.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="add">45066 - fixed sheet encoding size mismatch problems</action>
            <action dev="POI-DEVELOPERS" type="add">45003 - Support embeded HDGF visio documents</action>
            <action dev="POI-DEVELOPERS" type="fix">45001 - Partial fix for HWPF Range.insertBefore() and Range.delete() with unicode characters</action>
            <action dev="POI-DEVELOPERS" type="fix">44977 - Support for AM/PM in excel date formats</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="add">45066 - fixed sheet encoding size mismatch problems</action>
            <action dev="POI-DEVELOPERS" type="add">45003 - Support embeded HDGF visio documents</action>
            <action dev="POI-DEVELOPERS" type="fix">45001 - Partial fix for HWPF Range.insertBefore() and Range.delete() with unicode characters</action>
            <action dev="POI-DEVELOPERS" type="fix">44977 - Support for AM/PM in excel date formats</action>

src/java/org/apache/poi/hssf/model/Sheet.java

@@ -96,7 +96,7 @@ public final class Sheet implements Model {
     protected List                       condFormatting    =     new ArrayList();
 
     /** Add an UncalcedRecord if not true indicating formulas have not been calculated */
-    protected boolean uncalced = false;
+    protected boolean _isUncalced = false;
     
     public static final byte PANE_LOWER_RIGHT = (byte)0;
     public static final byte PANE_UPPER_RIGHT = (byte)1;
@@ -162,7 +162,7 @@ public final class Sheet implements Model {
                 }
             }
             else if (rec.getSid() == UncalcedRecord.sid) {
-                retval.uncalced = true;
+                retval._isUncalced = true;
             }
             else if (rec.getSid() == DimensionsRecord.sid)
             {
@@ -329,16 +329,8 @@ public final class Sheet implements Model {
             }
         }
         retval.records = records;
-//        if (retval.rows == null)
-//        {
-//            retval.rows = new RowRecordsAggregate();
-//        }
         retval.checkCells();
         retval.checkRows();
-//        if (retval.cells == null)
-//        {
-//            retval.cells = new ValueRecordsAggregate();
-//        }
         if (log.check( POILogger.DEBUG ))
             log.log(POILogger.DEBUG, "sheet createSheet (existing file) exited");
         return retval;
@@ -816,17 +808,17 @@ public final class Sheet implements Model {
             // Once the rows have been found in the list of records, start
             //  writing out the blocked row information. This includes the DBCell references
             if (record instanceof RowRecordsAggregate) {
-              pos += ((RowRecordsAggregate)record).serialize(pos, data, cells);   // rec.length;
+              pos += ((RowRecordsAggregate)record).serialize(pos, data, cells);
             } else if (record instanceof ValueRecordsAggregate) {
               //Do nothing here. The records were serialized during the RowRecordAggregate block serialization
             } else {
-              pos += record.serialize(pos, data );   // rec.length;
+              pos += record.serialize(pos, data );
             }
 
             // If the BOF record was just serialized then add the IndexRecord
             if (record.getSid() == BOFRecord.sid) {
               // Add an optional UncalcedRecord
-              if (uncalced) {
+              if (_isUncalced) {
                   UncalcedRecord rec = new UncalcedRecord();
                   pos += rec.serialize(pos, data);
               }
@@ -837,31 +829,10 @@ public final class Sheet implements Model {
                 pos += serializeIndexRecord(k, pos, data);
               }
             }
-
-            //// uncomment to test record sizes ////
-//            System.out.println( record.getClass().getName() );
-//            byte[] data2 = new byte[record.getRecordSize()];
-//            record.serialize(0, data2 );   // rec.length;
-//            if (LittleEndian.getUShort(data2, 2) != record.getRecordSize() - 4
-//                    && record instanceof RowRecordsAggregate == false
-//                    && record instanceof ValueRecordsAggregate == false
-//                    && record instanceof EscherAggregate == false)
-//            {
-//                throw new RuntimeException("Blah!!!  Size off by " + ( LittleEndian.getUShort(data2, 2) - record.getRecordSize() - 4) + " records.");
-//            }
-
-//asd:            int len = record.serialize(pos + offset, data );
-
-            /////  DEBUG BEGIN /////
-//asd:            if (len != record.getRecordSize())
-//asd:                throw new IllegalStateException("Record size does not match serialized bytes.  Serialized size = " + len + " but getRecordSize() returns " + record.getRecordSize() + ". Record object is " + record.getClass());
-            /////  DEBUG END /////
-
-//asd:            pos += len;   // rec.length;
-
         }
-        if (log.check( POILogger.DEBUG ))
+        if (log.check( POILogger.DEBUG )) {
             log.log(POILogger.DEBUG, "Sheet.serialize returning ");
+        }
         return pos-offset;
     }
 
@@ -875,10 +846,17 @@ public final class Sheet implements Model {
       for (int j = BOFRecordIndex+1; j < records.size(); j++)
       {
         Record tmpRec = (( Record ) records.get(j));
-        if (tmpRec instanceof RowRecordsAggregate)
+        if (tmpRec instanceof UncalcedRecord) {
-          break;
+            continue;
+        }
+        if (tmpRec instanceof RowRecordsAggregate) {
+            break;
+        }
         sheetRecSize+= tmpRec.getRecordSize();
       }
+      if (_isUncalced) {
+          sheetRecSize += UncalcedRecord.getStaticRecordSize();
+      }
       //Add the references to the DBCells in the IndexRecord (one for each block)
       int blockCount = rows.getRowBlockCount();
       //Calculate the size of this IndexRecord
@@ -2017,31 +1995,33 @@ public final class Sheet implements Model {
     {
         int retval = 0;
 
-        for ( int k = 0; k < records.size(); k++ )
+        for ( int k = 0; k < records.size(); k++) {
-        {
+            Record record = (Record) records.get(k);
-            retval += ( (Record) records.get( k ) ).getRecordSize();
+            if (record instanceof UncalcedRecord) {
-        }
+                // skip the UncalcedRecord if present, it's only encoded if the isUncalced flag is set
-        //Add space for the IndexRecord
+                continue;
-        if (rows != null) {
-            final int blocks = rows.getRowBlockCount();
-            retval += IndexRecord.getRecordSizeForBlockCount(blocks);
-
-            //Add space for the DBCell records
-            //Once DBCell per block.
-            //8 bytes per DBCell (non variable section)
-            //2 bytes per row reference
-            retval += (8 * blocks);
-            for (Iterator itr = rows.getIterator(); itr.hasNext();) {
-                RowRecord row = (RowRecord)itr.next();
-                if (cells != null && cells.rowHasCells(row.getRowNumber()))
-                    retval += 2;
             }
+            retval += record.getRecordSize();
+        }
+        if (rows != null) {
+            // Add space for the IndexRecord and DBCell records
+            final int nBlocks = rows.getRowBlockCount();
+            int nRows = 0;
+            if (cells != null) {
+                for (Iterator itr = rows.getIterator(); itr.hasNext();) {
+                    RowRecord row = (RowRecord)itr.next();
+                    if (cells.rowHasCells(row.getRowNumber())) {
+                        nRows++;
+                    }
+                }
+            }
+            retval += IndexRecord.getRecordSizeForBlockCount(nBlocks);
+            retval += DBCellRecord.calculateSizeOfRecords(nBlocks, nRows);
         }
         // Add space for UncalcedRecord
-        if (uncalced) {
+        if (_isUncalced) {
             retval += UncalcedRecord.getStaticRecordSize();
         }
-
         return retval;
     }
 
@@ -2518,13 +2498,13 @@ public final class Sheet implements Model {
      * @return whether an uncalced record must be inserted or not at generation
      */
     public boolean getUncalced() {
-        return uncalced;
+        return _isUncalced;
     }
     /**
      * @param uncalced whether an uncalced record must be inserted or not at generation
      */
     public void setUncalced(boolean uncalced) {
-        this.uncalced = uncalced;
+        this._isUncalced = uncalced;
     }
 
     /**

src/java/org/apache/poi/hssf/record/DBCellRecord.java

@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -16,7 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record;
 
 import org.apache.poi.util.LittleEndian;
@@ -29,10 +27,7 @@ import org.apache.poi.util.LittleEndian;
  * @author Jason Height
  * @version 2.0-pre
  */
-
+public final class DBCellRecord extends Record {
-public class DBCellRecord
-    extends Record
-{
     public final static int BLOCK_SIZE = 32;
     public final static short sid = 0xd7;
     private int               field_1_row_offset;
@@ -46,7 +41,6 @@ public class DBCellRecord
      * Constructs a DBCellRecord and sets its fields appropriately
      * @param in the RecordInputstream to read the record from
      */
-
     public DBCellRecord(RecordInputStream in)
     {
         super(in);
@@ -78,7 +72,6 @@ public class DBCellRecord
      *
      * @param offset    offset to the start of the first cell in the next DBCell block
      */
-
     public void setRowOffset(int offset)
     {
         field_1_row_offset = offset;
@@ -108,7 +101,6 @@ public class DBCellRecord
      *
      * @return rowoffset to the start of the first cell in the next DBCell block
      */
-
     public int getRowOffset()
     {
         return field_1_row_offset;
@@ -120,7 +112,6 @@ public class DBCellRecord
      * @param index of the cell offset to retrieve
      * @return celloffset from the celloffset array
      */
-
     public short getCellOffsetAt(int index)
     {
         return field_2_cell_offsets[ index ];
@@ -131,7 +122,6 @@ public class DBCellRecord
      *
      * @return number of cell offsets
      */
-
     public int getNumCellOffsets()
     {
         return field_2_cell_offsets.length;
@@ -175,9 +165,15 @@ public class DBCellRecord
         return 8 + (getNumCellOffsets() * 2);
     }
     
-    /** Returns the size of a DBCellRecord when it needs to reference a certain number of rows*/
+    /**
-    public static int getRecordSizeForRows(int rows) {
+     *  @returns the size of the group of <tt>DBCellRecord</tt>s needed to encode
-      return 8 + (rows * 2);
+     *  the specified number of blocks and rows
+     */
+    public static int calculateSizeOfRecords(int nBlocks, int nRows) {
+        // One DBCell per block.
+        // 8 bytes per DBCell (non variable section)
+        // 2 bytes per row reference
+        return nBlocks * 8 + nRows * 2;
     }
 
     public short getSid()

src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java

@@ -15,12 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
-/*
- * HSSFWorkbook.java
- *
- * Created on September 30, 2001, 3:37 PM
- */
 package org.apache.poi.hssf.usermodel;
 
 import org.apache.poi.POIDocument;
@@ -64,7 +58,6 @@ import java.util.Stack;
  * @author  Shawn Laubach (slaubach at apache dot org)
  * @version 2.0-pre
  */
-
 public class HSSFWorkbook extends POIDocument
 {
     private static final int DEBUG = POILogger.DEBUG;
@@ -88,7 +81,7 @@ public class HSSFWorkbook extends POIDocument
      * this holds the HSSFSheet objects attached to this workbook
      */
 
-    protected ArrayList sheets;
+    protected List _sheets;
 
     /**
      * this holds the HSSFName objects attached to this workbook
@@ -142,7 +135,7 @@ public class HSSFWorkbook extends POIDocument
     {
         super(null, null);
         workbook = book;
-        sheets = new ArrayList( INITIAL_CAPACITY );
+        _sheets = new ArrayList( INITIAL_CAPACITY );
         names = new ArrayList( INITIAL_CAPACITY );
     }
 
@@ -233,7 +226,7 @@ public class HSSFWorkbook extends POIDocument
            this.directory = null;
         }
 
-        sheets = new ArrayList(INITIAL_CAPACITY);
+        _sheets = new ArrayList(INITIAL_CAPACITY);
         names  = new ArrayList(INITIAL_CAPACITY);
 
         // Grab the data from the workbook stream, however
@@ -263,7 +256,7 @@ public class HSSFWorkbook extends POIDocument
 
             HSSFSheet hsheet = new HSSFSheet(this, sheet);
 
-            sheets.add(hsheet);
+            _sheets.add(hsheet);
 
             // workbook.setSheetName(sheets.size() -1, "Sheet"+sheets.size());
         }
@@ -361,12 +354,12 @@ public class HSSFWorkbook extends POIDocument
      */
 
     public void setSheetOrder(String sheetname, int pos ) {
-        sheets.add(pos,sheets.remove(getSheetIndex(sheetname)));
+        _sheets.add(pos,_sheets.remove(getSheetIndex(sheetname)));
         workbook.setSheetOrder(sheetname, pos);
     }
 
     private void validateSheetIndex(int index) {
-        int lastSheetIx = sheets.size() - 1;
+        int lastSheetIx = _sheets.size() - 1;
         if (index < 0 || index > lastSheetIx) {
             throw new IllegalArgumentException("Sheet index (" 
                     + index +") is out of range (0.." +    lastSheetIx + ")");
@@ -380,7 +373,7 @@ public class HSSFWorkbook extends POIDocument
     public void setSelectedTab(int index) {
         
         validateSheetIndex(index);
-        int nSheets = sheets.size();
+        int nSheets = _sheets.size();
         for (int i=0; i<nSheets; i++) {
                getSheetAt(i).setSelected(i == index);
         }
@@ -398,7 +391,7 @@ public class HSSFWorkbook extends POIDocument
         for (int i = 0; i < indexes.length; i++) {
             validateSheetIndex(indexes[i]);
         }
-        int nSheets = sheets.size();
+        int nSheets = _sheets.size();
         for (int i=0; i<nSheets; i++) {
             boolean bSelect = false;
             for (int j = 0; j < indexes.length; j++) {
@@ -420,7 +413,7 @@ public class HSSFWorkbook extends POIDocument
     public void setActiveSheet(int index) {
         
         validateSheetIndex(index);
-        int nSheets = sheets.size();
+        int nSheets = _sheets.size();
         for (int i=0; i<nSheets; i++) {
              getSheetAt(i).setActive(i == index);
         }
@@ -492,19 +485,15 @@ public class HSSFWorkbook extends POIDocument
      * set the sheet name.
      * Will throw IllegalArgumentException if the name is greater than 31 chars
      * or contains /\?*[]
-     * @param sheet number (0 based)
+     * @param sheetIx number (0 based)
      */
-    public void setSheetName(int sheet, String name)
+    public void setSheetName(int sheetIx, String name)
     {
-        if (workbook.doesContainsSheetName( name, sheet ))
+        if (workbook.doesContainsSheetName( name, sheetIx )) {
             throw new IllegalArgumentException( "The workbook already contains a sheet with this name" );
-
-        if (sheet > (sheets.size() - 1))
-        {
-            throw new RuntimeException("Sheet out of bounds");
         }
-
+        validateSheetIndex(sheetIx);
-        workbook.setSheetName( sheet, name);
+        workbook.setSheetName(sheetIx, name);
     }
 
 
@@ -516,15 +505,12 @@ public class HSSFWorkbook extends POIDocument
      * or contains /\?*[]
      * @param sheet number (0 based)
      */
-    public void setSheetName( int sheet, String name, short encoding )
+    public void setSheetName(int sheetIx, String name, short encoding)
     {
-        if (workbook.doesContainsSheetName( name, sheet ))
+        if (workbook.doesContainsSheetName( name, sheetIx )) {
             throw new IllegalArgumentException( "The workbook already contains a sheet with this name" );
-
-        if (sheet > (sheets.size() - 1))
-        {
-            throw new RuntimeException("Sheet out of bounds");
         }
+        validateSheetIndex(sheetIx);
 
         switch ( encoding ) {
         case ENCODING_COMPRESSED_UNICODE:
@@ -536,51 +522,39 @@ public class HSSFWorkbook extends POIDocument
             throw new RuntimeException( "Unsupported encoding" );
         }
 
-        workbook.setSheetName( sheet, name, encoding );
+        workbook.setSheetName( sheetIx, name, encoding );
     }
 
     /**
      * get the sheet name
-     * @param sheet Number
+     * @param sheetIx Number
      * @return Sheet name
      */
-
+    public String getSheetName(int sheetIx)
-    public String getSheetName(int sheet)
     {
-        if (sheet > (sheets.size() - 1))
+        validateSheetIndex(sheetIx);
-        {
+        return workbook.getSheetName(sheetIx);
-            throw new RuntimeException("Sheet out of bounds");
-        }
-        return workbook.getSheetName(sheet);
     }
 
     /**
      * check whether a sheet is hidden
-     * @param sheet Number
+     * @param sheetIx Number
      * @return True if sheet is hidden
      */
-
+    public boolean isSheetHidden(int sheetIx) {
-    public boolean isSheetHidden(int sheet) {
+        validateSheetIndex(sheetIx);
-        if (sheet > (sheets.size() - 1))
+        return workbook.isSheetHidden(sheetIx);
-        {
-            throw new RuntimeException("Sheet out of bounds");
-        }
-        return workbook.isSheetHidden(sheet);
     }
 
     /**
      * Hide or unhide a sheet
      *
-     * @param sheetnum The sheet number
+     * @param sheetIx The sheet index
      * @param hidden True to mark the sheet as hidden, false otherwise
      */
-
+    public void setSheetHidden(int sheetIx, boolean hidden) {
-    public void setSheetHidden(int sheet, boolean hidden) {
+        validateSheetIndex(sheetIx);
-        if (sheet > (sheets.size() - 1))
+        workbook.setSheetHidden(sheetIx, hidden);
-        {
-            throw new RuntimeException("Sheet out of bounds");
-        }
-        workbook.setSheetHidden(sheet,hidden);
     }
 
     /*
@@ -602,12 +576,12 @@ public class HSSFWorkbook extends POIDocument
 
     /** Returns the index of the given sheet
      * @param sheet the sheet to look up
-     * @return index of the sheet (0 based)
+     * @return index of the sheet (0 based). <tt>-1</tt> if not found
      */
     public int getSheetIndex(HSSFSheet sheet)
     {
-        for(int i=0; i<sheets.size(); i++) {
+        for(int i=0; i<_sheets.size(); i++) {
-            if(sheets.get(i) == sheet) {
+            if(_sheets.get(i) == sheet) {
                 return i;
             }
         }
@@ -636,9 +610,9 @@ public class HSSFWorkbook extends POIDocument
     {
         HSSFSheet sheet = new HSSFSheet(this);
 
-        sheets.add(sheet);
+        _sheets.add(sheet);
-        workbook.setSheetName(sheets.size() - 1, "Sheet" + (sheets.size() - 1));
+        workbook.setSheetName(_sheets.size() - 1, "Sheet" + (_sheets.size() - 1));
-        boolean isOnlySheet = sheets.size() == 1;
+        boolean isOnlySheet = _sheets.size() == 1;
         sheet.setSelected(isOnlySheet);
         sheet.setActive(isOnlySheet);
         return sheet;
@@ -652,13 +626,13 @@ public class HSSFWorkbook extends POIDocument
 
     public HSSFSheet cloneSheet(int sheetNum) {
         validateSheetIndex(sheetNum);
-        HSSFSheet srcSheet = (HSSFSheet) sheets.get(sheetNum);
+        HSSFSheet srcSheet = (HSSFSheet) _sheets.get(sheetNum);
         String srcName = workbook.getSheetName(sheetNum);
         HSSFSheet clonedSheet = srcSheet.cloneSheet(this);
         clonedSheet.setSelected(false);
         clonedSheet.setActive(false);
 
-        sheets.add(clonedSheet);
+        _sheets.add(clonedSheet);
         int i = 1;
         while (true) {
             // Try and find the next sheet name that is unique
@@ -672,7 +646,7 @@ public class HSSFWorkbook extends POIDocument
 
             //If the sheet name is unique, then set it otherwise move on to the next number.
             if (workbook.getSheetIndex(name) == -1) {
-              workbook.setSheetName(sheets.size()-1, name);
+              workbook.setSheetName(_sheets.size()-1, name);
               break;
             }
         }
@@ -693,14 +667,14 @@ public class HSSFWorkbook extends POIDocument
 
     public HSSFSheet createSheet(String sheetname)
     {
-        if (workbook.doesContainsSheetName( sheetname, sheets.size() ))
+        if (workbook.doesContainsSheetName( sheetname, _sheets.size() ))
             throw new IllegalArgumentException( "The workbook already contains a sheet of this name" );
 
         HSSFSheet sheet = new HSSFSheet(this);
 
-        sheets.add(sheet);
+        _sheets.add(sheet);
-        workbook.setSheetName(sheets.size() - 1, sheetname);
+        workbook.setSheetName(_sheets.size() - 1, sheetname);
-        boolean isOnlySheet = sheets.size() == 1;
+        boolean isOnlySheet = _sheets.size() == 1;
         sheet.setSelected(isOnlySheet);
         sheet.setActive(isOnlySheet);
         return sheet;
@@ -713,9 +687,14 @@ public class HSSFWorkbook extends POIDocument
 
     public int getNumberOfSheets()
     {
-        return sheets.size();
+        return _sheets.size();
     }
 
+    private HSSFSheet[] getSheets() {
+        HSSFSheet[] result = new HSSFSheet[_sheets.size()];
+        _sheets.toArray(result);
+        return result;
+    }
     /**
      * Get the HSSFSheet object at the given index.
      * @param index of the sheet number (0-based physical & logical)
@@ -724,7 +703,7 @@ public class HSSFWorkbook extends POIDocument
 
     public HSSFSheet getSheetAt(int index)
     {
-        return (HSSFSheet) sheets.get(index);
+        return (HSSFSheet) _sheets.get(index);
     }
 
     /**
@@ -737,13 +716,13 @@ public class HSSFWorkbook extends POIDocument
     {
         HSSFSheet retval = null;
 
-        for (int k = 0; k < sheets.size(); k++)
+        for (int k = 0; k < _sheets.size(); k++)
         {
             String sheetname = workbook.getSheetName(k);
 
             if (sheetname.equalsIgnoreCase(name))
             {
-                retval = (HSSFSheet) sheets.get(k);
+                retval = (HSSFSheet) _sheets.get(k);
             }
         }
         return retval;
@@ -772,11 +751,11 @@ public class HSSFWorkbook extends POIDocument
         boolean wasActive = getSheetAt(index).isActive();
         boolean wasSelected = getSheetAt(index).isSelected();
 
-        sheets.remove(index);
+        _sheets.remove(index);
         workbook.removeSheet(index);
 
         // set the remaining active/selected sheet
-        int nSheets = sheets.size();
+        int nSheets = _sheets.size();
         if (nSheets < 1) {
             // nothing more to do if there are no sheets left
             return;
@@ -1152,48 +1131,47 @@ public class HSSFWorkbook extends POIDocument
 
     public byte[] getBytes()
     {
-        if (log.check( POILogger.DEBUG ))
+        if (log.check( POILogger.DEBUG )) {
             log.log(DEBUG, "HSSFWorkbook.getBytes()");
+        }
+        
+        HSSFSheet[] sheets = getSheets();
+        int nSheets = sheets.length;
 
         // before getting the workbook size we must tell the sheets that
         // serialization is about to occur.
-        for (int k = 0; k < sheets.size(); k++)
+        for (int i = 0; i < nSheets; i++) {
-            ((HSSFSheet) sheets.get(k)).getSheet().preSerialize();
+            sheets[i].getSheet().preSerialize();
+        }
 
-        int wbsize = workbook.getSize();
+        int totalsize = workbook.getSize();
 
-        // log.debug("REMOVEME: old sizing method "+workbook.serialize().length);
+        // pre-calculate all the sheet sizes and set BOF indexes
-        // ArrayList sheetbytes = new ArrayList(sheets.size());
+        int[] estimatedSheetSizes = new int[nSheets];
-        int totalsize = wbsize;
+        for (int k = 0; k < nSheets; k++) {
-
-        for (int k = 0; k < sheets.size(); k++)
-        {
             workbook.setSheetBof(k, totalsize);
-            totalsize += ((HSSFSheet) sheets.get(k)).getSheet().getSize();
+            int sheetSize = sheets[k].getSheet().getSize();
+            estimatedSheetSizes[k] = sheetSize;
+            totalsize += sheetSize;
         }
 
 
-/*        if (totalsize < 4096)
-        {
-            totalsize = 4096;
-        }*/
         byte[] retval = new byte[totalsize];
         int pos = workbook.serialize(0, retval);
 
-        // System.arraycopy(wb, 0, retval, 0, wb.length);
+        for (int k = 0; k < nSheets; k++) {
-        for (int k = 0; k < sheets.size(); k++)
+            int serializedSize = sheets[k].getSheet().serialize(pos, retval);
-        {
+            if (serializedSize != estimatedSheetSizes[k]) {
-
+                // Wrong offset values have been passed in the call to setSheetBof() above.
-            // byte[] sb = (byte[])sheetbytes.get(k);
+                // For books with more than one sheet, this discrepancy would cause excel 
-            // System.arraycopy(sb, 0, retval, pos, sb.length);
+                // to report errors and loose data while reading the workbook
-            int len = ((HSSFSheet) sheets.get(k)).getSheet().serialize(pos,
+                throw new IllegalStateException("Actual serialized sheet size (" + serializedSize 
-                                retval);
+                        + ") differs from pre-calculated size (" + estimatedSheetSizes[k] 
-            pos += len;   // sb.length;
+                        + ") for sheet (" + k + ")");
+                // TODO - add similar sanity check to ensure that Sheet.serializeIndexRecord() does not write mis-aligned offsets either
+            }
+            pos += serializedSize;
         }
-/*        for (int k = pos; k < totalsize; k++)
-        {
-            retval[k] = 0;
-        }*/
         return retval;
     }
 

src/testcases/org/apache/poi/hssf/model/TestSheet.java

@@ -17,6 +17,7 @@
 
 package org.apache.poi.hssf.model;
 
+import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 import org.apache.poi.hssf.record.*;
 import org.apache.poi.hssf.record.aggregates.ColumnInfoRecordsAggregate;
@@ -351,5 +352,25 @@ public final class TestSheet extends TestCase {
         xfindex = sheet.getXFIndexForColAt((short) 10);
         assertEquals(DEFAULT_IDX, xfindex);
     }
+
+    /**
+     * Prior to bug 45066, POI would get the estimated sheet size wrong 
+     * when an <tt>UncalcedRecord</tt> was present.<p/>
+     */
+    public void testUncalcSize_bug45066() {
+
+        List records = new ArrayList();
+        records.add(new BOFRecord());
+        records.add(new UncalcedRecord());
+        records.add(new EOFRecord());
+        Sheet sheet = Sheet.createSheet( records, 0, 0 );
+
+        int estimatedSize = sheet.getSize();
+        int serializedSize = sheet.serialize(0, new byte[estimatedSize]);
+        if (serializedSize != estimatedSize) {
+            throw new AssertionFailedError("Identified bug 45066 b");
+        }
+        assertEquals(50, serializedSize);
+    }
 }
 

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java

@@ -20,12 +20,16 @@ package org.apache.poi.hssf.usermodel;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.util.List;
 
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.hssf.model.Sheet;
 import org.apache.poi.hssf.record.NameRecord;
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.RecordInputStream;
 import org.apache.poi.util.TempFile;
 /**
  *
@@ -376,4 +380,49 @@ public final class TestHSSFWorkbook extends TestCase {
         assertEquals("active", expectedActive, sheet.isActive());
         assertEquals("selected", expectedSelected, sheet.isSelected());
     }
-}
+    
+    /**
+     * If Sheet.getSize() returns a different result to Sheet.serialize(), this will cause the BOF
+     * records to be written with invalid offset indexes.  Excel does not like this, and such 
+     * errors are particularly hard to track down.  This test ensures that HSSFWorkbook throws
+     * a specific exception as soon as the situation is detected. See bugzilla 45066
+     */
+    public void testSheetSerializeSizeMismatch_bug45066() {
+        HSSFWorkbook wb = new HSSFWorkbook();
+        Sheet sheet = wb.createSheet("Sheet1").getSheet();
+        List sheetRecords = sheet.getRecords();
+        // one way (of many) to cause the discrepancy is with a badly behaved record:
+        sheetRecords.add(new BadlyBehavedRecord());
+        // There is also much logic inside Sheet that (if buggy) might also cause the discrepancy
+        try {
+            wb.getBytes();
+            throw new AssertionFailedError("Identified bug 45066 a");
+        } catch (IllegalStateException e) {
+            // Expected badly behaved sheet record to cause exception
+            assertTrue(e.getMessage().startsWith("Actual serialized sheet size"));
+        }
+    }
+    /**
+     * result returned by getRecordSize() differs from result returned by serialize()
+     */
+    private static final class BadlyBehavedRecord extends Record {
+        public BadlyBehavedRecord() {
+            // 
+        }
+        protected void fillFields(RecordInputStream in) {
+            throw new RuntimeException("Should not be called");
+        }
+        public short getSid() {
+            return 0x777;
+        }
+        public int serialize(int offset, byte[] data) {
+            return 4;
+        }
+        protected void validateSid(short id) {
+            throw new RuntimeException("Should not be called");
+        }
+        public int getRecordSize() {
+            return 8;
+        }
+    }
+ }

src/testcases/org/apache/poi/hssf/usermodel/TestSheetHiding.java

@@ -45,8 +45,8 @@ public final class TestSheetHiding extends TestCase {
 	 */
 	public void testTextSheets() throws Exception {
 		// Both should have two sheets
-		assertEquals(2, wbH.sheets.size());
+		assertEquals(2, wbH.getNumberOfSheets());
-		assertEquals(2, wbU.sheets.size());
+		assertEquals(2, wbU.getNumberOfSheets());
 
 		// All sheets should have one row
 		assertEquals(0, wbH.getSheetAt(0).getLastRowNum());