Merged revisions 638786-638802,638805-638811,638813-638814,638816-639230,639233-639241,639243-639253,639255-639486,639488-639601,639603-639835,639837-639917,639919-640056,640058-640710,640712-641156,641158-641184,641186-641795,641797-641798,641800-641933,641935-641963,641965-641966,641968-641995,641997-642230,642232-642562,642564-642565,642568-642570,642572-642573,642576-642736,642739-642877,642879,642881-642890,642892-642903,642905-642945,642947-643624,643626-643653,643655-643669,643671,643673-643830,643832-643833,643835-644342,644344-644472,644474-644508,644510-645347,645349-645351,645353-645559,645561-645565,645568-645951,645953-646193,646195-646311,646313-646404,646406-646665,646667-646853,646855-646869,646871-647151,647153-647185,647187-647277,647279-647566,647568-647573,647575,647578-647711,647714-647737,647739-647823,647825-648155,648157-648202,648204-648273,648275,648277-648302,648304-648333,648335-648588,648590-648622,648625-648673,648675-649141,649144,649146-649556,649558-649795,649799,649801-649910,649912-649913,649915-650128,650131-650132,650134-650137,650140-650914,650916-651991,651993-652284,652286-652287,652289,652291,652293-652297,652299-652328,652330-652425,652427-652445,652447-652560,652562-652933,652935,652937-652993,652995-653116,653118-653124,653126-653483,653487-653519,653522-653550,653552-653607,653609-653667,653669-653674,653676-653814,653817-653830,653832-653891,653893-653944,653946-654055,654057-654355,654357-654365,654367-654648,654651-655215,655217-655277,655279-655281,655283-655911,655913-656212,656214,656216-656251,656253-656698,656700-656756,656758-656892,656894-657135,657137-657165,657168-657179,657181-657354,657356-657357,657359-657701,657703-657874,657876-658032,658034-658284,658286,658288-658301,658303-658307,658309-658321,658323-658335,658337-658348,658351,658353-658832,658834-658983,658985,658987-659066,659068-659402,659404-659428,659430-659451,659453-659454,659456-659461,659463-659477,659479-659524,659526-659571,659574-663870 via svnmerge from

https://svn.apache.org:443/repos/asf/poi/trunk

........
  r659575 | nick | 2008-05-23 16:55:08 +0100 (Fri, 23 May 2008) | 1 line
  
  Help for bug #44840 - Improved handling of HSSFObjectData, especially for entries with data held not in POIFS
........
  r660256 | josh | 2008-05-26 19:02:23 +0100 (Mon, 26 May 2008) | 1 line
  
  Follow-on fix for bug 42564 (r653668). Array elements are stored internally column by column.
........
  r660263 | josh | 2008-05-26 19:25:02 +0100 (Mon, 26 May 2008) | 1 line
  
  Small fix for FormulaParser. Need case-insentive match for IF function name
........
  r660280 | josh | 2008-05-26 20:36:56 +0100 (Mon, 26 May 2008) | 1 line
  
  Added test cases for parsing IF expressions.  Segregated IF test cases into a new class
........
  r660344 | josh | 2008-05-27 01:57:23 +0100 (Tue, 27 May 2008) | 1 line
  
  Changed class hierarchy of Ptg to improve 'operand class' transformation.
........
  r660474 | nick | 2008-05-27 12:44:49 +0100 (Tue, 27 May 2008) | 1 line
  
  X, Y, Width and Height getters/setters on HSSFChart
........
  r660828 | josh | 2008-05-28 07:19:31 +0100 (Wed, 28 May 2008) | 1 line
  
  Fix for 45060 (and 45041) - Improved token class transformation during formula parsing
........
  r660834 | yegor | 2008-05-28 07:50:35 +0100 (Wed, 28 May 2008) | 1 line
  
  bump 3.1-beta2 announcement
........
  r660889 | nick | 2008-05-28 11:03:00 +0100 (Wed, 28 May 2008) | 1 line
  
  Fix bug #45087 - Correctly detect date formats like [Black]YYYY as being date based
........
  r663322 | josh | 2008-06-04 18:37:18 +0100 (Wed, 04 Jun 2008) | 1 line
  
  Test code clean-up (prior to bug 45126)
........
  r663436 | josh | 2008-06-05 04:12:35 +0100 (Thu, 05 Jun 2008) | 1 line
  
  Fix for bug 45123 - SharedFormulaRecord.convertSharedFormulas was ignoring token operand classes
........
  r663765 | josh | 2008-06-05 23:24:05 +0100 (Thu, 05 Jun 2008) | 1 line
  
  Fix for bug 45145 - made sure RowRecordsAggregate comes before ValueRecordsAggregate.  Also fixed BiffViewer to show correct record offsets
........
  r663855 | josh | 2008-06-06 09:32:54 +0100 (Fri, 06 Jun 2008) | 1 line
  
  Fix for 45133 - OBJ Record (5Dh) needs to pad the sub-record data to a 4-byte boundary
........


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

src/documentation/content/xdocs/changes.xml

@@ -46,6 +46,9 @@
            <action dev="POI-DEVELOPERS" type="add">Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx</action>
         </release>
         <release version="3.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="fix">45133 - Fixed OBJ Record (5Dh) to pad the sub-record data to a 4-byte boundary</action>
+           <action dev="POI-DEVELOPERS" type="fix">45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate</action>
+           <action dev="POI-DEVELOPERS" type="fix">45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes</action>
            <action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats like [Black]YYYY as being date based</action>
            <action dev="POI-DEVELOPERS" type="add">45060 - Improved token class transformation during formula parsing</action>
            <action dev="POI-DEVELOPERS" type="add">44840 - Improved handling of HSSFObjectData, especially for entries with data held not in POIFS</action>

src/documentation/content/xdocs/status.xml

@@ -43,6 +43,9 @@
            <action dev="POI-DEVELOPERS" type="add">Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx</action>
         </release>
         <release version="3.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="fix">45133 - Fixed OBJ Record (5Dh) to pad the sub-record data to a 4-byte boundary</action>
+           <action dev="POI-DEVELOPERS" type="fix">45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate</action>
+           <action dev="POI-DEVELOPERS" type="fix">45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes</action>
            <action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats like [Black]YYYY as being date based</action>
            <action dev="POI-DEVELOPERS" type="add">45060 - Improved token class transformation during formula parsing</action>
            <action dev="POI-DEVELOPERS" type="add">44840 - Improved handling of HSSFObjectData, especially for entries with data held not in POIFS</action>

src/java/org/apache/poi/hssf/dev/BiffViewer.java

@@ -87,7 +87,8 @@ public final class BiffViewer {
                         records.add(record);
                         if (activeRecord != null)
                             activeRecord.dump(ps);
-                        activeRecord = new RecordDetails(recStream.getSid(), recStream.getLength(), (int)recStream.getPos(), record);
+                        int startPos = (int)(recStream.getPos()-recStream.getLength() - 4);
+                        activeRecord = new RecordDetails(recStream.getSid(), recStream.getLength(), startPos, record);
                     }
                     if (dump) {
                         recStream.dumpBytes(ps);

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

@@ -66,7 +66,7 @@ public final class Sheet implements Model {
     protected ArrayList                  records           =     null;
               int                        preoffset         =     0;            // offset of the sheet in a new file
               int                        loc               =     0;
-    protected int                        dimsloc           =     0;
+    protected int                        dimsloc           =     -1;  // TODO - is it legal for dims record to be missing?
     protected DimensionsRecord           dims;
     protected DefaultColWidthRecord      defaultcolwidth   =     null;
     protected DefaultRowHeightRecord     defaultrowheight  =     null;
@@ -295,6 +295,8 @@ public final class Sheet implements Model {
             }
             else if ( rec.getSid() == IndexRecord.sid )
             {
+                // ignore INDEX record because it is only needed by Excel, 
+                // and POI always re-calculates its contents 
                 rec = null;
             }
 
@@ -329,8 +331,8 @@ public final class Sheet implements Model {
             }
         }
         retval.records = records;
-        retval.checkCells();
         retval.checkRows();
+        retval.checkCells();
         if (log.check( POILogger.DEBUG ))
             log.log(POILogger.DEBUG, "sheet createSheet (existing file) exited");
         return retval;
@@ -486,7 +488,15 @@ public final class Sheet implements Model {
         if (cells == null)
         {
             cells = new ValueRecordsAggregate();
-            records.add(getDimsLoc() + 1, cells);
+            // In the worksheet stream, the row records always occur before the cell (value) 
+            // records. Therefore POI's aggregates (RowRecordsAggregate, ValueRecordsAggregate) 
+            // should follow suit. Some methods in this class tolerate either order, while 
+            // others have been found to fail (see bug 45145).
+            int rraIndex = getDimsLoc() + 1;
+            if (records.get(rraIndex).getClass() != RowRecordsAggregate.class) {
+                throw new IllegalStateException("Cannot create value records before row records exist");
+            }
+            records.add(rraIndex+1, cells);
         }
     }
 
@@ -836,46 +846,61 @@ public final class Sheet implements Model {
         return pos-offset;
     }
 
-    private int serializeIndexRecord(final int BOFRecordIndex, final int offset, byte[] data) {
+    /**
-      IndexRecord index = new IndexRecord();
+     * @param indexRecordOffset also happens to be the end of the BOF record
-      index.setFirstRow(rows.getFirstRowNum());
+     * @return the size of the serialized INDEX record
-      index.setLastRowAdd1(rows.getLastRowNum()+1);
+     */
-      //Calculate the size of the records from the end of the BOF
+    private int serializeIndexRecord(final int bofRecordIndex, final int indexRecordOffset,
-      //and up to the RowRecordsAggregate...
+            byte[] data) {
-      int sheetRecSize = 0;
+        IndexRecord index = new IndexRecord();
-      for (int j = BOFRecordIndex+1; j < records.size(); j++)
+        index.setFirstRow(rows.getFirstRowNum());
-      {
+        index.setLastRowAdd1(rows.getLastRowNum() + 1);
-        Record tmpRec = (( Record ) records.get(j));
+        // Calculate the size of the records from the end of the BOF
-        if (tmpRec instanceof UncalcedRecord) {
+        // and up to the RowRecordsAggregate...
-            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
-      int indexRecSize = IndexRecord.getRecordSizeForBlockCount(blockCount);
 
-      int rowBlockOffset = 0;
+        // 'initial sheet records' are between INDEX and first ROW record.
-      int cellBlockOffset = 0;
+        int sizeOfInitialSheetRecords = 0;
-      int dbCellOffset = 0;
+        // start just after BOF record (INDEX is not present in this list)
-      for (int block=0;block<blockCount;block++) {
+        for (int j = bofRecordIndex + 1; j < records.size(); j++) {
-        rowBlockOffset += rows.getRowBlockSize(block);
+            Record tmpRec = ((Record) records.get(j));
-        cellBlockOffset += null == cells ? 0 : cells.getRowCellBlockSize(rows.getStartRowNumberForBlock(block),
+            if (tmpRec instanceof UncalcedRecord) {
-                                                     rows.getEndRowNumberForBlock(block));
+                continue;
-        //Note: The offsets are relative to the Workbook BOF. Assume that this is
+            }
-        //0 for now.....
+            if (tmpRec instanceof RowRecordsAggregate) {
-        index.addDbcell(offset + indexRecSize + sheetRecSize + dbCellOffset + rowBlockOffset + cellBlockOffset);
+                break;
-        //Add space required to write the dbcell record(s) (whose references were just added).
+            }
-        dbCellOffset += (8 + (rows.getRowCountForBlock(block) * 2));
+            sizeOfInitialSheetRecords += tmpRec.getRecordSize();
-      }
+        }
-      return index.serialize(offset, data);
+        if (_isUncalced) {
+            sizeOfInitialSheetRecords += UncalcedRecord.getStaticRecordSize();
+        }
+
+        // Add the references to the DBCells in the IndexRecord (one for each block)
+        // Note: The offsets are relative to the Workbook BOF. Assume that this is
+        // 0 for now.....
+
+        int blockCount = rows.getRowBlockCount();
+        // Calculate the size of this IndexRecord
+        int indexRecSize = IndexRecord.getRecordSizeForBlockCount(blockCount);
+
+        int currentOffset = indexRecordOffset + indexRecSize + sizeOfInitialSheetRecords;
+
+        for (int block = 0; block < blockCount; block++) {
+            // each row-block has a DBCELL record.
+            // The offset of each DBCELL record needs to be updated in the INDEX record
+
+            // account for row records in this row-block
+            currentOffset += rows.getRowBlockSize(block);
+            // account for cell value records after those
+            currentOffset += null == cells ? 0 : cells.getRowCellBlockSize(rows
+                    .getStartRowNumberForBlock(block), rows.getEndRowNumberForBlock(block));
+
+            // currentOffset is now the location of the DBCELL record for this row-block
+            index.addDbcell(currentOffset);
+            // Add space required to write the DBCELL record (whose reference was just added).
+            currentOffset += (8 + (rows.getRowCountForBlock(block) * 2));
+        }
+        return index.serialize(indexRecordOffset, data);
     }
 
 

src/java/org/apache/poi/hssf/record/ObjRecord.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,27 +15,21 @@
    limitations under the License.
 ==================================================================== */
         
-
-
 package org.apache.poi.hssf.record;
 
-
-
-import org.apache.poi.util.*;
-
 import java.io.ByteArrayInputStream;
-import java.util.List;
-import java.util.Iterator;
 import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.poi.util.LittleEndian;
 
 /**
  * The obj record is used to hold various graphic objects and controls.
  *
  * @author Glen Stampoultzis (glens at apache.org)
  */
-public class ObjRecord
+public final class ObjRecord extends Record {
-    extends Record
-{
     public final static short      sid                             = 0x5D;
     private List subrecords;
 
@@ -47,6 +40,7 @@ public class ObjRecord
     public ObjRecord()
     {
         subrecords = new ArrayList(2);
+        // TODO - ensure 2 sub-records (ftCmo  15h, and ftEnd  00h) are always created
     }
 
     /**
@@ -80,6 +74,7 @@ public class ObjRecord
         //following wont work properly
         int subSize = 0;
         byte[] subRecordData = in.readRemainder();
+
         RecordInputStream subRecStream = new RecordInputStream(new ByteArrayInputStream(subRecordData));
         while(subRecStream.hasNextRecord()) {
           subRecStream.nextRecord();
@@ -89,28 +84,19 @@ public class ObjRecord
         }
 
         /**
-         * Check if the RecordInputStream skipped EndSubRecord,
+         * Add the EndSubRecord explicitly.
-         * if it did then append it explicitly.
+         * 
-         * See Bug 41242 for details.
+         * TODO - the reason the EndSubRecord is always skipped is because its 'sid' is zero and
+         * that causes subRecStream.hasNextRecord() to return false.
+         * There may be more than the size of EndSubRecord left-over, if there is any padding 
+         * after that record.  The content of the EndSubRecord and the padding is all zeros.
+         * So there's not much to look at past the last substantial record.
+         * 
+         * See Bugs 41242/45133 for details.
          */
-        if (subRecordData.length - subSize == 4){
+        if (subRecordData.length - subSize >= 4) {
             subrecords.add(new EndSubRecord());
         }
-
-        /* JMH the size present/not present in the code below
-           needs to be considered in the RecordInputStream??
-        int pos = offset;
-        while (pos - offset <= size-2) // atleast one "short" must be present
-        {
-            short subRecordSid = LittleEndian.getShort(data, pos);
-            short subRecordSize = -1; // set default to "< 0"
-            if (pos-offset <= size-4) { // see if size info is present, else default to -1
-                subRecordSize = LittleEndian.getShort(data, pos + 2);
-            }
-            Record subRecord = SubRecord.createSubRecord(subRecordSid, subRecordSize, data, pos + 4);
-            subrecords.add(subRecord);
-            pos += subRecord.getRecordSize();
-        }*/
     }
 
     public String toString()
@@ -140,6 +126,8 @@ public class ObjRecord
             Record record = (Record) iterator.next();
             pos += record.serialize(pos, data);
         }
+        // assume padding (if present) does not need to be written.
+        // it is probably zero already, and it probably doesn't matter anyway
 
         return getRecordSize();
     }
@@ -155,7 +143,9 @@ public class ObjRecord
             Record record = (Record) iterator.next();
             size += record.getRecordSize();
         }
-        return 4  + size;
+        int oddBytes = size & 0x03;
+        int padding = oddBytes == 0 ? 0 : 4 - oddBytes;
+        return 4  + size + padding;
     }
 
     public short getSid()
@@ -192,9 +182,4 @@ public class ObjRecord
 
         return rec;
     }
-
+}
-}  // END OF CLASS
-
-
-
-

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

@@ -201,6 +201,10 @@ public final class SharedFormulaRecord extends Record {
         if (ptgs != null)
           for (int k = 0; k < ptgs.size(); k++) {
             Ptg ptg = (Ptg) ptgs.get(k);
+            byte originalOperandClass = -1;
+            if (!ptg.isBaseToken()) {
+                originalOperandClass = ptg.getPtgClass();
+            }
             if (ptg instanceof RefNPtg) {
               RefNPtg refNPtg = (RefNPtg)ptg;
               ptg = new ReferencePtg(fixupRelativeRow(formulaRow,refNPtg.getRow(),refNPtg.isRowRelative()),
@@ -249,7 +253,11 @@ public final class SharedFormulaRecord extends Record {
                                 areaNAPtg.isLastRowRelative(),
                                 areaNAPtg.isFirstColRelative(),
                                 areaNAPtg.isLastColRelative());
-            } 
+            }
+            if (!ptg.isBaseToken()) {
+                ptg.setClass(originalOperandClass);
+            }
+            
             newPtgStack.add(ptg);
         }
         return newPtgStack;

src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java

@@ -37,7 +37,7 @@ import java.util.List;
 public final class ValueRecordsAggregate
     extends Record
 {
-    public final static short sid       = -1000;
+    public final static short sid       = -1001; // 1000 clashes with RowRecordsAggregate
     int                       firstcell = -1;
     int                       lastcell  = -1;
   CellValueRecordInterface[][] records;

src/java/org/apache/poi/ss/usermodel/FormulaEvaluator.java

@@ -337,10 +337,17 @@ public class FormulaEvaluator {
 
             // since we don't know how to handle these yet :(
             Ptg ptg = ptgs[i];
+<<<<<<< .mine
+            if (ptg instanceof ControlPtg) {
+               // skip Parentheses, Attr, etc
+               continue;
+			}
+=======
             if (ptg instanceof ControlPtg) {
                 // skip Parentheses, Attr, etc
                 continue;
             }
+>>>>>>> .r663896
             if (ptg instanceof MemErrPtg) { continue; }
             if (ptg instanceof MissingArgPtg) { continue; }
             if (ptg instanceof NamePtg) { 

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

@@ -19,11 +19,15 @@ package org.apache.poi.hssf.model;
 
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
+
+import org.apache.poi.hssf.eventmodel.ERFListener;
+import org.apache.poi.hssf.eventmodel.EventRecordFactory;
 import org.apache.poi.hssf.record.*;
 import org.apache.poi.hssf.record.aggregates.ColumnInfoRecordsAggregate;
 import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate;
 import org.apache.poi.hssf.record.aggregates.ValueRecordsAggregate;
 
+import java.io.ByteArrayInputStream;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
@@ -34,8 +38,7 @@ import java.util.List;
  * @author Glen Stampoultzis (glens at apache.org)
  */
 public final class TestSheet extends TestCase {
-    public void testCreateSheet() throws Exception
+    public void testCreateSheet() {
-    {
         // Check we're adding row and cell aggregates
         List records = new ArrayList();
         records.add( new BOFRecord() );
@@ -52,8 +55,7 @@ public final class TestSheet extends TestCase {
         assertTrue( sheet.records.get(pos++) instanceof EOFRecord );
     }
 
-    public void testAddMergedRegion()
+    public void testAddMergedRegion() {
-    {
         Sheet sheet = Sheet.createSheet();
         int regionsToAdd = 4096;
         int startRecords = sheet.getRecords().size();
@@ -91,8 +93,7 @@ public final class TestSheet extends TestCase {
         }
     }
 
-    public void testRemoveMergedRegion()
+    public void testRemoveMergedRegion() {
-    {
         Sheet sheet = Sheet.createSheet();
         int regionsToAdd = 4096;
 
@@ -139,13 +140,11 @@ public final class TestSheet extends TestCase {
         assertEquals("Should be no more merged regions", 0, sheet.getNumMergedRegions());
     }
 
-    public void testGetMergedRegionAt()
+    public void testGetMergedRegionAt() {
-    {
         //TODO
     }
 
-    public void testGetNumMergedRegions()
+    public void testGetNumMergedRegions() {
-    {
         //TODO
     }
 
@@ -163,14 +162,13 @@ public final class TestSheet extends TestCase {
 
         Sheet sheet = Sheet.createSheet(records, 0);
         assertNotNull("Row [2] was skipped", sheet.getRow(2));
-
     }
 
     /**
      * Make sure page break functionality works (in memory)
      *
      */
-    public void testRowPageBreaks(){
+    public void testRowPageBreaks() {
         short colFrom = 0;
         short colTo = 255;
 
@@ -226,7 +224,7 @@ public final class TestSheet extends TestCase {
      * Make sure column pag breaks works properly (in-memory)
      *
      */
-    public void testColPageBreaks(){
+    public void testColPageBreaks() {
         short rowFrom = 0;
         short rowTo = (short)65535;
 
@@ -292,20 +290,20 @@ public final class TestSheet extends TestCase {
         final short DEFAULT_IDX = 0xF; // 15
         short xfindex = Short.MIN_VALUE;
         Sheet sheet = Sheet.createSheet();
-        
+
         // without ColumnInfoRecord
         xfindex = sheet.getXFIndexForColAt((short) 0);
         assertEquals(DEFAULT_IDX, xfindex);
         xfindex = sheet.getXFIndexForColAt((short) 1);
         assertEquals(DEFAULT_IDX, xfindex);
-        
+
         ColumnInfoRecord nci = ( ColumnInfoRecord ) sheet.createColInfo();
         sheet.columns.insertColumn(nci);
-        
+
         // single column ColumnInfoRecord
         nci.setFirstColumn((short) 2);
         nci.setLastColumn((short) 2);
-        nci.setXFIndex(TEST_IDX);            
+        nci.setXFIndex(TEST_IDX);
         xfindex = sheet.getXFIndexForColAt((short) 0);
         assertEquals(DEFAULT_IDX, xfindex);
         xfindex = sheet.getXFIndexForColAt((short) 1);
@@ -318,7 +316,7 @@ public final class TestSheet extends TestCase {
         // ten column ColumnInfoRecord
         nci.setFirstColumn((short) 2);
         nci.setLastColumn((short) 11);
-        nci.setXFIndex(TEST_IDX);            
+        nci.setXFIndex(TEST_IDX);
         xfindex = sheet.getXFIndexForColAt((short) 1);
         assertEquals(DEFAULT_IDX, xfindex);
         xfindex = sheet.getXFIndexForColAt((short) 2);
@@ -333,7 +331,7 @@ public final class TestSheet extends TestCase {
         // single column ColumnInfoRecord starting at index 0
         nci.setFirstColumn((short) 0);
         nci.setLastColumn((short) 0);
-        nci.setXFIndex(TEST_IDX);            
+        nci.setXFIndex(TEST_IDX);
         xfindex = sheet.getXFIndexForColAt((short) 0);
         assertEquals(TEST_IDX, xfindex);
         xfindex = sheet.getXFIndexForColAt((short) 1);
@@ -342,7 +340,7 @@ public final class TestSheet extends TestCase {
         // ten column ColumnInfoRecord starting at index 0
         nci.setFirstColumn((short) 0);
         nci.setLastColumn((short) 9);
-        nci.setXFIndex(TEST_IDX);            
+        nci.setXFIndex(TEST_IDX);
         xfindex = sheet.getXFIndexForColAt((short) 0);
         assertEquals(TEST_IDX, xfindex);
         xfindex = sheet.getXFIndexForColAt((short) 7);
@@ -354,7 +352,7 @@ public final class TestSheet extends TestCase {
     }
 
     /**
-     * Prior to bug 45066, POI would get the estimated sheet size wrong 
+     * Prior to bug 45066, POI would get the estimated sheet size wrong
      * when an <tt>UncalcedRecord</tt> was present.<p/>
      */
     public void testUncalcSize_bug45066() {
@@ -363,7 +361,7 @@ public final class TestSheet extends TestCase {
         records.add(new BOFRecord());
         records.add(new UncalcedRecord());
         records.add(new EOFRecord());
-        Sheet sheet = Sheet.createSheet( records, 0, 0 );
+        Sheet sheet = Sheet.createSheet(records, 0, 0);
 
         int estimatedSize = sheet.getSize();
         int serializedSize = sheet.serialize(0, new byte[estimatedSize]);
@@ -372,5 +370,73 @@ public final class TestSheet extends TestCase {
         }
         assertEquals(50, serializedSize);
     }
+
+    /**
+     * Prior to bug 45145 <tt>RowRecordsAggregate</tt> and <tt>ValueRecordsAggregate</tt> could
+     * sometimes occur in reverse order.  This test reproduces one of those situations and makes
+     * sure that RRA comes before VRA.<br/>
+     *
+     * The code here represents a normal POI use case where a spreadsheet is created from scratch.
+     */
+    public void testRowValueAggregatesOrder_bug45145() {
+
+        Sheet sheet = Sheet.createSheet();
+
+        RowRecord rr = new RowRecord(5);
+        sheet.addRow(rr);
+
+        CellValueRecordInterface cvr = new BlankRecord();
+        cvr.setColumn((short)0);
+        cvr.setRow(5);
+        sheet.addValueRecord(5, cvr);
+
+
+        int dbCellRecordPos = getDbCellRecordPos(sheet);
+        if (dbCellRecordPos == 264) {
+            // The overt symptom of the bug
+            // DBCELL record pos is calculated wrong if VRA comes before RRA
+            throw new AssertionFailedError("Identified  bug 45145");
+        }
+
+        // make sure that RRA and VRA are in the right place
+        int rraIx = sheet.getDimsLoc()+1;
+        List recs = sheet.getRecords();
+        assertEquals(RowRecordsAggregate.class, recs.get(rraIx).getClass());
+        assertEquals(ValueRecordsAggregate.class, recs.get(rraIx+1).getClass());
+
+        assertEquals(254, dbCellRecordPos);
+    }
+
+    /**
+     * @return the value calculated for the position of the first DBCELL record for this sheet.
+     * That value is found on the IndexRecord.
+     */
+    private static int getDbCellRecordPos(Sheet sheet) {
+        int size = sheet.getSize();
+        byte[] data = new byte[size];
+        sheet.serialize(0, data);
+        EventRecordFactory erf = new EventRecordFactory();
+        MyIndexRecordListener myIndexListener = new MyIndexRecordListener();
+        erf.registerListener(myIndexListener, new short[] { IndexRecord.sid, });
+        erf.processRecords(new ByteArrayInputStream(data));
+        IndexRecord indexRecord = myIndexListener.getIndexRecord();
+        int dbCellRecordPos = indexRecord.getDbcellAt(0);
+        return dbCellRecordPos;
+    }
+
+    private static final class MyIndexRecordListener implements ERFListener {
+
+        private IndexRecord _indexRecord;
+        public MyIndexRecordListener() {
+            // no-arg constructor
+        }
+        public boolean processRecord(Record rec) {
+            _indexRecord = (IndexRecord)rec;
+            return true;
+        }
+        public IndexRecord getIndexRecord() {
+            return _indexRecord;
+        }
+    }
 }
 

src/testcases/org/apache/poi/hssf/record/AllRecordTests.java

@@ -95,6 +95,7 @@ public final class AllRecordTests {
 		result.addTestSuite(TestSeriesTextRecord.class);
 		result.addTestSuite(TestSeriesToChartGroupRecord.class);
 		result.addTestSuite(TestSheetPropertiesRecord.class);
+		result.addTestSuite(TestSharedFormulaRecord.class);
 		result.addTestSuite(TestStringRecord.class);
 		result.addTestSuite(TestSubRecord.class);
 		result.addTestSuite(TestSupBookRecord.class);

src/testcases/org/apache/poi/hssf/record/TestObjRecord.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
@@ -18,18 +17,19 @@
 
 package org.apache.poi.hssf.record;
 
-import junit.framework.*;
-
 import java.util.Arrays;
 import java.util.List;
 
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
 /**
  * Tests the serialization and deserialization of the ObjRecord class works correctly.
  * Test data taken directly from a real Excel file.
  *
  * @author Yegor Kozlov
  */
-public class TestObjRecord extends TestCase {
+public final class TestObjRecord extends TestCase {
     /**
      * OBJ record data containing two sub-records.
      * The data taken directly from a real Excel file.
@@ -38,22 +38,27 @@ public class TestObjRecord extends TestCase {
      *     [ftCmo]
      *     [ftEnd]
      */
-    public static byte[] recdata = {
+    private static final byte[] recdata = {
         0x15, 0x00, 0x12, 0x00, 0x06, 0x00, 0x01, 0x00, 0x11, 0x60,
         (byte)0xF4, 0x02, 0x41, 0x01, 0x14, 0x10, 0x1F, 0x02, 0x00, 0x00,
         0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+        // TODO - this data seems to require two extra bytes padding. not sure where original file is.
+        // it's not bug 38607 attachment 17639
     };
 
+    private static final byte[] recdataNeedingPadding = {
+    	21, 0, 18, 0, 0, 0, 1, 0, 17, 96, 0, 0, 0, 0, 56, 111, -52, 3, 0, 0, 0, 0, 6, 0, 2, 0, 0, 0, 0, 0, 0, 0
+    };
 
-    public void testLoad() throws Exception {
+    public void testLoad() {
         ObjRecord record = new ObjRecord(new TestcaseRecordInputStream(ObjRecord.sid, (short)recdata.length, recdata));
 
-        assertEquals( recdata.length, record.getRecordSize() - 4);
+        assertEquals(28, record.getRecordSize() - 4);
 
         List subrecords = record.getSubRecords();
-        assertEquals( 2, subrecords.size() );
+        assertEquals(2, subrecords.size() );
-        assertTrue( subrecords.get(0) instanceof CommonObjectDataSubRecord);
+        assertTrue(subrecords.get(0) instanceof CommonObjectDataSubRecord);
-        assertTrue( subrecords.get(1) instanceof EndSubRecord );
+        assertTrue(subrecords.get(1) instanceof EndSubRecord );
 
     }
 
@@ -61,8 +66,8 @@ public class TestObjRecord extends TestCase {
         ObjRecord record = new ObjRecord(new TestcaseRecordInputStream(ObjRecord.sid, (short)recdata.length, recdata));
 
         byte [] recordBytes = record.serialize();
-        assertEquals(recdata.length, recordBytes.length - 4);
+        assertEquals(28, recordBytes.length - 4);
-        byte[] subData = new byte[recordBytes.length - 4];
+        byte[] subData = new byte[recdata.length];
         System.arraycopy(recordBytes, 4, subData, 0, subData.length);
         assertTrue(Arrays.equals(recdata, subData));
     }
@@ -92,4 +97,20 @@ public class TestObjRecord extends TestCase {
         assertTrue( subrecords.get(0) instanceof CommonObjectDataSubRecord);
         assertTrue( subrecords.get(1) instanceof EndSubRecord );
     }
+    
+    public void testReadWriteWithPadding_bug45133() {
+        ObjRecord record = new ObjRecord(new TestcaseRecordInputStream(ObjRecord.sid, (short)recdataNeedingPadding.length, recdataNeedingPadding));
+        
+        if (record.getRecordSize() == 34) {
+        	throw new AssertionFailedError("Identified bug 45133");
+        }
+
+        assertEquals(36, record.getRecordSize());
+
+        List subrecords = record.getSubRecords();
+        assertEquals(3, subrecords.size() );
+        assertEquals(CommonObjectDataSubRecord.class, subrecords.get(0).getClass());
+        assertEquals(GroupMarkerSubRecord.class, subrecords.get(1).getClass());
+        assertEquals(EndSubRecord.class, subrecords.get(2).getClass());
+    }
 }

src/testcases/org/apache/poi/hssf/record/TestSharedFormulaRecord.java

@@ -0,0 +1,97 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.record;
+
+import java.util.List;
+import java.util.Stack;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.ComparisonFailure;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.RefAPtg;
+
+/**
+ * @author Josh Micich
+ */
+public final class TestSharedFormulaRecord extends TestCase {
+
+	/**
+	 * Binary data for an encoded formula.  Taken from attachment 22062 (bugzilla 45123/45421).
+	 * The shared formula is in Sheet1!C6:C21, with text "SUMPRODUCT(--(End_Acct=$C6),--(End_Bal))"
+	 * This data is found at offset 0x1A4A (within the shared formula record).
+	 * The critical thing about this formula is that it contains shared formula tokens (tRefN*,
+	 * tAreaN*) with operand class 'array'.
+	 */
+	private static final byte[] SHARED_FORMULA_WITH_REF_ARRAYS_DATA = {
+		0x1A, 0x00,
+		0x63, 0x02, 0x00, 0x00, 0x00,
+		0x6C, 0x00, 0x00, 0x02, (byte)0x80,  // tRefNA
+		0x0B,
+		0x15,
+		0x13,
+		0x13,
+		0x63, 0x03, 0x00, 0x00, 0x00,
+		0x15,
+		0x13,
+		0x13,
+		0x42, 0x02, (byte)0xE4, 0x00,
+	};
+	
+	/**
+	 * The method <tt>SharedFormulaRecord.convertSharedFormulas()</tt> converts formulas from
+	 * 'shared formula' to 'single cell formula' format.  It is important that token operand
+	 * classes are preserved during this transformation, because Excel may not tolerate the
+	 * incorrect encoding.  The formula here is one such example (Excel displays #VALUE!).
+	 */
+	public void testConvertSharedFormulasOperandClasses_bug45123() {
+		
+		TestcaseRecordInputStream in = new TestcaseRecordInputStream(0, SHARED_FORMULA_WITH_REF_ARRAYS_DATA);
+		short encodedLen = in.readShort();
+		Stack sharedFormula = Ptg.createParsedExpressionTokens(encodedLen, in);
+		
+		Stack convertedFormula = SharedFormulaRecord.convertSharedFormulas(sharedFormula, 100, 200);
+		
+		RefAPtg refPtg = (RefAPtg) convertedFormula.get(1);
+		assertEquals("$C101", refPtg.toFormulaString(null));
+		if (refPtg.getPtgClass() == Ptg.CLASS_REF) {
+			throw new AssertionFailedError("Identified bug 45123");
+		}
+		
+		confirmOperandClasses(toPtgArray(sharedFormula), toPtgArray(convertedFormula));
+	}
+
+	private static void confirmOperandClasses(Ptg[] originalPtgs, Ptg[] convertedPtgs) {
+		assertEquals(originalPtgs.length, convertedPtgs.length);
+		for (int i = 0; i < convertedPtgs.length; i++) {
+			Ptg originalPtg = originalPtgs[i];
+			Ptg convertedPtg = convertedPtgs[i];
+			if (originalPtg.getPtgClass() != convertedPtg.getPtgClass()) {
+				throw new ComparisonFailure("Different operand class for token[" + i + "]",
+						String.valueOf(originalPtg.getPtgClass()), String.valueOf(convertedPtg.getPtgClass()));
+			}
+		}
+	}
+
+	private static Ptg[] toPtgArray(List list) {
+		Ptg[] result = new Ptg[list.size()];
+		list.toArray(result);
+		return result;
+	}
+}

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

@@ -17,19 +17,11 @@
 
 package org.apache.poi.hssf.usermodel;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
-import java.io.IOException;
-
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.util.AreaReference;
 import org.apache.poi.hssf.util.CellReference;
-import org.apache.poi.util.TempFile;
 
 /**
  * 
@@ -49,9 +41,7 @@ public final class TestNamedRange extends TestCase {
 	}
 
 	/** Test of TestCase method, of class test.RangeTest. */
-	public void testNamedRange() 
+	public void testNamedRange() {
-		throws IOException
-	{
 		HSSFWorkbook wb = openSample("Simple.xls");
 
 		//Creating new Named Range
@@ -64,7 +54,7 @@ public final class TestNamedRange extends TestCase {
 		newNamedRange.setNameName("RangeTest");
 		//Setting its reference
 		newNamedRange.setReference(sheetName + "!$D$4:$E$8");
-  
+
 		//Getting NAmed Range
 		HSSFName namedRange1 = wb.getNameAt(0);
 		//Getting it sheet name
@@ -76,22 +66,10 @@ public final class TestNamedRange extends TestCase {
 		SanityChecker c = new SanityChecker();
 		c.checkHSSFWorkbook(wb);
 
-		File file = TempFile.createTempFile("testNamedRange", ".xls");
+		wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
-
-		FileOutputStream fileOut = new FileOutputStream(file);
-		wb.write(fileOut);
-
-		fileOut.close();
-
-		assertTrue("file exists",file.exists());
-
-		FileInputStream in = new FileInputStream(file);
-		wb = new HSSFWorkbook(in);
 		HSSFName nm =wb.getNameAt(wb.getNameIndex("RangeTest"));
 		assertTrue("Name is "+nm.getNameName(),"RangeTest".equals(nm.getNameName()));
 		assertEquals(wb.getSheetName(0)+"!$D$4:$E$8", nm.getReference());
-
-
 	}
 	 
 	/**
@@ -143,13 +121,11 @@ public final class TestNamedRange extends TestCase {
 	/**
 	 * Test that multiple named ranges can be added written and read
 	 */
-	public void testMultipleNamedWrite() 
+	public void testMultipleNamedWrite() {
-		throws IOException
-	{
 		HSSFWorkbook wb	 = new HSSFWorkbook();
 		 
 
-		HSSFSheet sheet = wb.createSheet("testSheet1");
+		wb.createSheet("testSheet1");
 		String sheetName = wb.getSheetName(0);
 
 		assertEquals("testSheet1", sheetName);
@@ -170,18 +146,7 @@ public final class TestNamedRange extends TestCase {
 		HSSFName namedRange1 = wb.getNameAt(0);
 		String referece = namedRange1.getReference();
 
-		File file = TempFile.createTempFile("testMultiNamedRange", ".xls");
+		wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
- 
-		FileOutputStream fileOut = new FileOutputStream(file);
-		wb.write(fileOut);
-		fileOut.close();
-
-		 
-		assertTrue("file exists",file.exists());
-
-
-		FileInputStream in = new FileInputStream(file);
-		wb = new HSSFWorkbook(in);
 		HSSFName nm =wb.getNameAt(wb.getNameIndex("RangeTest"));
 		assertTrue("Name is "+nm.getNameName(),"RangeTest".equals(nm.getNameName()));
 		assertTrue("Reference is "+nm.getReference(),(wb.getSheetName(0)+"!$D$4:$E$8").equals(nm.getReference()));
@@ -189,19 +154,14 @@ public final class TestNamedRange extends TestCase {
 		nm = wb.getNameAt(wb.getNameIndex("AnotherTest"));
 		assertTrue("Name is "+nm.getNameName(),"AnotherTest".equals(nm.getNameName()));
 		assertTrue("Reference is "+nm.getReference(),newNamedRange2.getReference().equals(nm.getReference()));
-
-
 	}
 
 	/**
 	 * Test case provided by czhang@cambian.com (Chun Zhang)
 	 * <p>
 	 * Addresses Bug <a href="http://issues.apache.org/bugzilla/show_bug.cgi?id=13775" target="_bug">#13775</a>
-	 * @throws IOException
 	 */
-	public void testMultiNamedRange() 
+	public void testMultiNamedRange() {
-		 throws IOException
-	 {
 
 		 // Create a new workbook
 		 HSSFWorkbook wb = new HSSFWorkbook ();
@@ -234,16 +194,8 @@ public final class TestNamedRange extends TestCase {
 		 namedRange2.setReference("sheet2" + "!$A$1:$O$21");
  
 		 // Write the workbook to a file
-		 File file = TempFile.createTempFile("testMuiltipletNamedRanges", ".xls");
-		 FileOutputStream fileOut = new FileOutputStream(file);
-		 wb.write(fileOut);
-		 fileOut.close();
-
-		 assertTrue("file exists",file.exists());
-
 		 // Read the Excel file and verify its content
-		 FileInputStream in = new FileInputStream(file);
+		 wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
-		 wb = new HSSFWorkbook(in);
 		 HSSFName nm1 =wb.getNameAt(wb.getNameIndex("RangeTest1"));
 		 assertTrue("Name is "+nm1.getNameName(),"RangeTest1".equals(nm1.getNameName()));
 		 assertTrue("Reference is "+nm1.getReference(),(wb.getSheetName(0)+"!$A$1:$L$41").equals(nm1.getReference()));
@@ -253,17 +205,15 @@ public final class TestNamedRange extends TestCase {
 		 assertTrue("Reference is "+nm2.getReference(),(wb.getSheetName(1)+"!$A$1:$O$21").equals(nm2.getReference()));
 	 }	   
 
-	public void testUnicodeNamedRange() throws Exception {
+	public void testUnicodeNamedRange() {
 		HSSFWorkbook workBook = new HSSFWorkbook();
-		HSSFSheet sheet = workBook.createSheet("Test");
+		workBook.createSheet("Test");
 		HSSFName name = workBook.createName();
 		name.setNameName("\u03B1");
 		name.setReference("Test!$D$3:$E$8");
 
-		ByteArrayOutputStream out = new ByteArrayOutputStream();
-		workBook.write(out);
 
-		HSSFWorkbook workBook2 = new HSSFWorkbook(new ByteArrayInputStream(out.toByteArray()));
+		HSSFWorkbook workBook2 = HSSFTestDataSamples.writeOutAndReadBack(workBook);
 		HSSFName name2 = workBook2.getNameAt(0);
 
 		assertEquals("\u03B1", name2.getNameName());
@@ -286,7 +236,6 @@ public final class TestNamedRange extends TestCase {
 
 	 	 assertNotNull("Print Area not defined for first sheet", retrievedPrintArea);
 		 assertEquals("'" + sheetName + "'!$A$1:$B$1", retrievedPrintArea);
-		 
 	 }
 
 	 /**
@@ -305,10 +254,8 @@ public final class TestNamedRange extends TestCase {
 
 		 assertNotNull("Print Area not defined for first sheet", retrievedPrintArea);
 		 assertEquals("'" + sheetName + "'!" + reference, retrievedPrintArea);
-		 
 	 }
 
-
 	 /**
 	  * Test to see if the print area can be retrieved from an excel created file
 	  */
@@ -321,88 +268,71 @@ public final class TestNamedRange extends TestCase {
 		assertEquals(reference, workbook.getPrintArea(0));
 	}
 
-
 	 /**
 	  * Test to see if the print area made it to the file
 	  */
-	 public void testPrintAreaFile()
+	 public void testPrintAreaFile() {
-	 throws IOException
-	 {
 	 	HSSFWorkbook workbook = new HSSFWorkbook();
-	 	HSSFSheet sheet = workbook.createSheet("Test Print Area");
+	 	workbook.createSheet("Test Print Area");
 	 	String sheetName = workbook.getSheetName(0);
 		 
 	 
 	 	String reference = sheetName+"!$A$1:$B$1";
 	 	workbook.setPrintArea(0, reference);
 		 
-		File file = TempFile.createTempFile("testPrintArea",".xls");
+		workbook = HSSFTestDataSamples.writeOutAndReadBack(workbook);
-		 
-		FileOutputStream fileOut = new FileOutputStream(file);
-		workbook.write(fileOut);
-		fileOut.close();
-		 
-		assertTrue("file exists",file.exists());
-		 
-		FileInputStream in = new FileInputStream(file);
-		workbook = new HSSFWorkbook(in);
 		 
 	 	String retrievedPrintArea = workbook.getPrintArea(0);	   
 	 	assertNotNull("Print Area not defined for first sheet", retrievedPrintArea);
 	 	assertEquals("References Match", "'" + sheetName + "'!$A$1:$B$1", retrievedPrintArea);
-		 
 	}
 
 	/**
 	 * Test to see if multiple print areas made it to the file
 	 */
-	public void testMultiplePrintAreaFile()
+	public void testMultiplePrintAreaFile() {
-	throws IOException
-	{
 		HSSFWorkbook workbook = new HSSFWorkbook();
 
-		HSSFSheet sheet = workbook.createSheet("Sheet1");
+		workbook.createSheet("Sheet1");
-		sheet = workbook.createSheet("Sheet2");
+		workbook.createSheet("Sheet2");
-		sheet = workbook.createSheet("Sheet3");
+		workbook.createSheet("Sheet3");
-
+		String reference1 = "Sheet1!$A$1:$B$1";
-		String sheetName = workbook.getSheetName(0);
+		String reference2 = "Sheet2!$B$2:$D$5";
-		String reference = null;
+		String reference3 = "Sheet3!$D$2:$F$5";
-
+		
-		reference = sheetName+"!$A$1:$B$1";
+		workbook.setPrintArea(0, reference1); 
-		workbook.setPrintArea(0, reference); 
-
-		sheetName = workbook.getSheetName(1);
-		String reference2 = sheetName+"!$B$2:$D$5";
 		workbook.setPrintArea(1, reference2);
-
-		sheetName = workbook.getSheetName(2);
-		String reference3 = sheetName+"!$D$2:$F$5";
 		workbook.setPrintArea(2, reference3);
 
-		File file = TempFile.createTempFile("testMultiPrintArea",".xls");
+		//Check created print areas
-
+		String retrievedPrintArea;
-		FileOutputStream fileOut = new FileOutputStream(file);
+		
-		workbook.write(fileOut);
+		retrievedPrintArea = workbook.getPrintArea(0);
-		fileOut.close();
-
-		assertTrue("file exists",file.exists());
-
-		FileInputStream in = new FileInputStream(file);
-		workbook = new HSSFWorkbook(in);
-
-		String retrievedPrintArea = workbook.getPrintArea(0);
 		assertNotNull("Print Area Not Found (Sheet 1)", retrievedPrintArea);
-		assertEquals(reference, retrievedPrintArea);
+		assertEquals(reference1, retrievedPrintArea);
 
-		String retrievedPrintArea2 = workbook.getPrintArea(1);
+		retrievedPrintArea = workbook.getPrintArea(1);
-		assertNotNull("Print Area Not Found (Sheet 2)", retrievedPrintArea2);
+		assertNotNull("Print Area Not Found (Sheet 2)", retrievedPrintArea);
-		assertEquals(reference2, retrievedPrintArea2);
+		assertEquals(reference2, retrievedPrintArea);
 
-		String retrievedPrintArea3 = workbook.getPrintArea(2);
+		retrievedPrintArea = workbook.getPrintArea(2);
-		assertNotNull("Print Area Not Found (Sheet 3)", retrievedPrintArea3);
+		assertNotNull("Print Area Not Found (Sheet 3)", retrievedPrintArea);
-		assertEquals(reference3, retrievedPrintArea3);
+		assertEquals(reference3, retrievedPrintArea);
 
+		// Check print areas after re-reading workbook
+		workbook = HSSFTestDataSamples.writeOutAndReadBack(workbook);
 
+		retrievedPrintArea = workbook.getPrintArea(0);
+		assertNotNull("Print Area Not Found (Sheet 1)", retrievedPrintArea);
+		assertEquals(reference1, retrievedPrintArea);
+
+		retrievedPrintArea = workbook.getPrintArea(1);
+		assertNotNull("Print Area Not Found (Sheet 2)", retrievedPrintArea);
+		assertEquals(reference2, retrievedPrintArea);
+
+		retrievedPrintArea = workbook.getPrintArea(2);
+		assertNotNull("Print Area Not Found (Sheet 3)", retrievedPrintArea);
+		assertEquals(reference3, retrievedPrintArea);
 	}
 
 	/**
@@ -530,31 +460,29 @@ public final class TestNamedRange extends TestCase {
 		HSSFSheet s = wb.getSheet(cref.getSheetName());
 		HSSFRow r = sheet.getRow(cref.getRow());
 		HSSFCell c = r.getCell(cref.getCol());
-		String contents = c.getStringCellValue();
+		String contents = c.getRichStringCellValue().getString();
 		assertEquals("Contents of cell retrieved by its named reference", contents, cvalue);
 	}
 
-    public void testDeletedReference() throws Exception {
+	public void testDeletedReference() throws Exception {
-        HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("24207.xls");
+		HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("24207.xls");
-        assertEquals(2, wb.getNumberOfNames());
+		assertEquals(2, wb.getNumberOfNames());
 
-        HSSFName name1 = wb.getNameAt(0);
+		HSSFName name1 = wb.getNameAt(0);
-        assertEquals("a", name1.getNameName());
+		assertEquals("a", name1.getNameName());
-        assertEquals("Sheet1!$A$1", name1.getReference());
+		assertEquals("Sheet1!$A$1", name1.getReference());
-        AreaReference ref1 = new AreaReference(name1.getReference());
+		AreaReference ref1 = new AreaReference(name1.getReference());
-        assertTrue("Successfully constructed first reference", true);
+		assertTrue("Successfully constructed first reference", true);
-
-        HSSFName name2 = wb.getNameAt(1);
-        assertEquals("b", name2.getNameName());
-        assertEquals("#REF!", name2.getReference());
-        assertTrue(name2.isDeleted());
-        try {
-            AreaReference ref2 = new AreaReference(name2.getReference());
-            fail("attempt to supply an invalid reference to AreaReference constructor results in exception");
-        } catch (Exception e){
-            ;
-        }
-
-    }
 
+		HSSFName name2 = wb.getNameAt(1);
+		assertEquals("b", name2.getNameName());
+		assertEquals("#REF!", name2.getReference());
+		assertTrue(name2.isDeleted());
+		try {
+			AreaReference ref2 = new AreaReference(name2.getReference());
+			fail("attempt to supply an invalid reference to AreaReference constructor results in exception");
+		} catch (StringIndexOutOfBoundsException e) { // TODO - use a different exception for this condition
+			// expected during successful test
+		}
+   }
 }