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-653812 via svnmerge from

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

........
  r651992 | nick | 2008-04-27 19:02:13 +0100 (Sun, 27 Apr 2008) | 1 line
  
  Fix from Trejkaz from bug #44857 - Avoid OOM on unknown escher records when EscherMetafileBlip is incorrect
........
  r652285 | yegor | 2008-04-30 07:18:05 +0100 (Wed, 30 Apr 2008) | 1 line
  
  start a new section for 3.1-beta2
........
  r652288 | yegor | 2008-04-30 07:19:38 +0100 (Wed, 30 Apr 2008) | 1 line
  
  correctly process PICT blips (see bug #44886)
........
  r652290 | yegor | 2008-04-30 07:21:04 +0100 (Wed, 30 Apr 2008) | 1 line
  
  more flexible creation of a cluster
........
  r652292 | yegor | 2008-04-30 07:22:02 +0100 (Wed, 30 Apr 2008) | 1 line
  
  a few more words in the release guide
........
  r652298 | yegor | 2008-04-30 07:29:11 +0100 (Wed, 30 Apr 2008) | 1 line
  
  more work on rendering ppt slides
........
  r652329 | nick | 2008-04-30 12:10:49 +0100 (Wed, 30 Apr 2008) | 1 line
  
  Tests to show that bugs 44891 and 44861 were both already fixed
........
  r652426 | josh | 2008-05-01 04:25:37 +0100 (Thu, 01 May 2008) | 1 line
  
  fixed bug 44892 - made HSSFWorkbook.getSheet(String) case insensitive
........
  r652446 | josh | 2008-05-01 08:42:18 +0100 (Thu, 01 May 2008) | 1 line
  
  44914 - Fix/suppress warning message - WARN. Unread n bytes of record 0xNN
........
  r652561 | josh | 2008-05-01 16:46:21 +0100 (Thu, 01 May 2008) | 1 line
  
  added disabled junit for bug 44916
........
  r652934 | josh | 2008-05-02 23:36:49 +0100 (Fri, 02 May 2008) | 1 line
  
  44921 - allow Ptg.writeBytes() to be called on relative ref Ptgs (RefN* and AreaN*)
........
  r652936 | josh | 2008-05-02 23:49:38 +0100 (Fri, 02 May 2008) | 1 line
  
  should have been submitted with r652934
........
  r652994 | josh | 2008-05-03 04:59:32 +0100 (Sat, 03 May 2008) | 1 line
  
  Fixed 44675 - Parameter operand classes (function metadata) required to encode SUM() etc properly. Added parse validation for number of parameters
........
  r653117 | josh | 2008-05-03 20:53:38 +0100 (Sat, 03 May 2008) | 1 line
  
  44929 - Improved error handling in HSSFWorkbook when attempting to read a BIFF5 file
........
  r653125 | josh | 2008-05-03 21:13:56 +0100 (Sat, 03 May 2008) | 1 line
  
  Swapped ArrayIndexOutOfBoundsException for plain array length check in AbstractFunctionPtg.getParameterClass(). (To help debugging when trying to find a real AIOOB)
........
  r653484 | yegor | 2008-05-05 14:59:11 +0100 (Mon, 05 May 2008) | 1 line
  
  take into account indentation in HSSFSheet.autosizeColumn
........
  r653485 | yegor | 2008-05-05 14:59:38 +0100 (Mon, 05 May 2008) | 1 line
  
  take into account indentation in HSSFSheet.autosizeColumn
........
  r653486 | yegor | 2008-05-05 15:00:30 +0100 (Mon, 05 May 2008) | 1 line
  
  getting ready to 3.1-beta2
........
  r653520 | yegor | 2008-05-05 17:12:21 +0100 (Mon, 05 May 2008) | 1 line
  
  bug #44235 is not reproducible in 3.1-beta1
........
  r653521 | yegor | 2008-05-05 17:13:24 +0100 (Mon, 05 May 2008) | 1 line
  
  restored mistakenly commented line
........
  r653551 | josh | 2008-05-05 19:30:49 +0100 (Mon, 05 May 2008) | 1 line
  
  Added test case method javadoc for old bug 44675
........
  r653608 | josh | 2008-05-05 22:38:07 +0100 (Mon, 05 May 2008) | 1 line
  
  Follow-on from 28754 - StringPtg.toFormulaString() should escape double quotes
........
  r653668 | josh | 2008-05-06 03:02:41 +0100 (Tue, 06 May 2008) | 1 line
  
  42564 - fixed ArrayPtg to use ConstantValueParser.  Fixed a few other ArrayPtg encoding issues.
........
  r653675 | josh | 2008-05-06 04:57:15 +0100 (Tue, 06 May 2008) | 1 line
  
  42570 - fixed LabelRecord to use empty string instead of null when the length is zero.
........


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

build.xml

@@ -579,6 +579,8 @@ under the License.
         file="${main.src.test}/org/apache/poi/hpsf/data"/> 
       <sysproperty key="POIFS.testdata.path"
         file="${main.src.test}/org/apache/poi/poifs/data"/> 
+      <sysproperty key="DDF.testdata.path"
+        file="${main.src.test}/org/apache/poi/ddf/data"/> 
       <sysproperty key="java.awt.headless" value="true"/>
       <formatter type="plain"/>
       <formatter type="xml"/>

src/documentation/content/xdocs/changes.xml

@@ -43,7 +43,20 @@
            <action dev="POI-DEVELOPERS" type="add">Created a common interface for handling PowerPoint files, irrespective of if they are .ppt or .pptx</action>
            <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-beta1" date="2008-04-??">
+        <release version="3.1-beta2" date="2008-05-??">
+           <action dev="POI-DEVELOPERS" type="fix">42570 - fixed LabelRecord to use empty string instead of null when the length is zero.</action>
+           <action dev="POI-DEVELOPERS" type="fix">42564 - fixed ArrayPtg to use ConstantValueParser.  Fixed a few other ArrayPtg encoding issues.</action>
+           <action dev="POI-DEVELOPERS" type="fix">Follow-on from 28754 - StringPtg.toFormulaString() should escape double quotes</action>
+           <action dev="POI-DEVELOPERS" type="fix">44929 - Improved error handling in HSSFWorkbook when attempting to read a BIFF5 file</action>
+           <action dev="POI-DEVELOPERS" type="fix">44675 - Parameter operand classes (function metadata) required to encode SUM() etc properly. Added parse validation for number of parameters</action>
+           <action dev="POI-DEVELOPERS" type="fix">44921 - allow Ptg.writeBytes() to be called on relative ref Ptgs (RefN* and AreaN*)</action>
+           <action dev="POI-DEVELOPERS" type="fix">44914 - Fix/suppress warning message "WARN. Unread n bytes of record 0xNN"</action>
+           <action dev="POI-DEVELOPERS" type="fix">44892 - made HSSFWorkbook.getSheet(String) case insensitive</action>
+           <action dev="POI-DEVELOPERS" type="fix">44886] - Correctly process PICT metafile in EscherMetafileBlip</action>
+           <action dev="POI-DEVELOPERS" type="fix">44893 - Take into account indentation in HSSFSheet.autoSizeColumn</action>
+        </release>
+        <release version="3.1-beta1" date="2008-04-28">
+           <action dev="POI-DEVELOPERS" type="fix">44857 - Avoid OOM on unknown escher records when EscherMetafileBlip is incorrect</action>
            <action dev="POI-DEVELOPERS" type="add">HSLF: Support for getting embedded sounds from slide show </action>
            <action dev="POI-DEVELOPERS" type="add">HSLF: Initial support for rendering slides into images</action>
            <action dev="POI-DEVELOPERS" type="add">HSLF: Support for getting OLE object data from slide show </action>

src/documentation/content/xdocs/index.xml

@@ -40,7 +40,24 @@
        People interested should follow the
        <link href="mailinglists.html">dev list</link> to track progress.</p>
     </section>
-    <section><title>February 06 2008 - POI 3.0.2 Released</title>
+    <section><title>POI 3.1-BETA1 Released (2008-04028)</title>
+      <p>
+        The POI team is pleased to announce the release of 3.1 BETA1 which is one of the final steps before 3.1 FINAL.
+        The status of this release is a beta, meaning that we encourage users to try it out.
+        If you find any bugs, please report them to the POI <link href="https://issues.apache.org/bugzilla/buglist.cgi?product=POI">bug database</link> or to
+        the <link href="./mailinglists.html">POI Developer List</link>.
+        </p><p>  A full list of changes	is available in
+      <link href="./changes.html">the changelog</link>, and 
+		<link href="http://www.apache.org/dyn/closer.cgi/poi/dev/">download</link>
+		the source and binaries from your
+    <link href="http://www.apache.org/dyn/closer.cgi/poi/dev/">local mirror</link>.
+      </p>
+      <p>
+        The release is also available from the central Maven repository 
+        under Group ID "org.apache.poi" and Version "3.1-beta1".
+      </p>  
+    </section>
+    <section><title>POI 3.0.2 Released</title>
       <p>The POI team is pleased to announce POI 3.0.2, the latest release of Apache POI.
         There have been many important bug fixes since the 3.0.1 release and a lot of new features.  A full list of changes	is available in 
       <link href="./changes.html">the changelog</link>, and 

src/documentation/content/xdocs/status.xml

@@ -40,7 +40,21 @@
            <action dev="POI-DEVELOPERS" type="add">Created a common interface for handling PowerPoint files, irrespective of if they are .ppt or .pptx</action>
            <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-beta1" date="2008-04-??">
+        <release version="3.1-beta2" date="2008-05-??">
+           <action dev="POI-DEVELOPERS" type="fix">42570 - fixed LabelRecord to use empty string instead of null when the length is zero.</action>
+           <action dev="POI-DEVELOPERS" type="fix">42564 - fixed ArrayPtg to use ConstantValueParser.  Fixed a few other ArrayPtg encoding issues.</action>
+           <action dev="POI-DEVELOPERS" type="fix">Follow-on from 28754 - StringPtg.toFormulaString() should escape double quotes</action>
+           <action dev="POI-DEVELOPERS" type="fix">44929 - Improved error handling in HSSFWorkbook when attempting to read a BIFF5 file</action>
+           <action dev="POI-DEVELOPERS" type="fix">44675 - Parameter operand classes (function metadata) required to encode SUM() etc properly. Added parse validation for number of parameters</action>
+           <action dev="POI-DEVELOPERS" type="fix">44921 - allow Ptg.writeBytes() to be called on relative ref Ptgs (RefN* and AreaN*)</action>
+           <action dev="POI-DEVELOPERS" type="fix">44914 - Fix/suppress warning message "WARN. Unread n bytes of record 0xNN"</action>
+           <action dev="POI-DEVELOPERS" type="fix">44892 - made HSSFWorkbook.getSheet(String) case insensitive</action>
+           <action dev="POI-DEVELOPERS" type="fix">44886] - Correctly process PICT metafile in EscherMetafileBlip</action>
+           <action dev="POI-DEVELOPERS" type="fix">44893 - Take into account indentation in HSSFSheet.autoSizeColumn</action>
+        </release>
+        <release version="3.1-beta1" date="2008-04-28">
+           <action dev="POI-DEVELOPERS" type="fix">44857 - Avoid OOM on unknown escher records when EscherMetafileBlip is incorrect</action>
+>>>>>>> .merge-right.r653675
            <action dev="POI-DEVELOPERS" type="add">HSLF: Support for getting embedded sounds from slide show </action>
            <action dev="POI-DEVELOPERS" type="add">HSLF: Initial support for rendering slides into images</action>
            <action dev="POI-DEVELOPERS" type="add">HSLF: Support for getting OLE object data from slide show </action>

src/documentation/release-guide.txt

@@ -19,7 +19,11 @@ POI Release Guide
   POI 3.0.2 and 3.1 were built using Ant 1.6.2 and Forrest 0.5
 
 (II) Making release artefacts
-  1. Update version id in build.xml. 
+  1. Update version id in build.xml
+{code:xml}
+  <property name="version.id" value="3.1-beta1"/>
+{code}
+
   2. Tag current version. Include the current revision number in the comment
 
 {code}
@@ -46,7 +50,7 @@ https://svn.apache.org/repos/asf/poi/trunk
 
   5. Start a new section in sites.xml and status.xml. 
 
-  6. Build as if the vote had passed. The buid date must be +7 days from current.
+  6. Build as if the vote had passed. The build date must be +7 days from current.
 {code}
 ant build
 {code}
@@ -109,27 +113,68 @@ Log-in on people.apache.org
 
 1. Go to ~/POI-3.1-BETA1
 
+zap previous version first.
+
+{code}
 cd ~/POI-3.1-BETA1/main
+{code}
 
 BETA and ALPHA releases:
+
+{code}
 cp *-src-* /www/www.apache.org/dist/poi/dev/src
 cp *-bin-* /www/www.apache.org/dist/poi/dev/bin
+{code}
 
 FINAL release:
+{code}
 cp *-src-* /www/www.apache.org/dist/poi/release/src
 cp *-bin-* /www/www.apache.org/dist/poi/release/bin
+{code}
 
+{code}
 cd ~/POI-3.1-BETA1/maven
-
 cp -r org.apache.poi /www/people.apache.org/repo/m1-ibiblio-rsync-repository/
 cp -r poi/poms      /www/people.apache.org/repo/m1-ibiblio-rsync-repository/poi
-
+{code}
 
 2. Make sure that the files are owned by the unix group apcvs and that they are writable by this group. 
 
 3. Wait for the distributions to appear on your favourite mirror
 
-4. Send announcements:
+4. test maven
- - to poi-user and poi-dev lists
+create a simple project and make sure the release artifacts are accessible by maven:
- - send announcements to announcement@apache.org, announcements@jakarta.apache.org
+
+{code}
+$ mvn archetype:create -DgroupId=org.apache.poi.scratchpad -DartifactId=maven-test
+cd maven-test
+{code}
+edit pom.xml and add the release artefacts to the project dependencies: 
+
+{code:xml}
+    <dependency>
+      <groupId>org.apache.poi</groupId>
+      <artifactId>poi</artifactId>
+      <version>3.1-beta1</version>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.poi</groupId>
+      <artifactId>poi-scratchpad</artifactId>
+      <version>3.1-beta1</version>
+    </dependency>
+{code}
+
+{code}
+mvn compile 
+{code}
+
+You should see [INFO] BUILD SUCCESSFUL in the end.
+
+5. Don't forget to upload the latest version of the site and javadocs
+
+6. Send announcements:
+ - to poi-user and poi-dev lists
+ - to announcement@apache.org, announcements@jakarta.apache.org
+
+Note, announcements should be sent from your @apache.org e-mail address.
 

src/java/org/apache/poi/ddf/EscherDggRecord.java

@@ -38,6 +38,7 @@ public class EscherDggRecord
     private int field_3_numShapesSaved;
     private int field_4_drawingsSaved;
     private FileIdCluster[] field_5_fileIdClusters;
+    private int maxDgId;
 
     public static class FileIdCluster
     {
@@ -87,6 +88,7 @@ public class EscherDggRecord
         for (int i = 0; i < field_5_fileIdClusters.length; i++)
         {
             field_5_fileIdClusters[i] = new FileIdCluster(LittleEndian.getInt( data, pos + size ), LittleEndian.getInt( data, pos + size + 4 ));
+            maxDgId = Math.max(maxDgId, field_5_fileIdClusters[i].getDrawingGroupId());
             size += 8;
         }
         bytesRemaining         -= size;
@@ -229,6 +231,13 @@ public class EscherDggRecord
         this.field_4_drawingsSaved = field_4_drawingsSaved;
     }
 
+    /**
+     * @return The maximum drawing group ID
+     */
+    public int getMaxDrawingGroupId(){
+        return maxDgId;
+    }
+
      public FileIdCluster[] getFileIdClusters()
     {
         return field_5_fileIdClusters;
@@ -240,10 +249,23 @@ public class EscherDggRecord
     }
 
     public void addCluster( int dgId, int numShapedUsed )
+    {
+        addCluster(dgId, numShapedUsed, true);
+    }
+
+    /**
+     * Add a new cluster
+     *
+     * @param dgId  id of the drawing group (stored in the record options)
+     * @param numShapedUsed initial value of the numShapedUsed field
+     * @param sort if true then sort clusters by drawing group id.(
+     *  In Excel the clusters are sorted but in PPT they are not)
+     */
+    public void addCluster( int dgId, int numShapedUsed, boolean sort )
     {
         List clusters = new ArrayList(Arrays.asList(field_5_fileIdClusters));
         clusters.add(new FileIdCluster(dgId, numShapedUsed));
-        Collections.sort(clusters, new Comparator()
+        if(sort) Collections.sort(clusters, new Comparator()
         {
             public int compare( Object o1, Object o2 )
             {
@@ -257,6 +279,7 @@ public class EscherDggRecord
                     return +1;
             }
         } );
+        maxDgId = Math.min(maxDgId, dgId);
         field_5_fileIdClusters = (FileIdCluster[]) clusters.toArray( new FileIdCluster[clusters.size()] );
     }
 }

src/java/org/apache/poi/ddf/EscherMetafileBlip.java

@@ -41,9 +41,20 @@ public class EscherMetafileBlip
     public static final short RECORD_ID_WMF = (short) 0xF018 + 3;
     public static final short RECORD_ID_PICT = (short) 0xF018 + 4;
 
+    /**
+     * BLIP signatures as defined in the escher spec
+     */
+    public static final short SIGNATURE_EMF  = 0x3D40;
+    public static final short SIGNATURE_WMF  = 0x2160;
+    public static final short SIGNATURE_PICT = 0x5420;
+
     private static final int HEADER_SIZE = 8;
 
     private byte[] field_1_UID;
+    /**
+     * The primary UID is only saved to disk if (blip_instance ^ blip_signature == 1)
+     */
+    private byte[] field_2_UID;
     private int field_2_cb;
     private int field_3_rcBounds_x1;
     private int field_3_rcBounds_y1;
@@ -72,6 +83,12 @@ public class EscherMetafileBlip
 
         field_1_UID = new byte[16];
         System.arraycopy( data, pos, field_1_UID, 0, 16 ); pos += 16;
+
+        if((getOptions() ^ getSignature()) == 0x10){
+            field_2_UID = new byte[16];
+            System.arraycopy( data, pos, field_2_UID, 0, 16 ); pos += 16;
+        }
+
         field_2_cb = LittleEndian.getInt( data, pos ); pos += 4;
         field_3_rcBounds_x1 = LittleEndian.getInt( data, pos ); pos += 4;
         field_3_rcBounds_y1 = LittleEndian.getInt( data, pos ); pos += 4;
@@ -118,9 +135,12 @@ public class EscherMetafileBlip
         int pos = offset;
         LittleEndian.putShort( data, pos, getOptions() ); pos += 2;
         LittleEndian.putShort( data, pos, getRecordId() ); pos += 2;
-        LittleEndian.putInt( data, getRecordSize() - HEADER_SIZE ); pos += 4;
+        LittleEndian.putInt( data, pos, getRecordSize() - HEADER_SIZE ); pos += 4;
 
-        System.arraycopy( field_1_UID, 0, data, pos, 16 ); pos += 16;
+        System.arraycopy( field_1_UID, 0, data, pos, field_1_UID.length ); pos += field_1_UID.length;
+        if((getOptions() ^ getSignature()) == 0x10){
+            System.arraycopy( field_2_UID, 0, data, pos, field_2_UID.length ); pos += field_2_UID.length;
+        }
         LittleEndian.putInt( data, pos, field_2_cb ); pos += 4;
         LittleEndian.putInt( data, pos, field_3_rcBounds_x1 ); pos += 4;
         LittleEndian.putInt( data, pos, field_3_rcBounds_y1 ); pos += 4;
@@ -135,7 +155,7 @@ public class EscherMetafileBlip
         System.arraycopy( raw_pictureData, 0, data, pos, raw_pictureData.length );
 
         listener.afterRecordSerialize(offset + getRecordSize(), getRecordId(), getRecordSize(), this);
-        return HEADER_SIZE + 16 + 1 + raw_pictureData.length;
+        return getRecordSize();
     }
 
     /**
@@ -161,7 +181,7 @@ public class EscherMetafileBlip
         }
         catch ( IOException e )
         {
-            log.log(POILogger.INFO, "Possibly corrupt compression or non-compressed data", e);
+            log.log(POILogger.WARN, "Possibly corrupt compression or non-compressed data", e);
             return data;
         }
     }
@@ -173,7 +193,11 @@ public class EscherMetafileBlip
      */
     public int getRecordSize()
     {
-        return 8 + 50 + raw_pictureData.length;
+        int size = 8 + 50 + raw_pictureData.length;
+        if((getOptions() ^ getSignature()) == 0x10){
+            size += field_2_UID.length;
+        }
+        return size;
     }
 
     public byte[] getUID()
@@ -186,6 +210,16 @@ public class EscherMetafileBlip
         this.field_1_UID = field_1_UID;
     }
 
+    public byte[] getPrimaryUID()
+    {
+        return field_2_UID;
+    }
+
+    public void setPrimaryUID( byte[] field_2_UID )
+    {
+        this.field_2_UID = field_2_UID;
+    }
+
     public int getUncompressedSize()
     {
         return field_2_cb;
@@ -264,6 +298,7 @@ public class EscherMetafileBlip
                 "  RecordId: 0x" + HexDump.toHex( getRecordId() ) + nl +
                 "  Options: 0x" + HexDump.toHex( getOptions() ) + nl +
                 "  UID: 0x" + HexDump.toHex( field_1_UID ) + nl +
+                (field_2_UID == null ? "" : ("  UID2: 0x" + HexDump.toHex( field_2_UID ) + nl)) +
                 "  Uncompressed Size: " + HexDump.toHex( field_2_cb ) + nl +
                 "  Bounds: " + getBounds() + nl +
                 "  Size in EMU: " + getSizeEMU() + nl +
@@ -273,4 +308,19 @@ public class EscherMetafileBlip
                 "  Extra Data:" + nl + extraData;
     }
 
+    /**
+     * Return the blip signature
+     *
+     * @return the blip signature
+     */
+    public short getSignature(){
+        short sig = 0;
+        switch(getRecordId()){
+            case RECORD_ID_EMF: sig = SIGNATURE_EMF; break;
+            case RECORD_ID_WMF: sig = SIGNATURE_WMF; break;
+            case RECORD_ID_PICT: sig = SIGNATURE_PICT; break;
+            default: log.log(POILogger.WARN, "Unknown metafile: " + getRecordId()); break;
+        }
+        return sig;
+    }
 }

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

@@ -122,7 +122,7 @@ public class BiffViewer {
 
     private static void dumpNormal(Record record, int startloc, short rectype, short recsize)
     {
-        //System.out.println("Offset 0x" + Integer.toHexString(startloc) + " (" + startloc + ")");
+        System.out.println("Offset 0x" + Integer.toHexString(startloc) + " (" + startloc + ")");
         System.out.println( "recordid = 0x" + Integer.toHexString( rectype ) + ", size = " + recsize );
         System.out.println( record.toString() );
 

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

@@ -380,12 +380,13 @@ public final class FormulaParser {
         } else {
             isVarArgs = !fm.hasFixedArgsLength();
             funcIx = fm.getIndex();
+            validateNumArgs(numArgs, fm);
         }
         AbstractFunctionPtg retval;
         if(isVarArgs) {
             retval = new FuncVarPtg(name, (byte)numArgs);
         } else {
-            retval = new FuncPtg(funcIx, (byte)numArgs);
+            retval = new FuncPtg(funcIx);
         }
         if (!name.equals(AbstractFunctionPtg.FUNCTION_NAME_IF)) {
             // early return for everything else besides IF()
@@ -447,6 +448,29 @@ public final class FormulaParser {
         return retval;
     }
 
+    private void validateNumArgs(int numArgs, FunctionMetadata fm) {
+        if(numArgs < fm.getMinParams()) {
+            String msg = "Too few arguments to function '" + fm.getName() + "'. ";
+            if(fm.hasFixedArgsLength()) {
+                msg += "Expected " + fm.getMinParams();
+            } else {
+                msg += "At least " + fm.getMinParams() + " were expected";
+            }
+            msg += " but got " + numArgs + ".";
+            throw new FormulaParseException(msg);
+         }
+        if(numArgs > fm.getMaxParams()) {
+            String msg = "Too many arguments to function '" + fm.getName() + "'. ";
+            if(fm.hasFixedArgsLength()) {
+                msg += "Expected " + fm.getMaxParams();
+            } else {
+                msg += "At most " + fm.getMaxParams() + " were expected";
+            }
+            msg += " but got " + numArgs + ".";
+            throw new FormulaParseException(msg);
+       }
+    }
+
     private static boolean isArgumentDelimiter(char ch) {
         return ch ==  ',' || ch == ')';
     }

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

@@ -476,9 +476,9 @@ public class Workbook implements Model
     }
 
     /**
-     * Determines whether a workbook contains the privided sheet name.
+     * Determines whether a workbook contains the provided sheet name.
      *
-     * @param name the name to test
+     * @param name the name to test (case insensitive match)
      * @param excludeSheetIdx the sheet to exclude from the check or -1 to include all sheets in the check.
      * @return true if the sheet contains the name, false otherwise.
      */
@@ -487,7 +487,7 @@ public class Workbook implements Model
         for ( int i = 0; i < boundsheets.size(); i++ )
         {
             BoundSheetRecord boundSheetRecord = (BoundSheetRecord) boundsheets.get( i );
-            if (excludeSheetIdx != i && name.equals(boundSheetRecord.getSheetname()))
+            if (excludeSheetIdx != i && name.equalsIgnoreCase(boundSheetRecord.getSheetname()))
                 return true;
         }
         return false;

src/java/org/apache/poi/hssf/record/ColumnInfoRecord.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,12 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
-/*
- * ColumnInfoRecord.java
- *
- * Created on December 8, 2001, 8:44 AM
- */
 package org.apache.poi.hssf.record;
 
 import org.apache.poi.util.LittleEndian;
@@ -29,29 +22,28 @@ import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
 
 /**
- * Title: ColumnInfo Record<P>
+ * Title: COLINFO Record<p/>
- * Description:  Defines with width and formatting for a range of columns<P>
+ * Description:  Defines with width and formatting for a range of columns<p/>
- * REFERENCE:  PG 293 Microsoft Excel 97 Developer's Kit (ISBN: 1-57231-498-2)<P>
+ * REFERENCE:  PG 293 Microsoft Excel 97 Developer's Kit (ISBN: 1-57231-498-2)<p/>
  * @author Andrew C. Oliver (acoliver at apache dot org)
  * @version 2.0-pre
  */
-
+public final class ColumnInfoRecord extends Record {
-public class ColumnInfoRecord
-    extends Record
-{
     public static final short     sid = 0x7d;
     private short                 field_1_first_col;
     private short                 field_2_last_col;
     private short                 field_3_col_width;
     private short                 field_4_xf_index;
     private short                 field_5_options;
-    static final private BitField hidden    = BitFieldFactory.getInstance(0x01);
+    private static final BitField hidden    = BitFieldFactory.getInstance(0x01);
-    static final private BitField outlevel  = BitFieldFactory.getInstance(0x0700);
+    private static final BitField outlevel  = BitFieldFactory.getInstance(0x0700);
-    static final private BitField collapsed = BitFieldFactory.getInstance(0x1000);
+    private static final BitField collapsed = BitFieldFactory.getInstance(0x1000);
+    // Excel seems write values 2, 10, and 260, even though spec says "must be zero"
     private short                 field_6_reserved;
 
     public ColumnInfoRecord()
     {
+        field_6_reserved = 2; // seems to be the most common value
     }
 
     /**
@@ -71,7 +63,18 @@ public class ColumnInfoRecord
         field_3_col_width = in.readShort();
         field_4_xf_index  = in.readShort();
         field_5_options   = in.readShort();
+        switch(in.remaining()) {
+            case 2: // usual case
                 field_6_reserved  = in.readShort();
+                break;
+            case 1:
+                // often COLINFO gets encoded 1 byte short
+                // shouldn't matter because this field is unused
+                field_6_reserved  = in.readByte(); 
+                break;
+            default:
+                throw new RuntimeException("Unusual record size remaining=(" + in.remaining() + ")");
+        }
     }
 
     protected void validateSid(short id)

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

@@ -15,30 +15,25 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record;
 
 import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.POILogFactory;
-import org.apache.poi.util.POILogger;
 import org.apache.poi.util.StringUtil;
 
 /**
- * Title:        FileSharing<P>
+ * Title:        FILESHARING<P>
  * Description:  stores the encrypted readonly for a workbook (write protect) 
- * REFERENCE:  PG 314 Microsoft Excel 97 Developer's Kit (ISBN: 1-57231-498-2)<P>
+ * This functionality is accessed from the options dialog box available when performing 'Save As'.<p/>
+ * REFERENCE:  PG 314 Microsoft Excel 97 Developer's Kit (ISBN: 1-57231-498-2)<p/>
  * @author Andrew C. Oliver (acoliver at apache dot org)
  */
-
+public final class FileSharingRecord extends Record {
-public class FileSharingRecord extends Record {
-    private static POILogger logger = POILogFactory.getLogger(FileSharingRecord.class);
 
     public final static short sid = 0x5b;
     private short             field_1_readonly;
     private short             field_2_password;
-    private byte              field_3_username_length;
+    private byte              field_3_username_unicode_options;
-    private short             field_4_unknown; // not documented
+    private String            field_3_username_value;
-    private String            field_5_username;
 
     public FileSharingRecord() {}
     
@@ -61,23 +56,15 @@ public class FileSharingRecord extends Record {
     protected void fillFields(RecordInputStream in) {
         field_1_readonly = in.readShort();
         field_2_password = in.readShort();
-        field_3_username_length = in.readByte();
         
-        // Is this really correct? The latest docs
+        int nameLen = in.readShort();
-        //  seem to hint there's nothing between the
-        //  username length and the username string
-        field_4_unknown = in.readShort();
         
-        // Ensure we don't try to read more data than
+        if(nameLen > 0) {
-        //  there actually is
+            // TODO - Current examples(3) from junits only have zero length username. 
-        if(field_3_username_length > in.remaining()) {
+            field_3_username_unicode_options = in.readByte();
-        	logger.log(POILogger.WARN, "FileSharingRecord defined a username of length " + field_3_username_length + ", but only " + in.remaining() + " bytes were left, truncating");
+            field_3_username_value = in.readCompressedUnicode(nameLen);
-        	field_3_username_length = (byte)in.remaining();
-        }
-        if(field_3_username_length > 0) {
-        	field_5_username = in.readCompressedUnicode(field_3_username_length);
         } else {
-        	field_5_username = "";
+            field_3_username_value = "";
         }
     }
 
@@ -135,45 +122,24 @@ public class FileSharingRecord extends Record {
     /**
      * @returns byte representing the length of the username field
      */
-    public byte getUsernameLength() {
+    public short getUsernameLength() {
-        return field_3_username_length ;
+        return (short) field_3_username_value.length();
-    }
-
-    /**
-     * @param byte representing the length of the username field
-     */
-    public void setUsernameLength(byte length) {
-        this.field_3_username_length = length;
     }
 
     /**
      * @returns username of the user that created the file
      */
     public String getUsername() {
-        return this.field_5_username;
+        return field_3_username_value;
     }
 
     /**
      * @param username of the user that created the file
      */
     public void setUsername(String username) {
-        this.field_5_username = username;
+        field_3_username_value = username;
-        this.field_3_username_length = (byte)username.length();
     }
 
-    /**
-     * @return short value of a "bonus field" in Excel that was not doc'd
-     */
-    public short getUnknown() {
-        return field_4_unknown;
-    }
-
-    /**
-     * @param unknown field value to set (bonus field that is not doc'd)
-     */
-    public void setUnknown(short unk) {
-        field_4_unknown = unk;
-    }
 
     public String toString() {
         StringBuffer buffer = new StringBuffer();
@@ -183,10 +149,6 @@ public class FileSharingRecord extends Record {
             .append(getReadOnly() == 1 ? "true" : "false").append("\n");
         buffer.append("    .password       = ")
             .append(Integer.toHexString(getPassword())).append("\n");
-        buffer.append("    .userlen        = ")
-            .append(Integer.toHexString(getUsernameLength())).append("\n");
-        buffer.append("    .unknown        = ")
-            .append(Integer.toHexString(getUnknown())).append("\n");
         buffer.append("    .username       = ")
             .append(getUsername()).append("\n");
         buffer.append("[/FILESHARING]\n");
@@ -194,18 +156,25 @@ public class FileSharingRecord extends Record {
     }
 
     public int serialize(int offset, byte [] data) {
+        // TODO - junit
         LittleEndian.putShort(data, 0 + offset, sid);
         LittleEndian.putShort(data, 2 + offset, (short)(getRecordSize()-4));
         LittleEndian.putShort(data, 4 + offset, getReadOnly());
         LittleEndian.putShort(data, 6 + offset, getPassword());
-        data[ 8 + offset ] =  getUsernameLength();
+        LittleEndian.putShort(data, 8 + offset, getUsernameLength());
-        LittleEndian.putShort(data, 9 + offset, getUnknown());
+        if(getUsernameLength() > 0) {
+            LittleEndian.putByte(data, 10 + offset, field_3_username_unicode_options);
             StringUtil.putCompressedUnicode( getUsername(), data, 11 + offset );
+        }
         return getRecordSize();
     }
 
     public int getRecordSize() {
-        return 11+getUsernameLength();
+        short nameLen = getUsernameLength();
+        if (nameLen < 1) {
+            return 10;
+        }
+        return 11+nameLen;
     }
 
     public short getSid() {
@@ -219,10 +188,7 @@ public class FileSharingRecord extends Record {
       FileSharingRecord clone = new FileSharingRecord();
       clone.setReadOnly(field_1_readonly);
       clone.setPassword(field_2_password);
-      clone.setUsernameLength(field_3_username_length);
+      clone.setUsername(field_3_username_value);
-      clone.setUnknown(field_4_unknown);
-      clone.setUsername(field_5_username);
       return clone;
     }
-
 }

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

@@ -40,7 +40,7 @@ import org.apache.poi.util.LittleEndian;
  * @version 2.0-pre
  */
 
-public class FormulaRecord
+public final class FormulaRecord
     extends Record
     implements CellValueRecordInterface, Comparable
 {
@@ -108,6 +108,11 @@ public class FormulaRecord
         } catch (java.lang.UnsupportedOperationException uoe)  {
           throw new RecordFormatException(uoe);
         }
+        if (in.remaining() == 10) {
+            // TODO - this seems to occur when IntersectionPtg is present
+            // 10 extra bytes are just 0x01 and 0x00
+            // This causes POI stderr: "WARN. Unread 10 bytes of record 0x6"
+        }
     }
 
     //public void setRow(short row)

src/java/org/apache/poi/hssf/record/LabelRecord.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,12 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
-/*
- * LabelRecord.java
- *
- * Created on November 11, 2001, 12:51 PM
- */
 package org.apache.poi.hssf.record;
 
 /**
@@ -33,13 +26,9 @@ package org.apache.poi.hssf.record;
  * @version 2.0-pre
  * @see org.apache.poi.hssf.record.LabelSSTRecord
  */
-
+public final class LabelRecord extends Record implements CellValueRecordInterface {
-public class LabelRecord
-    extends Record
-    implements CellValueRecordInterface
-{
     public final static short sid = 0x204;
-    //private short             field_1_row;
+
     private int               field_1_row;
     private short             field_2_column;
     private short             field_3_xf_index;
@@ -85,7 +74,6 @@ public class LabelRecord
 
     protected void fillFields(RecordInputStream in)
     {
-        //field_1_row          = LittleEndian.getShort(data, 0 + offset);
         field_1_row          = in.readUShort();
         field_2_column       = in.readShort();
         field_3_xf_index     = in.readShort();
@@ -97,23 +85,19 @@ public class LabelRecord
             } else {
                 field_6_value = in.readCompressedUnicode(field_4_string_len);
             }
-        } else field_6_value = null;
+        } else {
+            field_6_value = "";
+        }
     }
 
-/* READ ONLY ACCESS... THIS IS FOR COMPATIBILITY ONLY...USE LABELSST!
+/*
-      public void setRow(short row) {
+ * READ ONLY ACCESS... THIS IS FOR COMPATIBILITY ONLY...USE LABELSST! public
-        field_1_row = row;
+ * void setRow(short row) { field_1_row = row; }
-      }
+ * 
-
+ * public void setColumn(short col) { field_2_column = col; }
-      public void setColumn(short col) {
+ * 
-        field_2_column = col;
+ * public void setXFIndex(short index) { field_3_xf_index = index; }
-      }
-
-      public void setXFIndex(short index) {
-        field_3_xf_index = index;
-      }
  */
-    //public short getRow()
     public int getRow()
     {
         return field_1_row;

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

@@ -186,6 +186,16 @@ public final class SharedFormulaRecord extends Record {
      * counter part
      */
     protected static Stack convertSharedFormulas(Stack ptgs, int formulaRow, int formulaColumn) {
+        if(false) {
+            /*
+             * TODO - (May-2008) Stop converting relative ref Ptgs in shared formula records. 
+             * If/when POI writes out the workbook, this conversion makes an unnecessary diff in the BIFF records.
+             * Disabling this code breaks one existing junit.
+             * Some fix-up will be required to make Ptg.toFormulaString(HSSFWorkbook) work properly.
+             * That method will need 2 extra params: rowIx and colIx.
+             */
+            return ptgs;
+        }
         Stack newPtgStack = new Stack();
 
         if (ptgs != null)

src/java/org/apache/poi/hssf/record/constant/ConstantValueParser.java

@@ -24,9 +24,8 @@ import org.apache.poi.util.LittleEndian;
 
 /**
  * To support Constant Values (2.5.7) as required by the CRN record.
- * This class should probably also be used for two dimensional arrays which are encoded by 
+ * This class is also used for two dimensional arrays which are encoded by 
  * EXTERNALNAME (5.39) records and Array tokens.<p/>
- * TODO - code in ArrayPtg should be merged with this code.  It currently supports only 2 of the constant types
  * 
  * @author Josh Micich
  */

src/java/org/apache/poi/hssf/record/constant/ErrorConstant.java

@@ -47,6 +47,12 @@ public class ErrorConstant {
 	public int getErrorCode() {
 		return _errorCode;
 	}
+	public String getText() {
+		if(HSSFErrorConstants.isValidCode(_errorCode)) {
+			return HSSFErrorConstants.getText(_errorCode);
+		}
+		return "unknown error code (" + _errorCode + ")";
+	}
 
 	public static ErrorConstant valueOf(int errorCode) {
 		switch (errorCode) {
@@ -61,4 +67,11 @@ public class ErrorConstant {
 		System.err.println("Warning - unexpected error code (" + errorCode + ")");
 		return new ErrorConstant(errorCode);
 	}
+	public String toString() {
+		StringBuffer sb = new StringBuffer(64);
+		sb.append(getClass().getName()).append(" [");
+		sb.append(getText());
+		sb.append("]");
+		return sb.toString();
+	}
 }

src/java/org/apache/poi/hssf/record/formula/AbstractFunctionPtg.java

@@ -147,10 +147,12 @@ public abstract class AbstractFunctionPtg extends OperationPtg {
     }
 
     public byte getParameterClass(int index) {
-        try {
+        if (index >= paramClass.length) {
-            return paramClass[index];
+            // For var-arg (and other?) functions, the metadata does not list all the parameter
-        } catch (ArrayIndexOutOfBoundsException aioobe) {
+            // operand classes.  In these cases, all extra parameters are assumed to have the 
+            // same operand class as the last one specified.
             return paramClass[paramClass.length - 1];
         }
+        return paramClass[index];
     }
 }

src/java/org/apache/poi/hssf/record/formula/AreaNAPtg.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,19 +15,9 @@
    limitations under the License.
 ==================================================================== */
 
-/*
- * AreaPtg.java
- *
- * Created on November 17, 2001, 9:30 PM
- */
 package org.apache.poi.hssf.record.formula;
 
-import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.BitField;
-
 import org.apache.poi.hssf.record.RecordInputStream;
-import org.apache.poi.hssf.util.AreaReference;
-import org.apache.poi.hssf.util.CellReference;
 import org.apache.poi.ss.usermodel.Workbook;
 
 /**
@@ -36,8 +25,7 @@ import org.apache.poi.ss.usermodel.Workbook;
  * @author Jason Height (jheight at chariot dot net dot au)
  */
 
-public class AreaNAPtg
+public final class AreaNAPtg extends AreaPtg
-    extends AreaPtg
 {
     public final static short sid  = 0x6D;
 
@@ -50,20 +38,16 @@ public class AreaNAPtg
       super(in);
     }
 
-    public void writeBytes(byte [] array, int offset) {
-      throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
-    }
-
     public String getAreaPtgName() {
       return "AreaNAPtg";
     }
 
     public String toFormulaString(Workbook book)
     {
-      throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+      throw notImplemented();
     }
 
     public Object clone() {
-      throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+      throw notImplemented();
     }
 }

src/java/org/apache/poi/hssf/record/formula/AreaNPtg.java

@@ -36,8 +36,7 @@ import org.apache.poi.ss.usermodel.Workbook;
  * @author Jason Height (jheight at chariot dot net dot au)
  */
 
-public class AreaNPtg
+public final class AreaNPtg extends AreaPtg
-    extends AreaPtg
 {
   public final static short sid  = 0x2D;
 
@@ -50,23 +49,16 @@ public class AreaNPtg
     super(in);
   }
 
-  public void writeBytes(byte [] array, int offset) {
-    super.writeBytes(array,offset);
-    //this should be a warning...there doesn't seem to be any rationale to throwing an exception here...
-    //this excpeiton appears to break user defined named ranges...
-    //throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
-  }
-
   public String getAreaPtgName() {
     return "AreaNPtg";
   }
 
   public String toFormulaString(Workbook book)
   {
-    throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+    throw notImplemented();
   }
 
   public Object clone() {
-    throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+    throw notImplemented();
   }
 }

src/java/org/apache/poi/hssf/record/formula/AreaNVPtg.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,19 +15,9 @@
    limitations under the License.
 ==================================================================== */
 
-/*
- * AreaPtg.java
- *
- * Created on November 17, 2001, 9:30 PM
- */
 package org.apache.poi.hssf.record.formula;
 
-import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.BitField;
-
 import org.apache.poi.hssf.record.RecordInputStream;
-import org.apache.poi.hssf.util.AreaReference;
-import org.apache.poi.hssf.util.CellReference;
 import org.apache.poi.ss.usermodel.Workbook;
 
 /**
@@ -36,10 +25,7 @@ import org.apache.poi.ss.usermodel.Workbook;
  * @author  andy
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
+public final class AreaNVPtg extends AreaPtg {
-public class AreaNVPtg
-    extends AreaPtg
-{
   public final static short sid  = 0x4D;
 
   protected AreaNVPtg() {
@@ -51,20 +37,16 @@ public class AreaNVPtg
     super(in);
   }
 
-  public void writeBytes(byte [] array, int offset) {
-    throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
-  }
-
   public String getAreaPtgName() {
     return "AreaNVPtg";
   }
 
   public String toFormulaString(Workbook book)
   {
-    throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+    throw notImplemented();
   }
 
   public Object clone() {
-    throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+    throw notImplemented();
   }
 }

src/java/org/apache/poi/hssf/record/formula/AreaPtg.java

@@ -15,7 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record.formula;
 
 import org.apache.poi.util.LittleEndian;
@@ -32,10 +31,15 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  andy
  * @author Jason Height (jheight at chariot dot net dot au)
  */
+public class AreaPtg extends Ptg implements AreaI {
+    /**
+     * TODO - (May-2008) fix subclasses of AreaPtg 'AreaN~' which are used in shared formulas.
+     * see similar comment in ReferencePtg
+     */
+    protected final RuntimeException notImplemented() {
+          return new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+    }
 
-public class AreaPtg
-    extends Ptg implements AreaI
-{
     public final static short sid  = 0x25;
     private final static int  SIZE = 9;
     /** zero based, unsigned 16 bit */

src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java

@@ -17,22 +17,17 @@
 
 package org.apache.poi.hssf.record.formula;
 
-import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.BitField;
-import org.apache.poi.util.BitFieldFactory;
-import org.apache.poi.util.StringUtil;
-
-import org.apache.poi.hssf.util.CellReference;
-import org.apache.poi.ss.usermodel.Workbook;
-import org.apache.poi.hssf.record.RecordFormatException;
 import org.apache.poi.hssf.record.RecordInputStream;
-import org.apache.poi.hssf.record.SSTRecord;
 import org.apache.poi.hssf.record.UnicodeString;
+import org.apache.poi.hssf.record.constant.ConstantValueParser;
+import org.apache.poi.hssf.record.constant.ErrorConstant;
+import org.apache.poi.ss.usermodel.Workbook;
+import org.apache.poi.util.LittleEndian;
 
 /**
  * ArrayPtg - handles arrays
  * 
- * The ArrayPtg is a little wierd, the size of the Ptg when parsing initially only
+ * The ArrayPtg is a little weird, the size of the Ptg when parsing initially only
  * includes the Ptg sid and the reserved bytes. The next Ptg in the expression then follows.
  * It is only after the "size" of all the Ptgs is met, that the ArrayPtg data is actually
  * held after this. So Ptg.createParsedExpression keeps track of the number of 
@@ -40,22 +35,16 @@ import org.apache.poi.hssf.record.UnicodeString;
  *  
  * @author Jason Height (jheight at chariot dot net dot au)
  */
+public class ArrayPtg extends Ptg {
+	public static final byte sid  = 0x20;
 
-public class ArrayPtg extends Ptg
+	private static final int RESERVED_FIELD_LEN = 7;
-{
+	// TODO - fix up field visibility and subclasses
-    public final static byte sid  = 0x20;
+	protected byte[] field_1_reserved;
-    protected byte field_1_reserved;
+	// data from these fields comes after the Ptg data of all tokens in current formula
-    protected byte field_2_reserved;
-    protected byte field_3_reserved;
-    protected byte field_4_reserved;
-    protected byte field_5_reserved;
-    protected byte field_6_reserved;
-    protected byte field_7_reserved;
-    
-    
 	protected short  token_1_columns;
 	protected short token_2_rows;
-    protected Object[][] token_3_arrayValues;
+	protected Object[] token_3_arrayValues;
 
 	protected ArrayPtg() {
 	  //Required for clone methods
@@ -63,13 +52,11 @@ public class ArrayPtg extends Ptg
 
 	public ArrayPtg(RecordInputStream in)
 	{
-    	field_1_reserved = in.readByte();
+		field_1_reserved = new byte[RESERVED_FIELD_LEN];
-    	field_2_reserved = in.readByte();
+		// TODO - add readFully method to RecordInputStream
-    	field_3_reserved = in.readByte();
+		for(int i=0; i< RESERVED_FIELD_LEN; i++) {
-    	field_4_reserved = in.readByte();
+			field_1_reserved[i] = in.readByte();
-    	field_5_reserved = in.readByte();
+		}
-    	field_6_reserved = in.readByte();
-    	field_7_reserved = in.readByte();
 	}
 	
 	/** 
@@ -78,29 +65,19 @@ public class ArrayPtg extends Ptg
 	 * See page 304-305 of Excel97-2007BinaryFileFormat(xls)Specification.pdf
 	 */
 	public void readTokenValues(RecordInputStream in) {
-        token_1_columns = (short)(0x00ff & in.readByte());
+		short nColumns = in.readUByte();
-        token_2_rows = in.readShort();
+		short nRows = in.readShort();
-        
 		//The token_1_columns and token_2_rows do not follow the documentation.
 		//The number of physical rows and columns is actually +1 of these values.
 		//Which is not explicitly documented.
-        token_1_columns++;
+		nColumns++;
-        token_2_rows++;        
+		nRows++;
 		
-        token_3_arrayValues = new Object[token_1_columns][token_2_rows];
+		token_1_columns = nColumns;
+		token_2_rows = nRows;
 		
-        for (int x=0;x<token_1_columns;x++) {
+		int totalCount = nRows * nColumns;
-			for (int y=0;y<token_2_rows;y++) {
+		token_3_arrayValues = ConstantValueParser.parse(in, totalCount);
-				byte grbit = in.readByte();
-				if (grbit == 0x01) {
-					token_3_arrayValues[x][y] = new Double(in.readDouble());
-				} else if (grbit == 0x02) {
-					//Ignore the doco, it is actually a unicode string with all the
-					//trimmings ie 16 bit size, option byte etc
-					token_3_arrayValues[x][y] = in.readUnicodeString();
-				} else throw new RecordFormatException("Unknown grbit '"+grbit+"' at " + x + "," + y + " with " + in.remaining() + " bytes left");
-			}
-        }
 	}
 
 	public String toString()
@@ -111,70 +88,44 @@ public class ArrayPtg extends Ptg
 		buffer.append("rows = ").append(getRowCount()).append("\n");
 		for (int x=0;x<getColumnCount();x++) {
 			for (int y=0;y<getRowCount();y++) {
-        		Object o = token_3_arrayValues[x][y];
+				Object o = token_3_arrayValues[getValueIndex(x, y)];
 	   			buffer.append("[").append(x).append("][").append(y).append("] = ").append(o).append("\n"); 
 			}
 		}
 		return buffer.toString();
 	}
 
-    public void writeBytes(byte [] array, int offset)
+	/* package */ int getValueIndex(int colIx, int rowIx) {
-    {
+		if(colIx < 0 || colIx >= token_1_columns) {
-        array[offset++] = (byte) (sid + ptgClass);
+			throw new IllegalArgumentException("Specified colIx (" + colIx 
-        array[offset++] = field_1_reserved;
+					+ ") is outside the allowed range (0.." + (token_1_columns-1) + ")");
-        array[offset++] = field_2_reserved;
-        array[offset++] = field_3_reserved;
-        array[offset++] = field_4_reserved;
-        array[offset++] = field_5_reserved;
-        array[offset++] = field_6_reserved;
-        array[offset++] = field_7_reserved;
-        
 		}
-    public int writeTokenValueBytes(byte [] array, int offset) {
+		if(rowIx < 0 || rowIx >= token_2_rows) {
-    	int pos = 0;
+			throw new IllegalArgumentException("Specified rowIx (" + rowIx 
-    	array[pos + offset] = (byte)(token_1_columns-1);
+					+ ") is outside the allowed range (0.." + (token_2_rows-1) + ")");
-        pos++;
-        LittleEndian.putShort(array, pos+offset, (short)(token_2_rows-1));
-        pos += 2;
-        for (int x=0;x<getColumnCount();x++) {
-        	for (int y=0;y<getRowCount();y++) {
-        		Object o = token_3_arrayValues[x][y];
-        		if (o instanceof Double) {
-        			array[pos+offset] = 0x01;
-        			pos++;
-        			LittleEndian.putDouble(array, pos+offset, ((Double)o).doubleValue());
-        			pos+=8;
-        		} else if (o instanceof UnicodeString) {
-        			array[pos+offset] = 0x02;
-        			pos++;        			
-        			UnicodeString s = (UnicodeString)o;
-        			//JMH TBD Handle string continuation. Id do it now but its 4am.
-        	        UnicodeString.UnicodeRecordStats stats = new UnicodeString.UnicodeRecordStats();
-        	        s.serialize(stats, pos + offset, array);
-        	        pos += stats.recordSize; 
-        		} else throw new RuntimeException("Coding error");
 		}
-        }
+		return rowIx * token_1_columns + colIx;
-        return pos;
 	}
 
-    public void setRowCount(short row)
+	public void writeBytes(byte[] data, int offset) {
-    {
+		
-        token_2_rows = row;
+		LittleEndian.putByte(data, offset + 0, sid + ptgClass);
+		System.arraycopy(field_1_reserved, 0, data, offset+1, RESERVED_FIELD_LEN);
 	}
 
-    public short getRowCount()
+	public int writeTokenValueBytes(byte[] data, int offset) {
-    {
+
+		LittleEndian.putByte(data,  offset + 0, token_1_columns-1);
+		LittleEndian.putShort(data, offset + 1, (short)(token_2_rows-1));
+		ConstantValueParser.encode(data, offset + 3, token_3_arrayValues);
+		return 3 + ConstantValueParser.getEncodedSize(token_3_arrayValues);
+	}
+
+	public short getRowCount() {
 		return token_2_rows;
 	}
 
-    public void setColumnCount(short col)
+	public short getColumnCount() {
-    {
-        token_1_columns = (byte)col;
-    }
-
-    public short getColumnCount()
-    {
 		return token_1_columns;
 	}
 
@@ -182,19 +133,7 @@ public class ArrayPtg extends Ptg
 	public int getSize()
 	{
 		int size = 1+7+1+2;
-        for (int x=0;x<getColumnCount();x++) {
+		size += ConstantValueParser.getEncodedSize(token_3_arrayValues);
-        	for (int y=0;y<getRowCount();y++) {
-        		Object o = token_3_arrayValues[x][y];
-        		if (o instanceof UnicodeString) {
-        			size++;
-        	        UnicodeString.UnicodeRecordStats rs = new UnicodeString.UnicodeRecordStats();
-                    ((UnicodeString)o).getRecordSize(rs);        			
-        			size += rs.recordSize;
-        		} else if (o instanceof Double) {
-        			size += 9;
-        		}
-        	}
-        }
 		return size;
 	}
 
@@ -203,45 +142,52 @@ public class ArrayPtg extends Ptg
 		StringBuffer b = new StringBuffer();
 		b.append("{");
 		for (int x=0;x<getColumnCount();x++) {
-          	for (int y=0;y<getRowCount();y++) {
+		  	if (x > 0) {
-          		Object o = token_3_arrayValues[x][y];
+				b.append(";");
-        		if (o instanceof String) {
-        			b.append((String)o);
-        		} else if (o instanceof Double) {
-        			b.append(((Double)o).doubleValue());
 			}
-        		if (y != getRowCount())
+		  	for (int y=0;y<getRowCount();y++) {
+				if (y > 0) {
 					b.append(",");
 				}
-          	if (x != getColumnCount())
+		  		Object o = token_3_arrayValues[getValueIndex(x, y)];
-          		b.append(";");
+		  		b.append(getConstantText(o));
+		  	}
 		  }
 		b.append("}");
 		return b.toString();
 	}
 	
+	private static String getConstantText(Object o) {
+
+		if (o == null) {
+			return ""; // TODO - how is 'empty value' represented in formulas?
+		}
+		if (o instanceof UnicodeString) {
+			return "\"" + ((UnicodeString)o).getString() + "\"";
+		}
+		if (o instanceof Double) {
+			return ((Double)o).toString();
+		}
+		if (o instanceof Boolean) {
+			((Boolean)o).toString();
+		}
+		if (o instanceof ErrorConstant) {
+			return ((ErrorConstant)o).getText();
+		}
+		throw new IllegalArgumentException("Unexpected constant class (" + o.getClass().getName() + ")");
+	}
+	
 	public byte getDefaultOperandClass() {
 		return Ptg.CLASS_ARRAY;
 	}
 	
 	public Object clone() {
 	  ArrayPtg ptg = new ArrayPtg();
-      ptg.field_1_reserved = field_1_reserved;
+	  ptg.field_1_reserved = (byte[]) field_1_reserved.clone();
-      ptg.field_2_reserved = field_2_reserved;
-      ptg.field_3_reserved = field_3_reserved;
-      ptg.field_4_reserved = field_4_reserved;
-      ptg.field_5_reserved = field_5_reserved;
-      ptg.field_6_reserved = field_6_reserved;
-      ptg.field_7_reserved = field_7_reserved;
 	  
 	  ptg.token_1_columns = token_1_columns;
 	  ptg.token_2_rows = token_2_rows;
-      ptg.token_3_arrayValues = new Object[getColumnCount()][getRowCount()];
+	  ptg.token_3_arrayValues = (Object[]) token_3_arrayValues.clone();
-      for (int x=0;x<getColumnCount();x++) {
-      	for (int y=0;y<getRowCount();y++) {
-      		ptg.token_3_arrayValues[x][y] = token_3_arrayValues[x][y];
-      	}
-      }      
 	  ptg.setClass(ptgClass);
 	  return ptg;
 	}

src/java/org/apache/poi/hssf/record/formula/ArrayPtgA.java

@@ -17,56 +17,31 @@
 
 package org.apache.poi.hssf.record.formula;
 
-import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.BitField;
-import org.apache.poi.util.BitFieldFactory;
-import org.apache.poi.util.StringUtil;
-
-import org.apache.poi.hssf.util.CellReference;
-import org.apache.poi.ss.usermodel.Workbook;
-import org.apache.poi.hssf.record.RecordFormatException;
 import org.apache.poi.hssf.record.RecordInputStream;
-import org.apache.poi.hssf.record.SSTRecord;
-import org.apache.poi.hssf.record.UnicodeString;
 
 /**
  * ArrayPtgA - handles arrays
  *  
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
+public final class ArrayPtgA extends ArrayPtg {
-public class ArrayPtgA extends ArrayPtg
-{
     public final static byte sid  = 0x60;
 
-    protected ArrayPtgA() {
+    private ArrayPtgA() {
-    	super();
       //Required for clone methods
     }
 
-    public ArrayPtgA(RecordInputStream in)
+    public ArrayPtgA(RecordInputStream in) {
-    {
     	super(in);
     }
         
     public Object clone() {
       ArrayPtgA ptg = new ArrayPtgA();
-      ptg.field_1_reserved = field_1_reserved;
+      ptg.field_1_reserved = (byte[]) field_1_reserved.clone();
-      ptg.field_2_reserved = field_2_reserved;
-      ptg.field_3_reserved = field_3_reserved;
-      ptg.field_4_reserved = field_4_reserved;
-      ptg.field_5_reserved = field_5_reserved;
-      ptg.field_6_reserved = field_6_reserved;
-      ptg.field_7_reserved = field_7_reserved;
       
       ptg.token_1_columns = token_1_columns;
       ptg.token_2_rows = token_2_rows;
-      ptg.token_3_arrayValues = new Object[getColumnCount()][getRowCount()];
+      ptg.token_3_arrayValues = (Object[]) token_3_arrayValues.clone();
-      for (int x=0;x<getColumnCount();x++) {
-      	for (int y=0;y<getRowCount();y++) {
-      		ptg.token_3_arrayValues[x][y] = token_3_arrayValues[x][y];
-      	}
-      }      
       ptg.setClass(ptgClass);
       return ptg;
     }

src/java/org/apache/poi/hssf/record/formula/ArrayPtgV.java

@@ -17,22 +17,12 @@
 
 package org.apache.poi.hssf.record.formula;
 
-import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.BitField;
-import org.apache.poi.util.BitFieldFactory;
-import org.apache.poi.util.StringUtil;
-
-import org.apache.poi.hssf.util.CellReference;
-import org.apache.poi.ss.usermodel.Workbook;
-import org.apache.poi.hssf.record.RecordFormatException;
 import org.apache.poi.hssf.record.RecordInputStream;
-import org.apache.poi.hssf.record.SSTRecord;
-import org.apache.poi.hssf.record.UnicodeString;
 
 /**
  * ArrayPtg - handles arrays
  * 
- * The ArrayPtg is a little wierd, the size of the Ptg when parsing initially only
+ * The ArrayPtg is a little weird, the size of the Ptg when parsing initially only
  * includes the Ptg sid and the reserved bytes. The next Ptg in the expression then follows.
  * It is only after the "size" of all the Ptgs is met, that the ArrayPtg data is actually
  * held after this. So Ptg.createParsedExpression keeps track of the number of 
@@ -40,38 +30,24 @@ import org.apache.poi.hssf.record.UnicodeString;
  *  
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
+public final class ArrayPtgV extends ArrayPtg {
-public class ArrayPtgV extends ArrayPtg
-{
     public final static byte sid  = 0x40;
 
-    protected ArrayPtgV() {
+    private ArrayPtgV() {
       //Required for clone methods
     }
 
-    public ArrayPtgV(RecordInputStream in)
+    public ArrayPtgV(RecordInputStream in) {
-    {
     	super(in);
     }
     
     public Object clone() {
       ArrayPtgV ptg = new ArrayPtgV();
-      ptg.field_1_reserved = field_1_reserved;
+      ptg.field_1_reserved = (byte[]) field_1_reserved.clone();
-      ptg.field_2_reserved = field_2_reserved;
-      ptg.field_3_reserved = field_3_reserved;
-      ptg.field_4_reserved = field_4_reserved;
-      ptg.field_5_reserved = field_5_reserved;
-      ptg.field_6_reserved = field_6_reserved;
-      ptg.field_7_reserved = field_7_reserved;
       
       ptg.token_1_columns = token_1_columns;
       ptg.token_2_rows = token_2_rows;
-      ptg.token_3_arrayValues = new Object[getColumnCount()][getRowCount()];
+      ptg.token_3_arrayValues = (Object[]) token_3_arrayValues.clone();
-      for (int x=0;x<getColumnCount();x++) {
-      	for (int y=0;y<getRowCount();y++) {
-      		ptg.token_3_arrayValues[x][y] = token_3_arrayValues[x][y];
-      	}
-      }      
       ptg.setClass(ptgClass);
       return ptg;
     }

src/java/org/apache/poi/hssf/record/formula/FuncPtg.java

@@ -57,10 +57,12 @@ public final class FuncPtg extends AbstractFunctionPtg {
         }
         numParams = fm.getMinParams();
     }
-    public FuncPtg(int functionIndex, int numberOfParameters) {
+    public FuncPtg(int functionIndex) {
         field_2_fnc_index = (short) functionIndex;
-        numParams = numberOfParameters;
+        FunctionMetadata fm = FunctionMetadataRegistry.getFunctionByIndex(functionIndex);
-        paramClass = new byte[] { Ptg.CLASS_VALUE, }; // TODO
+        numParams = fm.getMinParams(); // same as max since these are not var-arg funcs
+        returnClass = fm.getReturnClassCode();
+        paramClass = fm.getParameterClassCodes();
     }
 
     public void writeBytes(byte[] array, int offset) {

src/java/org/apache/poi/hssf/record/formula/FuncVarPtg.java

@@ -54,8 +54,8 @@ public final class FuncVarPtg extends AbstractFunctionPtg{
             returnClass = Ptg.CLASS_VALUE;
             paramClass = new byte[] {Ptg.CLASS_VALUE};
         } else {
-            returnClass = Ptg.CLASS_VALUE;
+            returnClass = fm.getReturnClassCode();
-            paramClass = new byte[] {Ptg.CLASS_VALUE};
+            paramClass = fm.getParameterClassCodes();
         }
     }
 

src/java/org/apache/poi/hssf/record/formula/Ptg.java

@@ -30,7 +30,6 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author avik
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
 public abstract class Ptg
 {
 
@@ -88,6 +87,10 @@ public abstract class Ptg
     }
     */
 
+    /**
+     * Reads <tt>size</tt> bytes of the input stream, to create an array of <tt>Ptg</tt>s.
+     * Extra data (beyond <tt>size</tt>) may be read if and <tt>ArrayPtg</tt>s are present.
+     */
     public static Stack createParsedExpressionTokens(short size,  RecordInputStream in )
     {
         Stack stack = new Stack();
@@ -104,6 +107,9 @@ public abstract class Ptg
             } else pos += ptg.getSize();
             stack.push( ptg );
         }
+        if(pos != size) {
+            throw new RuntimeException("Ptg array size mismatch");
+        }
         if (arrayPtgs != null) {
             for (int i=0;i<arrayPtgs.size();i++) {
                 ArrayPtg p = (ArrayPtg)arrayPtgs.get(i);

src/java/org/apache/poi/hssf/record/formula/RefNAPtg.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,26 +15,16 @@
    limitations under the License.
 ==================================================================== */
 
-/*
- * ValueReferencePtg.java
- *
- * Created on November 21, 2001, 5:27 PM
- */
 package org.apache.poi.hssf.record.formula;
 
-import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.BitField;
-
 import org.apache.poi.hssf.record.RecordInputStream;
-import org.apache.poi.hssf.util.CellReference;
 import org.apache.poi.ss.usermodel.Workbook;
 
 /**
  * RefNAPtg
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
+public final class RefNAPtg extends ReferencePtg
-public class RefNAPtg extends ReferencePtg
 {
     public final static byte sid  = 0x6C;
 
@@ -48,21 +37,16 @@ public class RefNAPtg extends ReferencePtg
       super(in);
     }
 
-    public void writeBytes(byte [] array, int offset)
-    {
-      throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
-    }
-
     public String getRefPtgName() {
       return "RefNAPtg";
     }
 
     public String toFormulaString(Workbook book)
     {
-      throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+      throw notImplemented();
     }
 
     public Object clone() {
-      throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+        throw notImplemented();
     }
 }

src/java/org/apache/poi/hssf/record/formula/RefNPtg.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,25 +15,16 @@
    limitations under the License.
 ==================================================================== */
 
-/*
- * RefNPtg.java
- *
- */
 package org.apache.poi.hssf.record.formula;
 
-import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.BitField;
-
 import org.apache.poi.hssf.record.RecordInputStream;
-import org.apache.poi.hssf.util.CellReference;
 import org.apache.poi.ss.usermodel.Workbook;
 
 /**
  * RefNPtg
  * @author Jason Height (jheight at apache dot com)
  */
-
+public final class RefNPtg extends ReferencePtg
-public class RefNPtg extends ReferencePtg
 {
     public final static byte sid  = 0x2C;
 
@@ -49,21 +39,16 @@ public class RefNPtg extends ReferencePtg
       super(in);
     }
 
-    public void writeBytes(byte [] array, int offset)
-    {
-      throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
-    }
-
     public String getRefPtgName() {
       return "RefNPtg";
     }
 
     public String toFormulaString(Workbook book)
     {
-      throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+        throw notImplemented();
     }
 
     public Object clone() {
-      throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+        throw notImplemented();
     }
 }

src/java/org/apache/poi/hssf/record/formula/RefNVPtg.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,11 +17,7 @@
 
 package org.apache.poi.hssf.record.formula;
 
-import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.BitField;
-
 import org.apache.poi.hssf.record.RecordInputStream;
-import org.apache.poi.hssf.util.CellReference;
 import org.apache.poi.ss.usermodel.Workbook;
 
 /**
@@ -30,8 +25,7 @@ import org.apache.poi.ss.usermodel.Workbook;
  * @author Jason Height (jheight at chariot dot net dot au)
  */
 
-public class RefNVPtg extends ReferencePtg
+public final class RefNVPtg extends ReferencePtg {
-{
   public final static byte sid  = 0x4C;
 
   protected RefNVPtg() {
@@ -45,21 +39,16 @@ public class RefNVPtg extends ReferencePtg
     super(in);
   }
 
-  public void writeBytes(byte [] array, int offset)
-  {
-    throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
-  }
-
   public String getRefPtgName() {
     return "RefNVPtg";
   }
 
   public String toFormulaString(Workbook book)
   {
-    throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+      throw notImplemented();
   }
 
   public Object clone() {
-    throw new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+      throw notImplemented();
   }
 }

src/java/org/apache/poi/hssf/record/formula/ReferencePtg.java

@@ -30,8 +30,20 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  Andrew C. Oliver (acoliver@apache.org)
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
 public class ReferencePtg extends Ptg {
+    /**
+     * TODO - (May-2008) fix subclasses of ReferencePtg 'RefN~' which are used in shared formulas.
+     * (See bugzilla 44921)
+     * The 'RefN~' instances do not work properly, and are expected to be converted by 
+     * SharedFormulaRecord.convertSharedFormulas().  
+     * This conversion currently does not take place for formulas of named ranges, conditional 
+     * format rules and data validation rules.  
+     * Furthermore, conversion is probably not appropriate in those instances.
+     */
+    protected final RuntimeException notImplemented() {
+        return new RuntimeException("Coding Error: This method should never be called. This ptg should be converted");
+    }
+
     private final static int SIZE = 5;
     public final static byte sid  = 0x24;
     private final static int MAX_ROW_NUMBER = 65536;             

src/java/org/apache/poi/hssf/record/formula/StringPtg.java

@@ -24,95 +24,106 @@ import org.apache.poi.util.StringUtil;
 import org.apache.poi.hssf.record.RecordInputStream;
 
 /**
- * Number
+ * String Stores a String value in a formula value stored in the format
- * Stores a String value in a formula value stored in the format <length 2 bytes>char[]
+ * <length 2 bytes>char[]
+ * 
  * @author Werner Froidevaux
  * @author Jason Height (jheight at chariot dot net dot au)
  * @author Bernard Chesnoy
  */
-
+public final class StringPtg extends Ptg {
-public class StringPtg
-    extends Ptg
-{
     public final static int SIZE = 9;
     public final static byte sid = 0x17;
-    //NOTE: OO doc says 16bit lenght, but BiffViewer says 8
+    private static final BitField fHighByte = BitFieldFactory.getInstance(0x01);
-    // Book says something totally different, so dont look there!
+    /** the character (")used in formulas to delimit string literals */
-    int field_1_length;
+    private static final char FORMULA_DELIMITER = '"';
-    byte field_2_options;
+
-    BitField fHighByte = BitFieldFactory.getInstance(0x01);
+    /**
+     * NOTE: OO doc says 16bit length, but BiffViewer says 8 Book says something
+     * totally different, so don't look there!
+     */
+    private int field_1_length;
+    private byte field_2_options;
     private String field_3_string;
 
     private StringPtg() {
-      //Required for clone methods
+        // Required for clone methods
     }
 
     /** Create a StringPtg from a byte array read from disk */
-    public StringPtg(RecordInputStream in)
+    public StringPtg(RecordInputStream in) {
-    {
+        field_1_length = in.readUByte();
-        field_1_length = in.readByte() & 0xFF;
         field_2_options = in.readByte();
         if (fHighByte.isSet(field_2_options)) {
-            field_3_string= in.readUnicodeLEString(field_1_length);
+            field_3_string = in.readUnicodeLEString(field_1_length);
-        }else {
+        } else {
-            field_3_string=in.readCompressedUnicode(field_1_length);
+            field_3_string = in.readCompressedUnicode(field_1_length);
         }
 
-        //setValue(new String(data, offset+3, data[offset+1] + 256*data[offset+2]));
+        // setValue(new String(data, offset+3, data[offset+1] + 256*data[offset+2]));
     }
 
-    /** Create a StringPtg from a string representation of  the number
+    /**
-     *  Number format is not checked, it is expected to be validated in the parser
+     * Create a StringPtg from a string representation of the number Number
-     *   that calls this method.
+     * format is not checked, it is expected to be validated in the parser that
-     *  @param value : String representation of a floating point number
+     * calls this method.
+     * 
+     * @param value :
+     *            String representation of a floating point number
      */
     public StringPtg(String value) {
-        if (value.length() >255) {
+        if (value.length() > 255) {
-            throw new IllegalArgumentException("String literals in formulas cant be bigger than 255 characters ASCII");
+            throw new IllegalArgumentException(
+                    "String literals in formulas can't be bigger than 255 characters ASCII");
         }
-        this.field_2_options=0;        
+        field_2_options = 0;
-        field_2_options = (byte)this.fHighByte.setBoolean(field_2_options, StringUtil.hasMultibyte(value));
+        field_2_options = (byte) fHighByte.setBoolean(field_2_options, StringUtil
-        this.field_3_string=value;
+                .hasMultibyte(value));
-        this.field_1_length=value.length(); //for the moment, we support only ASCII strings in formulas we create
+        field_3_string = value;
+        field_1_length = value.length(); // for the moment, we support only ASCII strings in formulas we create
     }
 
-    /*
+    public String getValue() {
-    public void setValue(String value)
-    {
-        field_1_value = value;
-    }*/
-
-
-    public String getValue()
-    {
         return field_3_string;
     }
 
-    public void writeBytes(byte [] array, int offset)
+    public void writeBytes(byte[] array, int offset) {
-    {
+        array[offset + 0] = sid;
-        array[ offset + 0 ] = sid;
+        array[offset + 1] = (byte) field_1_length;
-        array[ offset + 1 ] = (byte)field_1_length;
+        array[offset + 2] = field_2_options;
-        array[ offset + 2 ] = field_2_options;
         if (fHighByte.isSet(field_2_options)) {
-            StringUtil.putUnicodeLE(getValue(),array,offset+3);
+            StringUtil.putUnicodeLE(getValue(), array, offset + 3);
-        }else {
-            StringUtil.putCompressedUnicode(getValue(),array,offset+3);
-        }
-    }
-
-    public int getSize()
-    {
-        if (fHighByte.isSet(field_2_options)) {
-            return 2*field_1_length+3;
         } else {
-            return field_1_length+3;
+            StringUtil.putCompressedUnicode(getValue(), array, offset + 3);
         }
     }
 
-    public String toFormulaString(Workbook book)
+    public int getSize() {
-    {
+        if (fHighByte.isSet(field_2_options)) {
-        return "\""+getValue()+"\"";
+            return 2 * field_1_length + 3;
+        } else {
+            return field_1_length + 3;
         }
+    }
+
+    public String toFormulaString(Workbook book) {
+        String value = field_3_string;
+        int len = value.length();
+        StringBuffer sb = new StringBuffer(len + 4);
+        sb.append(FORMULA_DELIMITER);
+
+        for (int i = 0; i < len; i++) {
+            char c = value.charAt(i);
+            if (c == FORMULA_DELIMITER) {
+                sb.append(FORMULA_DELIMITER);
+            }
+            sb.append(c);
+        }
+
+        sb.append(FORMULA_DELIMITER);
+        return sb.toString();
+    }
+
     public byte getDefaultOperandClass() {
         return Ptg.CLASS_VALUE;
     }
@@ -120,10 +131,16 @@ public class StringPtg
     public Object clone() {
         StringPtg ptg = new StringPtg();
         ptg.field_1_length = field_1_length;
-     ptg.field_2_options=field_2_options;
+        ptg.field_2_options = field_2_options;
-     ptg.field_3_string=field_3_string;
+        ptg.field_3_string = field_3_string;
         return ptg;
     }
 
+    public String toString() {
+        StringBuffer sb = new StringBuffer(64);
+        sb.append(getClass().getName()).append(" [");
+        sb.append(field_3_string);
+        sb.append("]");
+        return sb.toString();
+    }
 }
-

src/java/org/apache/poi/hssf/record/formula/function/FunctionDataBuilder.java

@@ -42,8 +42,10 @@ final class FunctionDataBuilder {
 		_mutatingFunctionIndexes = new HashSet();
 	}
 
-	public void add(int functionIndex, String functionName, int minParams, int maxParams, boolean hasFootnote) {
+	public void add(int functionIndex, String functionName, int minParams, int maxParams,
-		FunctionMetadata fm = new FunctionMetadata(functionIndex, functionName, minParams, maxParams);
+			byte returnClassCode, byte[] parameterClassCodes, boolean hasFootnote) {
+		FunctionMetadata fm = new FunctionMetadata(functionIndex, functionName, minParams, maxParams,
+				returnClassCode, parameterClassCodes);
 		
 		Integer indexKey = new Integer(functionIndex);
 		

src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadata.java

@@ -27,12 +27,17 @@ public final class FunctionMetadata {
 	private final String _name;
 	private final int _minParams;
 	private final int _maxParams;
+	private final byte _returnClassCode;
+	private final byte[] _parameterClassCodes;
 
-	/* package */ FunctionMetadata(int index, String name, int minParams, int maxParams) {
+	/* package */ FunctionMetadata(int index, String name, int minParams, int maxParams,
+			byte returnClassCode, byte[] parameterClassCodes) {
 		_index = index;
 		_name = name;
 		_minParams = minParams;
 		_maxParams = maxParams;
+		_returnClassCode = returnClassCode;
+		_parameterClassCodes = parameterClassCodes;
 	}
 	public int getIndex() {
 		return _index;
@@ -49,6 +54,12 @@ public final class FunctionMetadata {
 	public boolean hasFixedArgsLength() {
 		return _minParams == _maxParams;
 	}
+	public byte getReturnClassCode() {
+		return _returnClassCode;
+	}
+	public byte[] getParameterClassCodes() {
+		return (byte[]) _parameterClassCodes.clone();
+	}
 	public String toString() {
 		StringBuffer sb = new StringBuffer(64);
 		sb.append(getClass().getName()).append(" [");

src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadataReader.java

@@ -26,6 +26,8 @@ import java.util.HashSet;
 import java.util.Set;
 import java.util.regex.Pattern;
 
+import org.apache.poi.hssf.record.formula.Ptg;
+
 /**
  * Converts the text meta-data file into a <tt>FunctionMetadataRegistry</tt>
  * 
@@ -36,6 +38,12 @@ final class FunctionMetadataReader {
 	private static final String METADATA_FILE_NAME = "functionMetadata.txt";
 
 	private static final Pattern TAB_DELIM_PATTERN = Pattern.compile("\t");
+	private static final Pattern SPACE_DELIM_PATTERN = Pattern.compile(" ");
+	private static final byte[] EMPTY_BYTE_ARRAY = { };
+
+	// special characters from the ooo document
+	private static final int CHAR_ELLIPSIS_8230 = 8230;
+	private static final int CHAR_NDASH_8211 = 8211;
 	
 	private static final String[] DIGIT_ENDING_FUNCTION_NAMES = {
 		// Digits at the end of a function might be due to a left-over footnote marker.
@@ -86,14 +94,66 @@ final class FunctionMetadataReader {
 		String functionName = parts[1];
 		int minParams = parseInt(parts[2]);
 		int maxParams = parseInt(parts[3]);
-		// 4 returnClass
+		byte returnClassCode = parseReturnTypeCode(parts[4]);
-		// 5 parameterClasses
+		byte[] parameterClassCodes = parseOperandTypeCodes(parts[5]);
 		// 6 isVolatile
 		boolean hasNote = parts[7].length() > 0;
 
 		validateFunctionName(functionName);
-		// TODO - make POI use returnClass, parameterClasses, isVolatile
+		// TODO - make POI use isVolatile
-		fdb.add(functionIndex, functionName, minParams, maxParams, hasNote);
+		fdb.add(functionIndex, functionName, minParams, maxParams, 
+				returnClassCode, parameterClassCodes, hasNote);
+	}
+	
+
+	private static byte parseReturnTypeCode(String code) {
+		if(code.length() == 0) {
+			return Ptg.CLASS_REF; // happens for GETPIVOTDATA
+		}
+		return parseOperandTypeCode(code);
+	}
+
+	private static byte[] parseOperandTypeCodes(String codes) {
+		if(codes.length() < 1) {
+			return EMPTY_BYTE_ARRAY; // happens for GETPIVOTDATA
+		}
+		if(isDash(codes)) {
+			// '-' means empty:
+			return EMPTY_BYTE_ARRAY;
+		}
+		String[] array = SPACE_DELIM_PATTERN.split(codes);
+		int nItems = array.length;
+		if(array[nItems-1].charAt(0) == CHAR_ELLIPSIS_8230) {
+			nItems --;
+		}
+		byte[] result = new byte[nItems];
+		for (int i = 0; i < nItems; i++) {
+			result[i] = parseOperandTypeCode(array[i]);
+		}
+		return result;
+	}
+
+	private static boolean isDash(String codes) {
+		if(codes.length() == 1) {
+			switch (codes.charAt(0)) {
+				case '-':
+				case CHAR_NDASH_8211: // this is what the ooo doc has
+					return true;
+			}
+		}
+		return false;
+	}
+
+	private static byte parseOperandTypeCode(String code) {
+		if(code.length() != 1) {
+			throw new RuntimeException("Bad operand type code format '" + code  + "' expected single char");
+		}
+		switch(code.charAt(0)) {
+			case 'V': return Ptg.CLASS_VALUE;
+			case 'R': return Ptg.CLASS_REF;
+			case 'A': return Ptg.CLASS_ARRAY;
+		}
+		throw new IllegalArgumentException("Unexpected operand type code '" + code + "'");
 	}
 
 	/**

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

@@ -17,7 +17,9 @@
 
 package org.apache.poi.hssf.usermodel;
 
+import org.apache.poi.ddf.EscherBitmapBlip;
 import org.apache.poi.ddf.EscherBlipRecord;
+import org.apache.poi.ddf.EscherMetafileBlip;
 import org.apache.poi.ss.usermodel.PictureData;
 
 /**
@@ -65,19 +67,19 @@ public class HSSFPictureData implements PictureData
      */
     public String suggestFileExtension()
     {
-        switch (blip.getOptions() & FORMAT_MASK)
+        switch (blip.getRecordId())
         {
-            case MSOBI_WMF:
+            case EscherMetafileBlip.RECORD_ID_WMF:
                 return "wmf";
-            case MSOBI_EMF:
+            case EscherMetafileBlip.RECORD_ID_EMF:
                 return "emf";
-            case MSOBI_PICT:
+            case EscherMetafileBlip.RECORD_ID_PICT:
                 return "pict";
-            case MSOBI_PNG:
+            case EscherBitmapBlip.RECORD_ID_PNG:
                 return "png";
-            case MSOBI_JPEG:
+            case EscherBitmapBlip.RECORD_ID_JPEG:
                 return "jpeg";
-            case MSOBI_DIB:
+            case EscherBitmapBlip.RECORD_ID_DIB:
                 return "dib";
             default:
                 return "";

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

@@ -1728,6 +1728,8 @@ public class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet
 
             HSSFCellStyle style = cell.getCellStyle();
             HSSFFont font = wb.getFontAt(style.getFontIndex());
+            //the number of spaces to indent the text in the cell
+            int indention = style.getIndention();
 
             if (cell.getCellType() == HSSFCell.CELL_TYPE_STRING) {
                 HSSFRichTextString rt = cell.getRichStringCellValue();
@@ -1760,9 +1762,9 @@ public class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet
                         trans.concatenate(
                         AffineTransform.getScaleInstance(1, fontHeightMultiple)
                         );
-                        width = Math.max(width, layout.getOutline(trans).getBounds().getWidth() / defaultCharWidth);
+                        width = Math.max(width, layout.getOutline(trans).getBounds().getWidth() / defaultCharWidth + indention);
                     } else {
-                        width = Math.max(width, layout.getBounds().getWidth() / defaultCharWidth);
+                        width = Math.max(width, layout.getBounds().getWidth() / defaultCharWidth + indention);
                     }
                 }
             } else {
@@ -1805,15 +1807,15 @@ public class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet
                         trans.concatenate(
                         AffineTransform.getScaleInstance(1, fontHeightMultiple)
                         );
-                        width = Math.max(width, layout.getOutline(trans).getBounds().getWidth() / defaultCharWidth);
+                        width = Math.max(width, layout.getOutline(trans).getBounds().getWidth() / defaultCharWidth + indention);
                     } else {
-                        width = Math.max(width, layout.getBounds().getWidth() / defaultCharWidth);
+                        width = Math.max(width, layout.getBounds().getWidth() / defaultCharWidth + indention);
                     }
                 }
             }
 
             if (width != -1) {
-                if (width > Short.MAX_VALUE) { //width can be bigger that Short.MAX_VALUE!
+                if (width > Short.MAX_VALUE) { //calculated width can be greater that Short.MAX_VALUE!
                      width = Short.MAX_VALUE;
                 }
                 sheet.setColumnWidth(column, (short) (width * 256));

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

@@ -185,6 +185,42 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
         this(fs.getRoot(), fs, preserveNodes);
     }
 
+    /**
+     * Normally, the Workbook will be in a POIFS Stream
+     * called "Workbook". However, some weird XLS generators use "WORKBOOK"
+     */
+    private static final String[] WORKBOOK_DIR_ENTRY_NAMES = {
+        "Workbook", // as per BIFF8 spec
+        "WORKBOOK",
+    };
+
+
+    private static String getWorkbookDirEntryName(DirectoryNode directory) {
+
+        String[] potentialNames = WORKBOOK_DIR_ENTRY_NAMES;
+        for (int i = 0; i < potentialNames.length; i++) {
+            String wbName = potentialNames[i];
+            try {
+                directory.getEntry(wbName);
+                return wbName;
+            } catch (FileNotFoundException e) {
+                // continue - to try other options
+            }
+        }
+
+        // check for previous version of file format
+        try {
+            directory.getEntry("Book");
+            throw new IllegalArgumentException("The supplied spreadsheet seems to be Excel 5.0/7.0 (BIFF5) format. "
+                    + "POI only supports BIFF8 format (from Excel versions 97/2000/XP/2003)");
+        } catch (FileNotFoundException e) {
+            // fall through
+        }
+
+        throw new IllegalArgumentException("The supplied POIFSFileSystem does not contain a BIFF8 'Workbook' entry. "
+            + "Is it really an excel file?");
+    }
+
     /**
      * given a POI POIFSFileSystem object, and a specific directory
      *  within it, read in its Workbook and populate the high and
@@ -203,6 +239,8 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
             throws IOException
     {
         super(directory, fs);
+        String workbookName = getWorkbookDirEntryName(directory);
+
         this.preserveNodes = preserveNodes;
 
         // If we're not preserving nodes, don't track the
@@ -215,27 +253,8 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
         sheets = new ArrayList(INITIAL_CAPACITY);
         names  = new ArrayList(INITIAL_CAPACITY);
 
-        // Normally, the Workbook will be in a POIFS Stream
-        //  called "Workbook". However, some wierd XLS generators
-        //  put theirs in one called "WORKBOOK"
-        String workbookName = "Workbook";
-        try {
-        	directory.getEntry(workbookName);
-        	// Is the default name
-        } catch(FileNotFoundException fe) {
-        	// Try the upper case form
-        	try {
-        		workbookName = "WORKBOOK";
-        		directory.getEntry(workbookName);
-        	} catch(FileNotFoundException wfe) {
-        		// Doesn't contain it in either form
-        		throw new IllegalArgumentException("The supplied POIFSFileSystem contained neither a 'Workbook' entry, nor a 'WORKBOOK' entry. Is it really an excel file?");
-        	}
-        }
-
-        
         // Grab the data from the workbook stream, however
-        //  it happens to be spelt.
+        //  it happens to be spelled.
         InputStream stream = directory.createDocumentInputStream(workbookName);
 
         EventRecordFactory factory = new EventRecordFactory();
@@ -617,6 +636,8 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
      *
      * @param sheetname     sheetname to set for the sheet.
      * @return HSSFSheet representing the new sheet.
+     * @throws IllegalArgumentException if there is already a sheet present with a case-insensitive
+     *  match for the specified name.
      */
 
     public HSSFSheet createSheet(String sheetname)
@@ -660,9 +681,9 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
     }
 
     /**
-     * Get sheet with the given name
+     * Get sheet with the given name (case insensitive match)
      * @param name of the sheet
-     * @return HSSFSheet with the name provided or null if it does not exist
+     * @return HSSFSheet with the name provided or <code>null</code> if it does not exist
      */
 
     public HSSFSheet getSheet(String name)
@@ -673,7 +694,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
         {
             String sheetname = workbook.getSheetName(k);
 
-            if (sheetname.equals(name))
+            if (sheetname.equalsIgnoreCase(name))
             {
                 retval = (HSSFSheet) sheets.get(k);
             }

src/ooxml/interfaces-jdk14/org/apache/poi/ss/usermodel/Workbook.java

@@ -18,7 +18,7 @@
 package org.apache.poi.ss.usermodel;
 
 import org.apache.poi.hssf.usermodel.*;
-import org.apache.poi.ss.util.SheetReferences;
+import org.apache.poi.hssf.util.SheetReferences;
 
 /**
  * This is a JDK 1.4 compatible interface for HSSFWorkbook.
@@ -38,12 +38,10 @@ public interface Workbook {
     String getSheetName(int sheet);
     HSSFSheet getSheetAt(int index);
     SheetReferences getSheetReferences();
-    short getSheetIndex(String name);
+    int getSheetIndex(String name);
-    short getSheetIndex(Sheet sheet);
+    int getSheetIndex(Sheet sheet);
-    short getSheetIndexFromExternSheetIndex(int externSheetNumber);
+    int getSheetIndexFromExternSheetIndex(int externSheetNumber);
-    short getSheetIndexFromExternSheetIndex(short externSheetNumber);
+	int getExternalSheetIndex(int internalSheetIndex);
-	short getExternalSheetIndex(int internalSheetIndex);
-    short getExternalSheetIndex(short internalSheetIndex);
 
     CreationHelper getCreationHelper();
 }

src/scratchpad/src/org/apache/poi/hslf/model/AutoShapes.java

@@ -369,5 +369,12 @@ public class AutoShapes {
             }
         };
 
+        shapes[ShapeTypes.StraightConnector1] = new ShapeOutline(){
+            public java.awt.Shape getOutline(Shape shape){
+                return new Line2D.Float(0, 0, 21600, 21600);
+            }
+        };
+
+
     }
 }

src/scratchpad/src/org/apache/poi/hslf/model/Freeform.java

@@ -19,6 +19,7 @@ package org.apache.poi.hslf.model;
 import org.apache.poi.ddf.*;
 import org.apache.poi.util.LittleEndian;
 import org.apache.poi.util.POILogger;
+import org.apache.poi.util.HexDump;
 
 import java.awt.geom.*;
 import java.util.ArrayList;
@@ -185,10 +186,6 @@ public class Freeform extends AutoShape {
             return null;
         }
 
-        Rectangle2D bounds = getAnchor2D();
-        float right = (float)bounds.getX();
-        float bottom = (float)bounds.getY();
-
         GeneralPath path = new GeneralPath();
         int numPoints = verticesProp.getNumberOfElementsInArray();
         int numSegments = segmentsProp.getNumberOfElementsInArray();
@@ -199,8 +196,8 @@ public class Freeform extends AutoShape {
                 short x = LittleEndian.getShort(p, 0);
                 short y = LittleEndian.getShort(p, 2);
                 path.moveTo(
-                        ((float)x*POINT_DPI/MASTER_DPI + right),
+                        ((float)x*POINT_DPI/MASTER_DPI),
-                        ((float)y*POINT_DPI/MASTER_DPI + bottom));
+                        ((float)y*POINT_DPI/MASTER_DPI));
             } else if (Arrays.equals(elem, SEGMENTINFO_CUBICTO) || Arrays.equals(elem, SEGMENTINFO_CUBICTO2)){
                 i++;
                 byte[] p1 = verticesProp.getElement(j++);
@@ -213,9 +210,9 @@ public class Freeform extends AutoShape {
                 short x3 = LittleEndian.getShort(p3, 0);
                 short y3 = LittleEndian.getShort(p3, 2);
                 path.curveTo(
-                        ((float)x1*POINT_DPI/MASTER_DPI + right), ((float)y1*POINT_DPI/MASTER_DPI + bottom),
+                        ((float)x1*POINT_DPI/MASTER_DPI), ((float)y1*POINT_DPI/MASTER_DPI),
-                        ((float)x2*POINT_DPI/MASTER_DPI + right), ((float)y2*POINT_DPI/MASTER_DPI + bottom),
+                        ((float)x2*POINT_DPI/MASTER_DPI), ((float)y2*POINT_DPI/MASTER_DPI),
-                        ((float)x3*POINT_DPI/MASTER_DPI + right), ((float)y3*POINT_DPI/MASTER_DPI + bottom));
+                        ((float)x3*POINT_DPI/MASTER_DPI), ((float)y3*POINT_DPI/MASTER_DPI));
 
             } else if (Arrays.equals(elem, SEGMENTINFO_LINETO)){
                 i++;
@@ -226,18 +223,26 @@ public class Freeform extends AutoShape {
                         short x = LittleEndian.getShort(p, 0);
                         short y = LittleEndian.getShort(p, 2);
                         path.lineTo(
-                                ((float)x*POINT_DPI/MASTER_DPI + right), ((float)y*POINT_DPI/MASTER_DPI + bottom));
+                                ((float)x*POINT_DPI/MASTER_DPI), ((float)y*POINT_DPI/MASTER_DPI));
                     }
                 } else if (Arrays.equals(pnext, SEGMENTINFO_CLOSE)){
                     path.closePath();
                 }
             }
         }
-
         return path;
     }
 
     public java.awt.Shape getOutline(){
-        return getPath();
+        GeneralPath path =  getPath();
+        Rectangle2D anchor = getAnchor2D();
+        Rectangle2D bounds = path.getBounds2D();
+        AffineTransform at = new AffineTransform();
+        at.translate(anchor.getX(), anchor.getY());
+        at.scale(
+                anchor.getWidth()/bounds.getWidth(),
+                anchor.getHeight()/bounds.getHeight()
+        );
+        return at.createTransformedShape(path);
     }
 }

src/scratchpad/src/org/apache/poi/hslf/model/MasterSheet.java

@@ -67,4 +67,21 @@ public abstract class MasterSheet extends Sheet {
         }
         return false;
     }
+
+    /**
+     * Return placeholder by text type
+     */
+    public TextShape getPlaceholder(int type){
+        Shape[] shape = getShapes();
+        for (int i = 0; i < shape.length; i++) {
+            if(shape[i] instanceof TextShape){
+                TextShape tx = (TextShape)shape[i];
+                TextRun run = tx.getTextRun();
+                if(run != null && run.getRunType() == type){
+                    return tx;
+                }
+            }
+        }
+        return null;
+    }
 }

src/scratchpad/src/org/apache/poi/hslf/model/Picture.java

@@ -28,6 +28,7 @@ import javax.imageio.ImageIO;
 import java.awt.image.BufferedImage;
 import java.awt.*;
 import java.awt.geom.Rectangle2D;
+import java.awt.geom.AffineTransform;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
@@ -244,6 +245,9 @@ public class Picture extends SimpleShape {
     }
 
     public void draw(Graphics2D graphics){
+        AffineTransform at = graphics.getTransform();
+        ShapePainter.paint(this, graphics);
+
         PictureData data = getPictureData();
         if (data  instanceof Bitmap){
             BufferedImage img = null;
@@ -260,5 +264,6 @@ public class Picture extends SimpleShape {
         } else {
             logger.log(POILogger.WARN, "Rendering of metafiles is not yet supported. image.type: " + (data == null ? "NA" : data.getClass().getName()));
         }
+        graphics.setTransform(at);
     }
 }

src/scratchpad/src/org/apache/poi/hslf/model/Shape.java

@@ -341,58 +341,7 @@ public abstract class Shape {
      * @param sh - owning shape
      */
     protected void afterInsert(Sheet sh){
-        PPDrawing ppdrawing = sh.getPPDrawing();
 
-        EscherContainerRecord dgContainer = (EscherContainerRecord) ppdrawing.getEscherRecords()[0];
-
-        EscherDgRecord dg = (EscherDgRecord) Shape.getEscherChild(dgContainer, EscherDgRecord.RECORD_ID);
-
-        int id = allocateShapeId(dg);
-        setShapeId(id);
-    }
-
-    /**
-     * Allocates new shape id for the new drawing group id.
-     *
-     * @param dg  EscherDgRecord of the sheet that owns the shape being created
-     *
-     * @return a new shape id.
-     */
-    protected int allocateShapeId(EscherDgRecord dg)
-    {
-        EscherDggRecord dgg = _sheet.getSlideShow().getDocumentRecord().getPPDrawingGroup().getEscherDggRecord();
-        if(dgg == null){
-            logger.log(POILogger.ERROR, "EscherDggRecord not found");
-            return 0;
-        }
-
-        dgg.setNumShapesSaved( dgg.getNumShapesSaved() + 1 );
-
-        // Add to existing cluster if space available
-        for (int i = 0; i < dgg.getFileIdClusters().length; i++)
-        {
-            EscherDggRecord.FileIdCluster c = dgg.getFileIdClusters()[i];
-            if (c.getDrawingGroupId() == dg.getDrawingGroupId() && c.getNumShapeIdsUsed() != 1024)
-            {
-                int result = c.getNumShapeIdsUsed() + (1024 * (i+1));
-                c.incrementShapeId();
-                dg.setNumShapes( dg.getNumShapes() + 1 );
-                dg.setLastMSOSPID( result );
-                if (result >= dgg.getShapeIdMax())
-                    dgg.setShapeIdMax( result + 1 );
-                return result;
-            }
-        }
-
-        // Create new cluster
-        dgg.addCluster( dg.getDrawingGroupId(), 0 );
-        dgg.getFileIdClusters()[dgg.getFileIdClusters().length-1].incrementShapeId();
-        dg.setNumShapes( dg.getNumShapes() + 1 );
-        int result = (1024 * dgg.getFileIdClusters().length);
-        dg.setLastMSOSPID( result );
-        if (result >= dgg.getShapeIdMax())
-            dgg.setShapeIdMax( result + 1 );
-        return result;
     }
 
     /**

src/scratchpad/src/org/apache/poi/hslf/model/ShapeGroup.java

@@ -196,13 +196,8 @@ public class ShapeGroup extends Shape{
 
         Sheet sheet = getSheet();
         shape.setSheet(sheet);
+        shape.setShapeId(sheet.allocateShapeId());
         shape.afterInsert(sheet);
-
-        if (shape instanceof TextShape) {
-            TextShape tbox = (TextShape) shape;
-            EscherTextboxWrapper txWrapper = tbox.getEscherTextboxWrapper();
-            if(txWrapper != null) getSheet().getPPDrawing().addTextboxWrapper(txWrapper);
-        }
     }
 
     /**
@@ -277,20 +272,9 @@ public class ShapeGroup extends Shape{
     }
 
     public void draw(Graphics2D graphics){
-        Rectangle2D anchor = getAnchor2D();
-        Rectangle2D coords = getCoordinates();
 
-        //transform coordinates
         AffineTransform at = graphics.getTransform();
-        /*
-        if(!anchor.equals(coords)){
-            graphics.scale(anchor.getWidth()/coords.getWidth(), anchor.getHeight()/coords.getHeight());
 
-            graphics.translate(
-                    anchor.getX()*coords.getWidth()/anchor.getWidth() - coords.getX(),
-                    anchor.getY()*coords.getHeight()/anchor.getHeight() - coords.getY());
-        }
-        */
         Shape[] sh = getShapes();
         for (int i = 0; i < sh.length; i++) {
             sh[i].draw(graphics);

src/scratchpad/src/org/apache/poi/hslf/model/Sheet.java

@@ -18,12 +18,10 @@
 
 package org.apache.poi.hslf.model;
 
-import org.apache.poi.ddf.EscherContainerRecord;
+import org.apache.poi.ddf.*;
-import org.apache.poi.ddf.EscherDgRecord;
-import org.apache.poi.ddf.EscherRecord;
-import org.apache.poi.ddf.EscherSpRecord;
 import org.apache.poi.hslf.record.*;
 import org.apache.poi.hslf.usermodel.SlideShow;
+import org.apache.poi.util.POILogger;
 
 import java.util.ArrayList;
 import java.util.Iterator;
@@ -248,15 +246,47 @@ public abstract class Sheet {
         spgr.addChildRecord(shape.getSpContainer());
 
         shape.setSheet(this);
+        shape.setShapeId(allocateShapeId());
         shape.afterInsert(this);
-
-        // If it's a TextShape, we need to tell the PPDrawing, as it has to
-        //  track TextboxWrappers specially
-        if (shape instanceof TextShape) {
-            TextShape tbox = (TextShape) shape;
-            EscherTextboxWrapper txWrapper = tbox.getEscherTextboxWrapper();
-            if(txWrapper != null) ppdrawing.addTextboxWrapper(txWrapper);
     }
+
+    /**
+     * Allocates new shape id for the new drawing group id.
+     *
+     * @return a new shape id.
+     */
+    public int allocateShapeId()
+    {
+        EscherDggRecord dgg = _slideShow.getDocumentRecord().getPPDrawingGroup().getEscherDggRecord();
+        EscherDgRecord dg = _container.getPPDrawing().getEscherDgRecord();
+
+        dgg.setNumShapesSaved( dgg.getNumShapesSaved() + 1 );
+
+        // Add to existing cluster if space available
+        for (int i = 0; i < dgg.getFileIdClusters().length; i++)
+        {
+            EscherDggRecord.FileIdCluster c = dgg.getFileIdClusters()[i];
+            if (c.getDrawingGroupId() == dg.getDrawingGroupId() && c.getNumShapeIdsUsed() != 1024)
+            {
+                int result = c.getNumShapeIdsUsed() + (1024 * (i+1));
+                c.incrementShapeId();
+                dg.setNumShapes( dg.getNumShapes() + 1 );
+                dg.setLastMSOSPID( result );
+                if (result >= dgg.getShapeIdMax())
+                    dgg.setShapeIdMax( result + 1 );
+                return result;
+            }
+        }
+
+        // Create new cluster
+        dgg.addCluster( dg.getDrawingGroupId(), 0, false );
+        dgg.getFileIdClusters()[dgg.getFileIdClusters().length-1].incrementShapeId();
+        dg.setNumShapes( dg.getNumShapes() + 1 );
+        int result = (1024 * dgg.getFileIdClusters().length);
+        dg.setLastMSOSPID( result );
+        if (result >= dgg.getShapeIdMax())
+            dgg.setShapeIdMax( result + 1 );
+        return result;
     }
 
     /**
@@ -284,6 +314,13 @@ public abstract class Sheet {
         return lst.remove(shape.getSpContainer());
     }
 
+    /**
+     * Called by SlideShow ater a new sheet is created
+     */
+    public void onCreate(){
+
+    }
+
     /**
      * Return the master sheet .
      */

src/scratchpad/src/org/apache/poi/hslf/model/SimpleShape.java

@@ -126,8 +126,8 @@ public class SimpleShape extends Shape {
         EscherSimpleProperty p2 = (EscherSimpleProperty)getEscherProperty(opt, EscherProperties.LINESTYLE__NOLINEDRAWDASH);
         int p2val = p2 == null ? 0 : p2.getPropertyValue();
         Color clr = null;
-        if (p1 != null && (p2val  & 0x8) != 0){
+        if ((p2val  & 0x8) != 0 || (p2val  & 0x10) != 0){
-            int rgb = p1.getPropertyValue();
+            int rgb = p1 == null ? 0 : p1.getPropertyValue();
             if (rgb >= 0x8000000) {
                 int idx = rgb % 0x8000000;
                 if(getSheet() != null) {

src/scratchpad/src/org/apache/poi/hslf/model/Slide.java

@@ -21,12 +21,17 @@
 package org.apache.poi.hslf.model;
 
 import java.util.Vector;
+import java.util.Iterator;
 import java.awt.*;
 
 import org.apache.poi.hslf.record.SlideAtom;
 import org.apache.poi.hslf.record.TextHeaderAtom;
 import org.apache.poi.hslf.record.ColorSchemeAtom;
 import org.apache.poi.hslf.record.SlideListWithText.SlideAtomsSet;
+import org.apache.poi.ddf.EscherDggRecord;
+import org.apache.poi.ddf.EscherContainerRecord;
+import org.apache.poi.ddf.EscherDgRecord;
+import org.apache.poi.ddf.EscherSpRecord;
 
 /**
  * This class represents a slide in a PowerPoint Document. It allows 
@@ -126,6 +131,42 @@ public class Slide extends Sheet
 		_slideNo = newSlideNumber;
 	}
   
+    /**
+     * Called by SlideShow ater a new slide is created.
+     * <p>
+     * For Slide we need to do the following:
+     *  <li> set id of the drawing group.
+     *  <li> set shapeId for the container descriptor and background
+     * </p>
+     */
+    public void onCreate(){
+        //initialize drawing group id
+        EscherDggRecord dgg = getSlideShow().getDocumentRecord().getPPDrawingGroup().getEscherDggRecord();
+        EscherContainerRecord dgContainer = (EscherContainerRecord)getSheetContainer().getPPDrawing().getEscherRecords()[0];
+        EscherDgRecord dg = (EscherDgRecord) Shape.getEscherChild(dgContainer, EscherDgRecord.RECORD_ID);
+        int dgId = dgg.getMaxDrawingGroupId() + 1;
+        dg.setOptions((short)(dgId << 4));
+        dgg.setDrawingsSaved(dgg.getDrawingsSaved() + 1);
+
+        for (Iterator it = dgContainer.getChildContainers().iterator(); it.hasNext(); ) {
+            EscherContainerRecord c = (EscherContainerRecord)it.next();
+            EscherSpRecord spr = null;
+            switch(c.getRecordId()){
+                case EscherContainerRecord.SPGR_CONTAINER:
+                    EscherContainerRecord dc = (EscherContainerRecord)c.getChildRecords().get(0);
+                    spr = dc.getChildById(EscherSpRecord.RECORD_ID);
+                    break;
+                case EscherContainerRecord.SP_CONTAINER:
+                    spr = c.getChildById(EscherSpRecord.RECORD_ID);
+                    break;
+            }
+            if(spr != null) spr.setShapeId(allocateShapeId());
+        }
+
+        //PPT doen't increment the number of saved shapes for group descriptor and background 
+        dg.setNumShapes(1);
+    }
+
 	/**
 	 * Create a <code>TextBox</code> object that represents the slide's title.
 	 *

src/scratchpad/src/org/apache/poi/hslf/model/SlideMaster.java

@@ -92,6 +92,7 @@ public class SlideMaster extends MasterSheet {
             } else {
                 switch (txtype) {
                     case TextHeaderAtom.CENTRE_BODY_TYPE:
+                    case TextHeaderAtom.HALF_BODY_TYPE:
                     case TextHeaderAtom.QUARTER_BODY_TYPE:
                         txtype = TextHeaderAtom.BODY_TYPE;
                         break;

src/scratchpad/src/org/apache/poi/hslf/model/TextPainter.java

@@ -17,6 +17,7 @@
 package org.apache.poi.hslf.model;
 
 import org.apache.poi.hslf.usermodel.RichTextRun;
+import org.apache.poi.hslf.record.TextRulerAtom;
 import org.apache.poi.util.POILogger;
 import org.apache.poi.util.POILogFactory;
 
@@ -38,6 +39,13 @@ import java.util.ArrayList;
 public class TextPainter {
     protected POILogger logger = POILogFactory.getLogger(this.getClass());
 
+    /**
+     * Display unicode square if a bullet char can't be displayed,
+     * for example, if Wingdings font is used.
+     * TODO: map Wingdngs and Symbol to unicode Arial
+     */
+    protected static final char DEFAULT_BULLET_CHAR = '\u25a0';
+
     protected TextShape _shape;
 
     public TextPainter(TextShape shape){
@@ -49,6 +57,10 @@ public class TextPainter {
      */
     public AttributedString getAttributedString(TextRun txrun){
         String text = txrun.getText();
+        //TODO: properly process tabs
+        text = text.replace('\t', ' ');
+        text = text.replace((char)160, ' ');
+
         AttributedString at = new AttributedString(text);
         RichTextRun[] rt = txrun.getRichTextRuns();
         for (int i = 0; i < rt.length; i++) {
@@ -109,7 +121,24 @@ public class TextPainter {
             }
 
             float wrappingWidth = (float)anchor.getWidth() - _shape.getMarginLeft() - _shape.getMarginRight();
-            wrappingWidth -= rt.getTextOffset();
+            int bulletOffset = rt.getBulletOffset();
+            int textOffset = rt.getTextOffset();
+            int indent = rt.getIndentLevel();
+
+            TextRulerAtom ruler = run.getTextRuler();
+            if(ruler != null) {
+                int bullet_val = ruler.getBulletOffsets()[indent]*Shape.POINT_DPI/Shape.MASTER_DPI;
+                int text_val = ruler.getTextOffsets()[indent]*Shape.POINT_DPI/Shape.MASTER_DPI;
+                if(bullet_val > text_val){
+                    int a = bullet_val;
+                    bullet_val = text_val;
+                    text_val = a;
+                }
+                if(bullet_val != 0 ) bulletOffset = bullet_val;
+                if(text_val != 0) textOffset = text_val;
+            }
+
+            wrappingWidth -= textOffset;
 
             if (_shape.getWordWrap() == TextShape.WrapNone) {
                 wrappingWidth = _shape.getSheet().getSlideShow().getPageSize().width;
@@ -141,8 +170,9 @@ public class TextPainter {
             }
 
             el._align = rt.getAlignment();
-            el._text = textLayout;
+            el.advance = textLayout.getAdvance();
-            el._textOffset = rt.getTextOffset();
+            el._textOffset = textOffset;
+            el._text = new AttributedString(it, startIndex, endIndex);
 
             if (prStart){
                 int sp = rt.getSpaceBefore();
@@ -182,13 +212,25 @@ public class TextPainter {
                 Color clr = rt.getBulletColor();
                 if (clr != null) bat.addAttribute(TextAttribute.FOREGROUND, clr);
                 else bat.addAttribute(TextAttribute.FOREGROUND, it.getAttribute(TextAttribute.FOREGROUND));
-                bat.addAttribute(TextAttribute.FAMILY, it.getAttribute(TextAttribute.FAMILY));
-                bat.addAttribute(TextAttribute.SIZE, it.getAttribute(TextAttribute.SIZE));
 
-                TextLayout bulletLayout = new TextLayout(bat.getIterator(), graphics.getFontRenderContext());
+                int fontIdx = rt.getBulletFont();
+                if(fontIdx == -1) fontIdx = rt.getFontIndex();
+                PPFont bulletFont = _shape.getSheet().getSlideShow().getFont(fontIdx);
+                bat.addAttribute(TextAttribute.FAMILY, bulletFont.getFontName());
+
+                int bulletSize = rt.getBulletSize();
+                int fontSize = rt.getFontSize();
+                if(bulletSize != -1) fontSize = Math.round(fontSize*bulletSize*0.01f);
+                bat.addAttribute(TextAttribute.SIZE, new Float(fontSize));
+
+                if(!new Font(bulletFont.getFontName(), Font.PLAIN, 1).canDisplay(rt.getBulletChar())){
+                    bat.addAttribute(TextAttribute.FAMILY, "Arial");
+                    bat = new AttributedString("" + DEFAULT_BULLET_CHAR, bat.getIterator().getAttributes());
+                }
+
                 if(text.substring(startIndex, endIndex).length() > 1){
-                    el._bullet = bulletLayout;
+                    el._bullet = bat;
-                    el._bulletOffset = rt.getBulletOffset();
+                    el._bulletOffset = bulletOffset;
                 }
             }
             lines.add(el);
@@ -225,29 +267,32 @@ public class TextPainter {
                     break;
                 case TextShape.AlignCenter:
                     pen.x = anchor.getX() + _shape.getMarginLeft() +
-                            (anchor.getWidth() - elem._text.getAdvance() - _shape.getMarginLeft() - _shape.getMarginRight()) / 2;
+                            (anchor.getWidth() - elem.advance - _shape.getMarginLeft() - _shape.getMarginRight()) / 2;
                     break;
                 case TextShape.AlignRight:
                     pen.x = anchor.getX() + _shape.getMarginLeft() +
-                            (anchor.getWidth() - elem._text.getAdvance() - _shape.getMarginLeft() - _shape.getMarginRight());
+                            (anchor.getWidth() - elem.advance - _shape.getMarginLeft() - _shape.getMarginRight());
                     break;
             }
             if(elem._bullet != null){
-                elem._bullet.draw(graphics, (float)(pen.x + elem._bulletOffset), (float)pen.y);
+                graphics.drawString(elem._bullet.getIterator(), (float)(pen.x + elem._bulletOffset), (float)pen.y);
+            }
+            AttributedCharacterIterator chIt = elem._text.getIterator();
+            if(chIt.getEndIndex() > chIt.getBeginIndex()) {
+                graphics.drawString(chIt, (float)(pen.x + elem._textOffset), (float)pen.y);
             }
-            elem._text.draw(graphics, (float)(pen.x + elem._textOffset), (float)pen.y);
-
             y0 += elem.descent;
         }
     }
 
 
-    static class TextElement {
+    public static class TextElement {
-        public TextLayout _text;
+        public AttributedString _text;
         public int _textOffset;
-        public TextLayout _bullet;
+        public AttributedString _bullet;
         public int _bulletOffset;
         public int _align;
         public float ascent, descent;
+        public float advance;
     }
 }

src/scratchpad/src/org/apache/poi/hslf/model/TextRun.java

@@ -535,9 +535,13 @@ public class TextRun
 		//  them to \n
 		String text = rawText.replace('\r','\n');
 
-        //0xB acts like cariage return in page titles
+        int type = _headerAtom == null ? 0 : _headerAtom.getTextType();
+        if(type == TextHeaderAtom.TITLE_TYPE || type == TextHeaderAtom.CENTER_TITLE_TYPE){
+            //0xB acts like cariage return in page titles and like blank in the others
             text = text.replace((char) 0x0B, '\n');
-
+        } else {
+            text = text.replace((char) 0x0B, ' ');
+        }
 		return text;
 	}
 
@@ -655,4 +659,11 @@ public class TextRun
         return null;
     }
 
+    public TextRulerAtom getTextRuler(){
+        for (int i = 0; i < _records.length; i++) {
+            if(_records[i] instanceof TextRulerAtom) return (TextRulerAtom)_records[i];
+        }
+        return null;
+
+    }
 }

src/scratchpad/src/org/apache/poi/hslf/model/TextShape.java

@@ -261,9 +261,19 @@ public abstract class TextShape extends SimpleShape {
     public int getVerticalAlignment(){
         EscherOptRecord opt = (EscherOptRecord)getEscherChild(_escherContainer, EscherOptRecord.RECORD_ID);
         EscherSimpleProperty prop = (EscherSimpleProperty)getEscherProperty(opt, EscherProperties.TEXT__ANCHORTEXT);
-        int valign;
+        int valign = TextShape.AnchorTop;
         if (prop == null){
+            /**
+             * If vertical alignment was not found in the shape properties then try to
+             * fetch the master shape and search for the align property there.
+             */
             int type = getTextRun().getRunType();
+            MasterSheet master = getSheet().getMasterSheet();
+            if(master != null){
+                TextShape masterShape = master.getPlaceholder(type);
+                if(masterShape != null) valign = masterShape.getVerticalAlignment();
+            } else {
+                //not found in the master sheet. Use the hardcoded defaults.
                 switch (type){
                      case TextHeaderAtom.TITLE_TYPE:
                      case TextHeaderAtom.CENTER_TITLE_TYPE:
@@ -273,6 +283,7 @@ public abstract class TextShape extends SimpleShape {
                          valign = TextShape.AnchorTop;
                          break;
                  }
+            }
         } else {
             valign = prop.getPropertyValue();
         }

src/scratchpad/src/org/apache/poi/hslf/record/PPDrawing.java

@@ -24,11 +24,13 @@ import org.apache.poi.util.POILogger;
 
 import org.apache.poi.ddf.*;
 import org.apache.poi.hslf.model.ShapeTypes;
+import org.apache.poi.hslf.model.Shape;
 
 import java.io.IOException;
 import java.io.OutputStream;
 import java.util.List;
 import java.util.Vector;
+import java.util.Iterator;
 
 /**
  * These are actually wrappers onto Escher drawings. Make use of
@@ -52,6 +54,8 @@ public class PPDrawing extends RecordAtom
 	private EscherRecord[] childRecords;
 	private EscherTextboxWrapper[] textboxWrappers;
 
+    //cached EscherDgRecord
+    private EscherDgRecord dg;
 
 	/**
 	 * Get access to the underlying Escher Records
@@ -296,4 +300,24 @@ public class PPDrawing extends RecordAtom
 		tw[textboxWrappers.length] = txtbox;
 		textboxWrappers = tw;
 	}
+
+    /**
+     * Return EscherDgRecord which keeps track of the number of shapes and shapeId in this drawing group
+     *
+     * @return EscherDgRecord
+     */
+    public EscherDgRecord getEscherDgRecord(){
+        if(dg == null){
+            EscherContainerRecord dgContainer = (EscherContainerRecord)childRecords[0];
+            for(Iterator it = dgContainer.getChildRecords().iterator(); it.hasNext();){
+                EscherRecord r = (EscherRecord) it.next();
+                if(r instanceof EscherDgRecord){
+                    dg = (EscherDgRecord)r;
+                    break;
+                }
+            }
+        }
+        return dg;
+    }
+
 }

src/scratchpad/src/org/apache/poi/hslf/record/RecordTypes.java

@@ -89,7 +89,7 @@ public class RecordTypes {
     public static final Type TxMasterStyleAtom = new Type(4003,TxMasterStyleAtom.class);
     public static final Type TxCFStyleAtom = new Type(4004,null);
     public static final Type TxPFStyleAtom = new Type(4005,null);
-    public static final Type TextRulerAtom = new Type(4006,null);
+    public static final Type TextRulerAtom = new Type(4006,TextRulerAtom.class);
     public static final Type TextBookmarkAtom = new Type(4007,null);
     public static final Type TextBytesAtom = new Type(4008,TextBytesAtom.class);
     public static final Type TxSIStyleAtom = new Type(4009,null);

src/scratchpad/src/org/apache/poi/hslf/record/StyleTextPropAtom.java

@@ -127,8 +127,8 @@ public class StyleTextPropAtom extends RecordAtom
 				new ParagraphFlagsTextProp(),
                 new TextProp(2, 0x80, "bullet.char"),
 				new TextProp(2, 0x10, "bullet.font"),
-				new TextProp(4, 0x20, "bullet.color"),
                 new TextProp(2, 0x40, "bullet.size"),
+				new TextProp(4, 0x20, "bullet.color"),
                 new AlignmentTextProp(),
                 new TextProp(2, 0x100, "text.offset"),
 				new TextProp(2, 0x200, "para_unknown_2"),

src/scratchpad/src/org/apache/poi/hslf/record/TextRulerAtom.java

@@ -0,0 +1,194 @@
+/* ====================================================================
+   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.hslf.record;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.zip.InflaterInputStream;
+
+import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.POILogger;
+
+/**
+ * Ruler of a text as it differs from the style's ruler settings.
+ *
+ * @author Yegor Kozlov
+ */
+public class TextRulerAtom extends RecordAtom {
+
+    /**
+     * Record header.
+     */
+    private byte[] _header;
+
+    /**
+     * Record data.
+     */
+    private byte[] _data;
+
+    //ruler internals
+    private int defaultTabSize;
+    private int numLevels;
+    private int[] tabStops;
+    private int[] bulletOffsets = new int[5];
+    private int[] textOffsets = new int[5];
+
+    /**
+     * Constructs a new empty ruler atom.
+     */
+    protected TextRulerAtom() {
+        _header = new byte[8];
+        _data = new byte[0];
+
+        LittleEndian.putShort(_header, 2, (short)getRecordType());
+        LittleEndian.putInt(_header, 4, _data.length);
+    }
+
+    /**
+     * Constructs the ruler atom record from its
+     *  source data.
+     *
+     * @param source the source data as a byte array.
+     * @param start the start offset into the byte array.
+     * @param len the length of the slice in the byte array.
+     */
+    protected TextRulerAtom(byte[] source, int start, int len) {
+        // Get the header.
+        _header = new byte[8];
+        System.arraycopy(source,start,_header,0,8);
+
+        // Get the record data.
+        _data = new byte[len-8];
+        System.arraycopy(source,start+8,_data,0,len-8);
+
+        try {
+            read();
+        } catch (Exception e){
+            logger.log(POILogger.ERROR, "Failed to parse TextRulerAtom: " + e.getMessage()); 
+            e.printStackTrace();
+        }
+    }
+
+    /**
+     * Gets the record type.
+     *
+     * @return the record type.
+     */
+    public long getRecordType() {
+        return RecordTypes.TextRulerAtom.typeID;
+    }
+
+    /**
+     * Write the contents of the record back, so it can be written
+     * to disk.
+     *
+     * @param out the output stream to write to.
+     * @throws java.io.IOException if an error occurs.
+     */
+    public void writeOut(OutputStream out) throws IOException {
+        out.write(_header);
+        out.write(_data);
+    }
+
+    /**
+     * Read the record bytes and initialize the internal variables
+     */
+    private void read(){
+        int pos = 0;
+        short mask = LittleEndian.getShort(_data);  pos += 4;
+        short val;
+        int[] bits = {1, 0, 2, 3, 8, 4, 9, 5, 10, 6, 11, 7, 12};
+        for (int i = 0; i < bits.length; i++) {
+            if((mask & 1 << bits[i]) != 0){
+                switch (bits[i]){
+                    case 0:
+                        //defaultTabSize
+                        defaultTabSize = LittleEndian.getShort(_data, pos); pos += 2;
+                        break;
+                    case 1:
+                        //numLevels
+                        numLevels = LittleEndian.getShort(_data, pos); pos += 2;
+                        break;
+                    case 2:
+                        //tabStops
+                        val = LittleEndian.getShort(_data, pos); pos += 2;
+                        tabStops = new int[val*2];
+                        for (int j = 0; j < tabStops.length; j++) {
+                            tabStops[j] = LittleEndian.getUShort(_data, pos); pos += 2;
+                        }
+                        break;
+                    case 3:
+                    case 4:
+                    case 5:
+                    case 6:
+                    case 7:
+                        //bullet.offset
+                        val = LittleEndian.getShort(_data, pos); pos += 2;
+                        bulletOffsets[bits[i]-3] = val;
+                        break;
+                    case 8:
+                    case 9:
+                    case 10:
+                    case 11:
+                    case 12:
+                        //text.offset
+                        val = LittleEndian.getShort(_data, pos); pos += 2;
+                        textOffsets[bits[i]-8] = val;
+                        break;
+                }
+            }
+        }
+    }
+
+    /**
+     * Default distance between tab stops, in master coordinates (576 dpi).
+     */
+    public int getDefaultTabSize(){
+        return defaultTabSize;
+    }
+
+    /**
+     * Number of indent levels (maximum 5).
+     */
+    public int getNumberOfLevels(){
+        return numLevels;
+    }
+
+    /**
+     * Default distance between tab stops, in master coordinates (576 dpi).
+     */
+    public int[] getTabStops(){
+        return tabStops;
+    }
+
+    /**
+     * Paragraph's distance from shape's left margin, in master coordinates (576 dpi).
+     */
+    public int[] getTextOffsets(){
+        return textOffsets;
+    }
+
+    /**
+     * First line of paragraph's distance from shape's left margin, in master coordinates (576 dpi).
+     */
+    public int[] getBulletOffsets(){
+        return bulletOffsets;
+    }
+}

src/scratchpad/src/org/apache/poi/hslf/usermodel/RichTextRun.java

@@ -32,6 +32,8 @@ import org.apache.poi.hslf.model.textproperties.ParagraphFlagsTextProp;
 import org.apache.poi.hslf.model.textproperties.TextProp;
 import org.apache.poi.hslf.model.textproperties.TextPropCollection;
 import org.apache.poi.hslf.record.ColorSchemeAtom;
+import org.apache.poi.util.POILogger;
+import org.apache.poi.util.POILogFactory;
 
 
 /**
@@ -39,6 +41,8 @@ import org.apache.poi.hslf.record.ColorSchemeAtom;
  * 
  */
 public class RichTextRun {
+    protected POILogger logger = POILogFactory.getLogger(this.getClass());
+
 	/** The TextRun we belong to */
 	private TextRun parentRun;
 	/** The SlideShow we belong to */
@@ -199,11 +203,16 @@ public class RichTextRun {
         }
         if (prop == null){
             Sheet sheet = parentRun.getSheet();
+            if(sheet != null){
                 int txtype = parentRun.getRunType();
                 MasterSheet master = sheet.getMasterSheet();
-            if (master != null)
+                if (master != null){
                     prop = (BitMaskTextProp)master.getStyleAttribute(txtype, getIndentLevel(), propname, isCharacter);
                 }
+            } else {
+                logger.log(POILogger.WARN, "MasterSheet is not available");
+            }
+        }
 
         return prop == null ? false : prop.getSubValue(index);
     }
@@ -213,7 +222,7 @@ public class RichTextRun {
 	 *  it if required. 
 	 */
 	private void setCharFlagsTextPropVal(int index, boolean value) {
-        setFlag(true, index, value);
+        if(getFlag(true, index) != value) setFlag(true, index, value);
 	}
 
     public void setFlag(boolean isCharacter, int index, boolean value) {
@@ -281,10 +290,14 @@ public class RichTextRun {
 	 */
 	private int getParaTextPropVal(String propName) {
         TextProp prop = null;
+        boolean hardAttribute = false;
         if (paragraphStyle != null){
             prop = paragraphStyle.findByName(propName);
+
+            BitMaskTextProp maskProp = (BitMaskTextProp)paragraphStyle.findByName(ParagraphFlagsTextProp.NAME);
+            hardAttribute = maskProp != null && maskProp.getValue() == 0;
         }
-        if (prop == null){
+        if (prop == null && !hardAttribute){
             Sheet sheet = parentRun.getSheet();
             int txtype = parentRun.getRunType();
             MasterSheet master = sheet.getMasterSheet();
@@ -574,6 +587,13 @@ public class RichTextRun {
         return getFlag(false, ParagraphFlagsTextProp.BULLET_IDX);
     }
 
+    /**
+     * Returns whether this rich text run has bullets
+     */
+    public boolean isBulletHard() {
+        return getFlag(false, ParagraphFlagsTextProp.BULLET_IDX);
+    }
+
     /**
      * Sets the bullet character
      */

src/scratchpad/src/org/apache/poi/hslf/usermodel/SlideShow.java

@@ -24,10 +24,7 @@ import java.util.*;
 import java.awt.Dimension;
 import java.io.*;
 
-import org.apache.poi.ddf.EscherBSERecord;
+import org.apache.poi.ddf.*;
-import org.apache.poi.ddf.EscherContainerRecord;
-import org.apache.poi.ddf.EscherOptRecord;
-import org.apache.poi.ddf.EscherRecord;
 import org.apache.poi.hslf.*;
 import org.apache.poi.hslf.model.*;
 import org.apache.poi.hslf.model.Notes;
@@ -66,8 +63,6 @@ public class SlideShow
   // Lookup between the PersitPtr "sheet" IDs, and the position
   //  in the mostRecentCoreRecords array
   private Hashtable _sheetIdToCoreRecordsLookup;
-  // Used when adding new core records
-  private int _highestSheetId;
 
   // Records that are interesting
   private Document _documentRecord;
@@ -203,8 +198,6 @@ public class SlideShow
 	for(int i=0; i<allIDs.length; i++) {
 		_sheetIdToCoreRecordsLookup.put(new Integer(allIDs[i]), new Integer(i));
 	}
-	// Capture the ID of the highest sheet
-	_highestSheetId = allIDs[(allIDs.length-1)];
 
 	// Now convert the byte offsets back into record offsets
 	for(int i=0; i<_records.length; i++) {
@@ -616,12 +609,6 @@ public class SlideShow
   		// Set up a new  SlidePersistAtom for this slide
   		SlidePersistAtom sp = new SlidePersistAtom();
 
-  		// Reference is the 1-based index of the slide container in 
-  		//  the PersistPtr root.
-  		// It always starts with 3 (1 is Document, 2 is MainMaster, 3 is 
-  		//  the first slide), but quicksaves etc can leave gaps
-  		_highestSheetId++;
-  		sp.setRefID(_highestSheetId);
   		// First slideId is always 256
   		sp.setSlideIdentifier(prev == null ? 256 : (prev.getSlideIdentifier() + 1));
 
@@ -631,6 +618,9 @@ public class SlideShow
 
   		// Create a new Slide
   		Slide slide = new Slide(sp.getSlideIdentifier(), sp.getRefID(), _slides.length+1);
+        slide.setSlideShow(this);
+        slide.onCreate();
+
   		// Add in to the list of Slides
   		Slide[] s = new Slide[_slides.length+1];
   		System.arraycopy(_slides, 0, s, 0, _slides.length);
@@ -640,10 +630,10 @@ public class SlideShow
 
   		// Add the core records for this new Slide to the record tree
   		org.apache.poi.hslf.record.Slide slideRecord = slide.getSlideRecord();
-  		slideRecord.setSheetId(sp.getRefID());
   		int slideRecordPos = _hslfSlideShow.appendRootLevelRecord(slideRecord);
   		_records = _hslfSlideShow.getRecords();
 
+
   		// Add the new Slide into the PersistPtr stuff
   		int offset = 0;
   		int slideOffset = 0;
@@ -668,18 +658,22 @@ public class SlideShow
   			offset += out.size();
   		}
 
+        // persist ID is UserEditAtom.maxPersistWritten + 1
+        int psrId = usr.getMaxPersistWritten() + 1;
+        sp.setRefID(psrId);
+        slideRecord.setSheetId(psrId);
+
+        // Last view is now of the slide
+        usr.setLastViewType((short)UserEditAtom.LAST_VIEW_SLIDE_VIEW);
+        usr.setMaxPersistWritten(psrId);    //increment the number of persit objects
+
 		// Add the new slide into the last PersistPtr
   		// (Also need to tell it where it is)
 		slideRecord.setLastOnDiskOffset(slideOffset);
 		ptr.addSlideLookup(sp.getRefID(), slideOffset);
 		logger.log(POILogger.INFO, "New slide ended up at " + slideOffset);
 
-		// Last view is now of the slide
-  		usr.setLastViewType((short)UserEditAtom.LAST_VIEW_SLIDE_VIEW);
-  		usr.setMaxPersistWritten(_highestSheetId);
-
   		// All done and added
-  		slide.setSlideShow(this);
   		return slide;
 	}
 

src/scratchpad/testcases/org/apache/poi/hslf/model/TestFreeform.java

@@ -52,7 +52,7 @@ public class TestFreeform extends TestCase {
         Freeform p = new Freeform();
         p.setPath(path1);
 
-        GeneralPath path2 = p.getPath();
+        java.awt.Shape path2 = p.getOutline();
         assertTrue(new Area(path1).equals(new Area(path2)));
     }
 
@@ -63,7 +63,7 @@ public class TestFreeform extends TestCase {
         Freeform p = new Freeform();
         p.setPath(path1);
 
-        GeneralPath path2 = p.getPath();
+        java.awt.Shape path2 = p.getOutline();
         assertTrue(new Area(path1).equals(new Area(path2)));
     }
 
@@ -74,7 +74,7 @@ public class TestFreeform extends TestCase {
         Freeform p = new Freeform();
         p.setPath(path1);
 
-        GeneralPath path2 = p.getPath();
+        java.awt.Shape path2 = p.getOutline();
         assertTrue(new Area(path1).equals(new Area(path2)));
    }
 }

src/scratchpad/testcases/org/apache/poi/hslf/model/TestShapes.java

@@ -20,6 +20,8 @@ import junit.framework.TestCase;
 import org.apache.poi.hslf.usermodel.SlideShow;
 import org.apache.poi.hslf.usermodel.RichTextRun;
 import org.apache.poi.hslf.HSLFSlideShow;
+import org.apache.poi.ddf.EscherDggRecord;
+import org.apache.poi.ddf.EscherDgRecord;
 
 import java.awt.*;
 import java.awt.Rectangle;
@@ -311,18 +313,49 @@ public class TestShapes extends TestCase {
     public void testShapeId() throws IOException {
         SlideShow ppt = new SlideShow();
         Slide slide = ppt.createSlide();
-        Shape shape;
+        Shape shape = null;
 
+        //EscherDgg is a document-level record which keeps track of the drawing groups
+        EscherDggRecord dgg = ppt.getDocumentRecord().getPPDrawingGroup().getEscherDggRecord();
+        EscherDgRecord dg = slide.getSheetContainer().getPPDrawing().getEscherDgRecord();
+
+        int dggShapesUsed = dgg.getNumShapesSaved();   //total number of shapes in the ppt
+        int dggMaxId = dgg.getShapeIdMax();            //max number of shapeId
+
+        int dgMaxId = dg.getLastMSOSPID();   //max shapeId in the slide
+        int dgShapesUsed = dg.getNumShapes();          // number of shapes in the slide
+        //insert 3 shapes and make sure the Ids are properly incremented
+        for (int i = 0; i < 3; i++) {
             shape = new Line();
             assertEquals(0, shape.getShapeId());
             slide.addShape(shape);
             assertTrue(shape.getShapeId() > 0);
 
-        int shapeId = shape.getShapeId();
+            //check that EscherDgRecord is updated
+            assertEquals(shape.getShapeId(), dg.getLastMSOSPID());
+            assertEquals(dgMaxId + 1, dg.getLastMSOSPID());
+            assertEquals(dgShapesUsed + 1, dg.getNumShapes());
 
+            //check that EscherDggRecord is updated
+            assertEquals(shape.getShapeId() + 1, dgg.getShapeIdMax());
+            assertEquals(dggMaxId + 1, dgg.getShapeIdMax());
+            assertEquals(dggShapesUsed + 1, dgg.getNumShapesSaved());
+
+            dggShapesUsed = dgg.getNumShapesSaved();
+            dggMaxId = dgg.getShapeIdMax();
+            dgMaxId = dg.getLastMSOSPID();
+            dgShapesUsed = dg.getNumShapes();
+        }
+
+
+        //For each drawing group PPT allocates clusters with size=1024
+        //if the number of shapes is greater that 1024 a new cluster is allocated
+        //make sure it is so
+        int numClusters = dgg.getNumIdClusters();
+        for (int i = 0; i < 1025; i++) {
             shape = new Line();
-        assertEquals(0, shape.getShapeId());
             slide.addShape(shape);
-        assertEquals(shapeId + 1, shape.getShapeId());
+        }
+        assertEquals(numClusters + 1, dgg.getNumIdClusters());
     }
 }

src/scratchpad/testcases/org/apache/poi/hslf/record/TestTextRulerAtom.java

@@ -0,0 +1,76 @@
+
+/* ====================================================================
+   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.hslf.record;
+
+import org.apache.poi.hslf.HSLFSlideShow;
+import org.apache.poi.hslf.model.textproperties.CharFlagsTextProp;
+import org.apache.poi.hslf.model.textproperties.TextProp;
+import org.apache.poi.hslf.model.textproperties.TextPropCollection;
+import org.apache.poi.hslf.record.StyleTextPropAtom.*;
+import org.apache.poi.hslf.usermodel.SlideShow;
+import org.apache.poi.util.HexDump;
+
+import junit.framework.TestCase;
+import java.io.ByteArrayOutputStream;
+import java.util.LinkedList;
+import java.util.Arrays;
+
+/**
+ * Tests TextRulerAtom
+ *
+ * @author Yegor Kozlov
+ */
+public class TestTextRulerAtom extends TestCase {
+
+    //from a real file
+	private byte[] data_1 = new byte[] {
+		0x00, 0x00, (byte)0xA6, 0x0F, 0x18, 0x00, 0x00, 0x00,
+        (byte)0xF8, 0x1F, 0x00, 0x00, 0x75, 0x00, (byte)0xE2, 0x00, 0x59,
+        0x01, (byte)0xC3, 0x01, 0x1A, 0x03, (byte)0x87, 0x03, (byte)0xF8,
+        0x03, 0x69, 0x04, (byte)0xF6, 0x05, (byte)0xF6, 0x05
+	};
+
+
+    public void testReadRuler() throws Exception {
+		TextRulerAtom ruler = new TextRulerAtom(data_1, 0, data_1.length);
+        assertEquals(ruler.getNumberOfLevels(), 0);
+        assertEquals(ruler.getDefaultTabSize(), 0);
+
+        int[] tabStops = ruler.getTabStops();
+        assertNull(tabStops);
+
+        int[] textOffsets = ruler.getTextOffsets();
+        assertTrue(Arrays.equals(new int[]{226, 451, 903, 1129, 1526}, textOffsets));
+
+        int[] bulletOffsets = ruler.getBulletOffsets();
+        assertTrue(Arrays.equals(new int[]{117, 345, 794, 1016, 1526}, bulletOffsets));
+
+	}
+
+    public void testWriteRuler() throws Exception {
+		TextRulerAtom ruler = new TextRulerAtom(data_1, 0, data_1.length);
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        ruler.writeOut(out);
+
+        byte[] result = out.toByteArray();
+        assertTrue(Arrays.equals(result, data_1));
+	}
+}

src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestRichTextRun.java

@@ -90,8 +90,10 @@ public class TestRichTextRun extends TestCase {
 		
 		// Now set it to not bold
 		rtr.setBold(false);
-		assertNotNull(rtr._getRawCharacterStyle());
+		//setting bold=false doesn't change the internal state
-		assertNotNull(rtr._getRawParagraphStyle());
+        assertNull(rtr._getRawCharacterStyle());
+		assertNull(rtr._getRawParagraphStyle());
+
         assertFalse(rtr.isBold());
 		
 		// And now make it bold

src/testcases/org/apache/poi/ddf/AllPOIDDFTests.java

@@ -42,6 +42,7 @@ public final class AllPOIDDFTests {
         result.addTestSuite(TestEscherSplitMenuColorsRecord.class);
         result.addTestSuite(TestEscherSpRecord.class);
         result.addTestSuite(TestUnknownEscherRecord.class);
+        result.addTestSuite(TestEscherBlipRecord.class);
         return result;
     }
 }

src/testcases/org/apache/poi/ddf/TestEscherBlipRecord.java

@@ -0,0 +1,156 @@
+
+/* ====================================================================
+   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.ddf;
+
+import junit.framework.TestCase;
+import org.apache.poi.util.HexRead;
+import org.apache.poi.util.HexDump;
+
+import java.io.IOException;
+import java.io.File;
+import java.io.FileInputStream;
+import java.util.Iterator;
+import java.util.Arrays;
+
+/**
+ * Test read/serialize of escher blip records
+ *
+ * @author Yegor Kozlov
+ */
+public class TestEscherBlipRecord extends TestCase
+{
+    protected String cwd = System.getProperty("DDF.testdata.path");
+
+    //test reading/serializing of a PNG blip
+    public void testReadPNG() throws IOException {
+        //provided in bug-44886
+        byte[] data = read(new File(cwd, "Container.dat"));
+
+        EscherContainerRecord record = new EscherContainerRecord();
+        record.fillFields(data, 0, new DefaultEscherRecordFactory());
+        EscherContainerRecord bstore = (EscherContainerRecord)record.getChildRecords().get(1);
+        EscherBSERecord bse1 = (EscherBSERecord)bstore.getChildRecords().get(0);
+        assertEquals(EscherBSERecord.BT_PNG, bse1.getBlipTypeWin32());
+        assertEquals(EscherBSERecord.BT_PNG, bse1.getBlipTypeMacOS());
+        assertTrue(Arrays.equals(new byte[]{
+            0x65, 0x07, 0x4A, (byte)0x8D, 0x3E, 0x42, (byte)0x8B, (byte)0xAC,
+            0x1D, (byte)0x89, 0x35, 0x4F, 0x48, (byte)0xFA, 0x37, (byte)0xC2
+        }, bse1.getUid()));
+        assertEquals(255, bse1.getTag());
+        assertEquals(32308, bse1.getSize());
+
+        EscherBitmapBlip blip1 = (EscherBitmapBlip)bse1.getBlipRecord();
+        assertEquals(0x6E00, blip1.getOptions());
+        assertEquals(EscherBitmapBlip.RECORD_ID_PNG, blip1.getRecordId());
+        assertTrue(Arrays.equals(new byte[]{
+            0x65, 0x07, 0x4A, (byte)0x8D, 0x3E, 0x42, (byte)0x8B, (byte)0xAC,
+            0x1D, (byte)0x89, 0x35, 0x4F, 0x48, (byte)0xFA, 0x37, (byte)0xC2
+        }, blip1.getUID()));
+
+        //serialize and read again
+        byte[] ser = bse1.serialize();
+        EscherBSERecord bse2 = new EscherBSERecord();
+        bse2.fillFields(ser, 0, new DefaultEscherRecordFactory());
+        assertEquals(bse1.getRecordId(), bse2.getRecordId());
+        assertEquals(bse1.getBlipTypeWin32(), bse2.getBlipTypeWin32());
+        assertEquals(bse1.getBlipTypeMacOS(), bse2.getBlipTypeMacOS());
+        assertTrue(Arrays.equals(bse1.getUid(), bse2.getUid()));
+        assertEquals(bse1.getTag(), bse2.getTag());
+        assertEquals(bse1.getSize(), bse2.getSize());
+
+        EscherBitmapBlip blip2 = (EscherBitmapBlip)bse1.getBlipRecord();
+        assertEquals(blip1.getOptions(), blip2.getOptions());
+        assertEquals(blip1.getRecordId(), blip2.getRecordId());
+        assertEquals(blip1.getUID(), blip2.getUID());
+
+        assertTrue(Arrays.equals(blip1.getPicturedata(), blip1.getPicturedata()));
+    }
+
+    //test reading/serializing of a PICT metafile
+    public void testReadPICT() throws IOException {
+        //provided in bug-44886
+        byte[] data = read(new File(cwd, "Container.dat"));
+
+        EscherContainerRecord record = new EscherContainerRecord();
+        record.fillFields(data, 0, new DefaultEscherRecordFactory());
+        EscherContainerRecord bstore = (EscherContainerRecord)record.getChildRecords().get(1);
+        EscherBSERecord bse1 = (EscherBSERecord)bstore.getChildRecords().get(1);
+        //System.out.println(bse1);
+        assertEquals(EscherBSERecord.BT_WMF, bse1.getBlipTypeWin32());
+        assertEquals(EscherBSERecord.BT_PICT, bse1.getBlipTypeMacOS());
+        assertTrue(Arrays.equals(new byte[]{
+            (byte)0xC7, 0x15, 0x69, 0x2D, (byte)0xE5, (byte)0x89, (byte)0xA3, 0x6F,
+            0x66, 0x03, (byte)0xD6, 0x24, (byte)0xF7, (byte)0xDB, 0x1D, 0x13
+        }, bse1.getUid()));
+        assertEquals(255, bse1.getTag());
+        assertEquals(1133, bse1.getSize());
+
+        EscherMetafileBlip blip1 = (EscherMetafileBlip)bse1.getBlipRecord();
+        assertEquals(0x5430, blip1.getOptions());
+        assertEquals(EscherMetafileBlip.RECORD_ID_PICT, blip1.getRecordId());
+        assertTrue(Arrays.equals(new byte[]{
+            0x57, 0x32, 0x7B, (byte)0x91, 0x23, 0x5D, (byte)0xDB, 0x36,
+            0x7A, (byte)0xDB, (byte)0xFF, 0x17, (byte)0xFE, (byte)0xF3, (byte)0xA7, 0x05
+        }, blip1.getUID()));
+        assertTrue(Arrays.equals(new byte[]{
+            (byte)0xC7, 0x15, 0x69, 0x2D, (byte)0xE5, (byte)0x89, (byte)0xA3, 0x6F,
+            0x66, 0x03, (byte)0xD6, 0x24, (byte)0xF7, (byte)0xDB, 0x1D, 0x13
+        }, blip1.getPrimaryUID()));
+
+        //serialize and read again
+        byte[] ser = bse1.serialize();
+        EscherBSERecord bse2 = new EscherBSERecord();
+        bse2.fillFields(ser, 0, new DefaultEscherRecordFactory());
+        assertEquals(bse1.getRecordId(), bse2.getRecordId());
+        assertEquals(bse1.getOptions(), bse2.getOptions());
+        assertEquals(bse1.getBlipTypeWin32(), bse2.getBlipTypeWin32());
+        assertEquals(bse1.getBlipTypeMacOS(), bse2.getBlipTypeMacOS());
+        assertTrue(Arrays.equals(bse1.getUid(), bse2.getUid()));
+        assertEquals(bse1.getTag(), bse2.getTag());
+        assertEquals(bse1.getSize(), bse2.getSize());
+
+        EscherMetafileBlip blip2 = (EscherMetafileBlip)bse1.getBlipRecord();
+        assertEquals(blip1.getOptions(), blip2.getOptions());
+        assertEquals(blip1.getRecordId(), blip2.getRecordId());
+        assertEquals(blip1.getUID(), blip2.getUID());
+        assertEquals(blip1.getPrimaryUID(), blip2.getPrimaryUID());
+
+        assertTrue(Arrays.equals(blip1.getPicturedata(), blip1.getPicturedata()));
+    }
+
+    //integral test: check that the read-write-read round trip is consistent
+    public void testContainer() throws IOException {
+        byte[] data = read(new File(cwd, "Container.dat"));
+
+        EscherContainerRecord record = new EscherContainerRecord();
+        record.fillFields(data, 0, new DefaultEscherRecordFactory());
+
+        byte[] ser = record.serialize();
+        assertTrue(Arrays.equals(data, ser));
+    }
+
+    private byte[] read(File file) throws IOException {
+        byte[] data = new byte[(int)file.length()];
+        FileInputStream is = new FileInputStream(file);
+        is.read(data);
+        is.close();
+        return data;
+    }
+
+}

src/testcases/org/apache/poi/ddf/TestEscherContainerRecord.java

@@ -18,12 +18,23 @@
         
 package org.apache.poi.ddf;
 
+import java.io.File;
+import java.io.FileInputStream;
+
 import junit.framework.TestCase;
 import org.apache.poi.util.HexRead;
 import org.apache.poi.util.HexDump;
+import org.apache.poi.util.IOUtils;
 
 public class TestEscherContainerRecord extends TestCase
 {
+	private String ESCHER_DATA_PATH;
+	
+	protected void setUp() throws Exception {
+		super.setUp();
+		ESCHER_DATA_PATH = System.getProperty("DDF.testdata.path");
+	}
+
 	public void testFillFields() throws Exception
     {
         EscherRecordFactory f = new DefaultEscherRecordFactory();
@@ -137,4 +148,19 @@ public class TestEscherContainerRecord extends TestCase
         assertEquals(18, r.getRecordSize());
     }
 
+    /**
+     * We were having problems with reading too much data on an UnknownEscherRecord,
+     *  but hopefully we now read the correct size.
+     */
+    public void testBug44857() throws Exception {
+    	File f = new File(ESCHER_DATA_PATH, "Container.dat");
+    	assertTrue(f.exists());
+    	
+    	FileInputStream finp = new FileInputStream(f);
+    	byte[] data = IOUtils.toByteArray(finp);
+    	
+    	// This used to fail with an OutOfMemory
+    	EscherContainerRecord record = new EscherContainerRecord();
+    	record.fillFields(data, 0, new DefaultEscherRecordFactory());
+    }
 }

src/testcases/org/apache/poi/hssf/HSSFTestDataSamples.java

@@ -172,4 +172,28 @@ public final class HSSFTestDataSamples {
 			throw new RuntimeException(e);
 		}
 	}
+
+	/**
+	 * @return byte array of sample file content from file found in standard hssf test data dir 
+	 */
+	public static byte[] getTestDataFileContent(String fileName) {
+		ByteArrayOutputStream bos = new ByteArrayOutputStream();
+
+		try {
+			InputStream fis = HSSFTestDataSamples.openSampleFileStream(fileName);
+
+			byte[] buf = new byte[512];
+			while (true) {
+				int bytesRead = fis.read(buf);
+				if (bytesRead < 1) {
+					break;
+				}
+				bos.write(buf, 0, bytesRead);
+			}
+			fis.close();
+		} catch (IOException e) {
+			throw new RuntimeException(e);
+		}
+		return bos.toByteArray();
+	}
 }

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

@@ -65,6 +65,7 @@ public final class TestFormulaParser extends TestCase {
      * @return parsed token array already confirmed not <code>null</code>
      */
     private static Ptg[] parseFormula(String s) {
+    	// TODO - replace multiple copies of this code with calls to this method
         FormulaParser fp = new FormulaParser(s, null);
         fp.parse();
         Ptg[] result = fp.getRPNPtg();
@@ -86,7 +87,6 @@ public final class TestFormulaParser extends TestCase {
         assertTrue("",(ptgs[0] instanceof IntPtg));
         assertTrue("",(ptgs[1] instanceof IntPtg));
         assertTrue("",(ptgs[2] instanceof AddPtg));
-
     }
 
     public void testFormulaWithSpace2() {
@@ -169,8 +169,6 @@ public final class TestFormulaParser extends TestCase {
 		assertEquals("If FALSE offset", (short)7, ifPtg.getData());
 		
 		FuncVarPtg funcPtg = (FuncVarPtg)asts[8];
-		
-		
 	}
 
 	/**
@@ -190,8 +188,6 @@ public final class TestFormulaParser extends TestCase {
 		assertTrue("It is not an if", ifFunc.isOptimizedIf());
 		
 		assertTrue("Average Function set correctly", (asts[5] instanceof FuncVarPtg));
-		
-				    	
 	}
 	
 	public void testIfSingleCondition(){
@@ -213,8 +209,6 @@ public final class TestFormulaParser extends TestCase {
 		assertTrue("Ptg is not a Variable Function", (asts[6] instanceof FuncVarPtg));
 		FuncVarPtg funcPtg = (FuncVarPtg)asts[6];
 		assertEquals("Arguments", 2, funcPtg.getNumberOfOperands());
-		
-		
 	}
 
 	public void testSumIf() {
@@ -223,7 +217,6 @@ public final class TestFormulaParser extends TestCase {
 		fp.parse();
 		Ptg[] asts = fp.getRPNPtg();
 		assertEquals("4 Ptgs expected", 4, asts.length);
-		
 	}
 	
 	/**
@@ -235,51 +228,35 @@ public final class TestFormulaParser extends TestCase {
 		String currencyCell = "F3";
 		String function="\"TOTAL[\"&"+currencyCell+"&\"]\"";
 
-		FormulaParser fp = new FormulaParser(function, null);
+		Ptg[] asts = parseFormula(function);
-		fp.parse();
-		Ptg[] asts = fp.getRPNPtg();
 		assertEquals("5 ptgs expected", 5, asts.length);
 		assertTrue ("Ptg[0] is a string", (asts[0] instanceof StringPtg));
 		StringPtg firstString = (StringPtg)asts[0];		
 		
 		assertEquals("TOTAL[", firstString.getValue());
 		//the PTG order isn't 100% correct but it still works - dmui
-		
-					
 	}
 
 	public void testSimpleLogical() {
-		FormulaParser fp=new FormulaParser("IF(A1<A2,B1,B2)",null);
+      Ptg[] ptgs = parseFormula("IF(A1<A2,B1,B2)");
-		fp.parse();
-      Ptg[] ptgs = fp.getRPNPtg();
-      assertTrue("Ptg array should not be null", ptgs !=null);
       assertEquals("Ptg array length", 9, ptgs.length);
-      assertEquals("3rd Ptg is less than",LessThanPtg.class,ptgs[2].getClass());
+      assertEquals("3rd Ptg is less than", LessThanPtg.class, ptgs[2].getClass());
-
-
 	}
 	
 	public void testParenIf() {
-		FormulaParser fp=new FormulaParser("IF((A1+A2)<=3,\"yes\",\"no\")",null);
+		Ptg[] ptgs = parseFormula("IF((A1+A2)<=3,\"yes\",\"no\")");
-		fp.parse();
-		Ptg[] ptgs = fp.getRPNPtg();
-		assertTrue("Ptg array should not be null", ptgs !=null);
 		assertEquals("Ptg array length", 12, ptgs.length);
 		assertEquals("6th Ptg is less than equal",LessEqualPtg.class,ptgs[5].getClass());
 		assertEquals("11th Ptg is not a goto (Attr) ptg",AttrPtg.class,ptgs[10].getClass());
 	}
 	
 	public void testEmbeddedIf() {
-		FormulaParser fp=new FormulaParser("IF(3>=1,\"*\",IF(4<>1,\"first\",\"second\"))",null);
+		Ptg[] ptgs = parseFormula("IF(3>=1,\"*\",IF(4<>1,\"first\",\"second\"))");
-		fp.parse();
-		Ptg[] ptgs = fp.getRPNPtg();
-		assertTrue("Ptg array should not be null", ptgs !=null);
 		assertEquals("Ptg array length", 17, ptgs.length);
 		
 		assertEquals("6th Ptg is not a goto (Attr) ptg",AttrPtg.class,ptgs[5].getClass());
 		assertEquals("9th Ptg is not a not equal ptg",NotEqualPtg.class,ptgs[8].getClass());
 		assertEquals("15th Ptg is not the inner IF variable function ptg",FuncVarPtg.class,ptgs[14].getClass());
-		
 	}
 	
     public void testMacroFunction() {
@@ -302,15 +279,14 @@ public final class TestFormulaParser extends TestCase {
         Ptg[] ptg = fp.getRPNPtg();
         assertTrue("first ptg is string",ptg[0] instanceof StringPtg);
         assertTrue("second ptg is string",ptg[1] instanceof StringPtg);
-
     }
 
-    public void testConcatenate(){
+    public void testConcatenate() {
-         FormulaParser fp = new FormulaParser("CONCATENATE(\"first\",\"second\")",null);
+		FormulaParser fp = new FormulaParser("CONCATENATE(\"first\",\"second\")", null);
 		fp.parse();
 		Ptg[] ptg = fp.getRPNPtg();
-        assertTrue("first ptg is string",ptg[0] instanceof StringPtg);
+		assertTrue("first ptg is string", ptg[0] instanceof StringPtg);
-        assertTrue("second ptg is string",ptg[1] instanceof StringPtg);
+		assertTrue("second ptg is string", ptg[1] instanceof StringPtg);
 	}
 
     public void testWorksheetReferences()
@@ -398,13 +374,13 @@ public final class TestFormulaParser extends TestCase {
 		FormulaParser fp = new FormulaParser("40000/2", null);
 		fp.parse();
 		Ptg[] ptgs = fp.getRPNPtg();
-		        assertTrue("three tokens expected, got "+ptgs.length,ptgs.length == 3);
+		assertTrue("three tokens expected, got " + ptgs.length, ptgs.length == 3);
-		        assertTrue("IntPtg",(ptgs[0] instanceof IntPtg));
+		assertTrue("IntPtg", (ptgs[0] instanceof IntPtg));
-		        assertTrue("IntPtg",(ptgs[1] instanceof IntPtg));
+		assertTrue("IntPtg", (ptgs[1] instanceof IntPtg));
-		        assertTrue("DividePtg",(ptgs[2] instanceof DividePtg));
+		assertTrue("DividePtg", (ptgs[2] instanceof DividePtg));
 	}
 	
-	/** bug 35027, underscore in sheet name*/
+	/** bug 35027, underscore in sheet name */
 	public void testUnderscore() {
 		HSSFWorkbook wb = new HSSFWorkbook();
     	
@@ -592,7 +568,7 @@ public final class TestFormulaParser extends TestCase {
     	HSSFWorkbook book = new HSSFWorkbook();
 
         Ptg[] ptgs = {
-                new FuncPtg(10, 0),
+                new FuncPtg(10),
         };
         assertEquals("NA()", FormulaParser.toFormulaString(book, ptgs));
     }
@@ -775,8 +751,34 @@ public final class TestFormulaParser extends TestCase {
         StringPtg sp = (StringPtg) parseSingleToken(formula, StringPtg.class);
         assertEquals(expectedValue, sp.getValue());
     }
+    public void testParseStringLiterals_bug28754() {
 
-    public void testPaseStringLiterals() {
+        StringPtg sp;
+        try {
+            sp = (StringPtg) parseSingleToken("\"test\"\"ing\"", StringPtg.class);
+        } catch (RuntimeException e) {
+            if(e.getMessage().startsWith("Cannot Parse")) {
+                throw new AssertionFailedError("Identified bug 28754a");
+            }
+            throw e;
+        }
+        assertEquals("test\"ing", sp.getValue());
+
+        HSSFWorkbook wb = new HSSFWorkbook();
+        HSSFSheet sheet = wb.createSheet();
+        wb.setSheetName(0, "Sheet1");
+
+        HSSFRow row = sheet.createRow(0);
+        HSSFCell cell = row.createCell((short)0);
+        cell.setCellFormula("right(\"test\"\"ing\", 3)");
+        String actualCellFormula = cell.getCellFormula();
+        if("RIGHT(\"test\"ing\",3)".equals(actualCellFormula)) {
+            throw new AssertionFailedError("Identified bug 28754b");
+        }
+        assertEquals("RIGHT(\"test\"\"ing\",3)", actualCellFormula);
+    }
+
+    public void testParseStringLiterals() {
         confirmStringParse("goto considered harmful");
 
         confirmStringParse("goto 'considered' harmful");
@@ -810,11 +812,9 @@ public final class TestFormulaParser extends TestCase {
         parseExpectedException("#DIV/ 0+2");
 
 
-        if (false) { // TODO - add functionality to detect func arg count mismatch
         parseExpectedException("IF(TRUE)");
         parseExpectedException("countif(A1:B5, C1, D1)");
     }
-    }
 
     private static void parseExpectedException(String formula) {
         try {
@@ -887,6 +887,14 @@ public final class TestFormulaParser extends TestCase {
             assertTrue(e.getMessage().startsWith("Too few arguments suppled to operation token"));
         }
     }
+    /**
+     * Make sure that POI uses the right Func Ptg when encoding formulas.  Functions with variable
+     * number of args should get FuncVarPtg, functions with fixed args should get FuncPtg.<p/>
+     * 
+     * Prior to the fix for bug 44675 POI would encode FuncVarPtg for all functions.  In many cases
+     * Excel tolerates the wrong Ptg and evaluates the formula OK (e.g. SIN), but in some cases 
+     * (e.g. COUNTIF) Excel fails to evaluate the formula, giving '#VALUE!' instead. 
+     */
     public void testFuncPtgSelection() {
         HSSFWorkbook book = new HSSFWorkbook();
         Ptg[] ptgs;
@@ -900,4 +908,21 @@ public final class TestFormulaParser extends TestCase {
         assertEquals(2, ptgs.length);
         assertEquals(FuncPtg.class, ptgs[1].getClass());
     }
+    
+    public void testWrongNumberOfFunctionArgs() {
+        confirmArgCountMsg("sin()", "Too few arguments to function 'SIN'. Expected 1 but got 0.");
+        confirmArgCountMsg("countif(1, 2, 3, 4)", "Too many arguments to function 'COUNTIF'. Expected 2 but got 4.");
+        confirmArgCountMsg("index(1, 2, 3, 4, 5, 6)", "Too many arguments to function 'INDEX'. At most 4 were expected but got 6.");
+        confirmArgCountMsg("vlookup(1, 2)", "Too few arguments to function 'VLOOKUP'. At least 3 were expected but got 2.");
+    }
+
+    private static void confirmArgCountMsg(String formula, String expectedMessage) {
+        HSSFWorkbook book = new HSSFWorkbook();
+        try {
+            FormulaParser.parse(formula, book);
+            throw new AssertionFailedError("Didn't get parse exception as expected");
+        } catch (FormulaParseException e) {
+            assertEquals(expectedMessage, e.getMessage());
+        }
+    }
 }

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

@@ -67,6 +67,7 @@ public final class AllRecordTests {
 		result.addTestSuite(TestFormulaRecord.class);
 		result.addTestSuite(TestFrameRecord.class);
 		result.addTestSuite(TestHyperlinkRecord.class);
+		result.addTestSuite(TestLabelRecord.class);
 		result.addTestSuite(TestLegendRecord.class);
 		result.addTestSuite(TestLineFormatRecord.class);
 		result.addTestSuite(TestLinkedDataRecord.class);

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

@@ -0,0 +1,41 @@
+/* ====================================================================
+   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 org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+/**
+ * Tests for <tt>LabelRecord</tt>
+ * 
+ * @author Josh Micich
+ */
+public final class TestLabelRecord extends TestCase {
+
+	public void testEmptyString() {
+		HSSFWorkbook wb;
+		try {
+			wb = HSSFTestDataSamples.openSampleWorkbook("ex42570-20305.xls");
+		} catch (NullPointerException e) {
+			throw new AssertionFailedError("Identified bug 42570");
+		}
+		HSSFTestDataSamples.writeOutAndReadBack(wb);
+	}
+}

src/testcases/org/apache/poi/hssf/record/formula/AllFormulaTests.java

@@ -40,9 +40,11 @@ public final class AllFormulaTests {
 		result.addTestSuite(TestArea3DPtg.class);
 		result.addTestSuite(TestAreaErrPtg.class);
 		result.addTestSuite(TestAreaPtg.class);
+		result.addTestSuite(TestArrayPtg.class);
 		result.addTestSuite(TestErrPtg.class);
 		result.addTestSuite(TestExternalFunctionFormulas.class);
 		result.addTestSuite(TestFuncPtg.class);
+		result.addTestSuite(TestFuncVarPtg.class);
 		result.addTestSuite(TestIntersectionPtg.class);
 		result.addTestSuite(TestPercentPtg.class);
 		result.addTestSuite(TestRangePtg.class);

src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java

@@ -0,0 +1,95 @@
+/* ====================================================================
+   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.formula;
+
+import java.util.Arrays;
+
+import org.apache.poi.hssf.record.TestcaseRecordInputStream;
+import org.apache.poi.hssf.record.UnicodeString;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+/**
+ * Tests for <tt>ArrayPtg</tt>
+ * 
+ * @author Josh Micich
+ */
+public final class TestArrayPtg extends TestCase {
+
+	private static final byte[] ENCODED_PTG_DATA = {
+		0x40, 0x00,
+		0x08, 0x00,
+		0, 0, 0, 0, 0, 0, 0, 0, 
+	};
+	private static final byte[] ENCODED_CONSTANT_DATA = {
+		2,    // 3 columns
+		1, 0, // 2 rows
+		4, 1, 0, 0, 0, 0, 0, 0, 0, // TRUE
+		2, 4, 0, 0, 65, 66, 67, 68, // "ABCD"
+		2, 1, 0, 0, 69, // "E"
+		1, 0, 0, 0, 0, 0, 0, 0, 0, // 0
+		4, 0, 0, 0, 0, 0, 0, 0, 0, // FALSE
+		2, 2, 0, 0, 70, 71, // "FG"
+	};
+
+	/**
+	 * Lots of problems with ArrayPtg's encoding of 
+	 */
+	public void testReadWriteTokenValueBytes() {
+		
+		ArrayPtg ptg = new ArrayPtgV(new TestcaseRecordInputStream(ArrayPtgV.sid, ENCODED_PTG_DATA));
+		
+		ptg.readTokenValues(new TestcaseRecordInputStream(0, ENCODED_CONSTANT_DATA));
+		assertEquals(3, ptg.getColumnCount());
+		assertEquals(2, ptg.getRowCount());
+		Object[] values = ptg.token_3_arrayValues;
+		assertEquals(6, values.length);
+		
+		
+		assertEquals(Boolean.TRUE, values[0]);
+		assertEquals(new UnicodeString("ABCD"), values[1]);
+		assertEquals(new Double(0), values[3]);
+		assertEquals(Boolean.FALSE, values[4]);
+		assertEquals(new UnicodeString("FG"), values[5]);
+		
+		byte[] outBuf = new byte[ENCODED_CONSTANT_DATA.length];
+		ptg.writeTokenValueBytes(outBuf, 0);
+		
+		if(outBuf[0] == 4) {
+			throw new AssertionFailedError("Identified bug 42564b");
+		}
+		assertTrue(Arrays.equals(ENCODED_CONSTANT_DATA, outBuf));
+	}
+
+	/**
+	 * make sure constant elements are stored row by row 
+	 */
+	public void testElementOrdering() {
+		ArrayPtg ptg = new ArrayPtgV(new TestcaseRecordInputStream(ArrayPtgV.sid, ENCODED_PTG_DATA));
+		ptg.readTokenValues(new TestcaseRecordInputStream(0, ENCODED_CONSTANT_DATA));
+		assertEquals(3, ptg.getColumnCount());
+		assertEquals(2, ptg.getRowCount());
+		
+		assertEquals(0, ptg.getValueIndex(0, 0));
+		assertEquals(1, ptg.getValueIndex(1, 0));
+		assertEquals(2, ptg.getValueIndex(2, 0));
+		assertEquals(3, ptg.getValueIndex(0, 1));
+		assertEquals(4, ptg.getValueIndex(1, 1));
+		assertEquals(5, ptg.getValueIndex(2, 1));
+	}
+}

src/testcases/org/apache/poi/hssf/record/formula/TestFuncVarPtg.java

@@ -0,0 +1,53 @@
+/* ====================================================================
+   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.formula;
+
+import org.apache.poi.hssf.model.FormulaParser;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+/**
+ * @author Josh Micich
+ */
+public final class TestFuncVarPtg extends TestCase {
+
+	/**
+	 * The first fix for bugzilla 44675 broke the encoding of SUM formulas (and probably others).
+	 * The operand classes of the parameters to SUM() should be coerced to 'reference' not 'value'.
+	 * In the case of SUM, Excel evaluates the formula to '#VALUE!' if a parameter operand class is
+	 * wrong.  In other cases Excel seems to tolerate bad operand classes.</p>
+	 * This functionality is related to the setParameterRVA() methods of <tt>FormulaParser</tt>
+	 */
+	public void testOperandClass() {
+		HSSFWorkbook book = new HSSFWorkbook();
+		Ptg[] ptgs = FormulaParser.parse("sum(A1:A2)", book);
+		assertEquals(2, ptgs.length);
+		assertEquals(AreaPtg.class, ptgs[0].getClass());
+		
+		switch(ptgs[0].getPtgClass()) {
+			case Ptg.CLASS_REF:
+				// correct behaviour
+				break;
+			case Ptg.CLASS_VALUE:
+				throw new AssertionFailedError("Identified bug 44675b");
+			default:
+				throw new RuntimeException("Unexpected operand class");
+		}
+	}
+}

src/testcases/org/apache/poi/hssf/record/formula/TestReferencePtg.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,20 +17,22 @@
 
 package org.apache.poi.hssf.record.formula;
 
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.usermodel.HSSFSheet;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 
 /**
  * Tests for {@link ReferencePtg}.
  */
-public class TestReferencePtg extends AbstractPtgTestCase
+public final class TestReferencePtg extends TestCase {
-{
     /**
      * Tests reading a file containing this ptg.
      */
-    public void testReading() throws Exception
+    public void testReading() {
-    {
+        HSSFWorkbook workbook = HSSFTestDataSamples.openSampleWorkbook("ReferencePtg.xls");
-        HSSFWorkbook workbook = loadWorkbook("ReferencePtg.xls");
         HSSFSheet sheet = workbook.getSheetAt(0);
 
         // First row
@@ -72,6 +73,18 @@ public class TestReferencePtg extends AbstractPtgTestCase
         assertEquals("Wrong formula string for reference", "A32770",
                 sheet.getRow(32769).getCell((short) 1).getCellFormula());
     }
+    
+    public void testBug44921() {
+        HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex44921-21902.xls");
+        
+        try {
+            HSSFTestDataSamples.writeOutAndReadBack(wb);
+        } catch (RuntimeException e) {
+            if(e.getMessage().equals("Coding Error: This method should never be called. This ptg should be converted")) {
+                throw new AssertionFailedError("Identified bug 44921");
+            }
+            throw e;
+        }
+    }
 }
 
-

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

@@ -46,6 +46,7 @@ public class AllUserModelTests {
 		result.addTestSuite(TestHSSFHeaderFooter.class);
 		result.addTestSuite(TestHSSFHyperlink.class);
 		result.addTestSuite(TestHSSFPalette.class);
+		result.addTestSuite(TestHSSFPatriarch.class);
 		result.addTestSuite(TestHSSFPicture.class);
 		result.addTestSuite(TestHSSFPictureData.class);
 		result.addTestSuite(TestHSSFRichTextString.class);

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

@@ -733,7 +733,7 @@ public final class TestBugs extends TestCase {
      *  with the NameRecord, once you get past the BOFRecord
      *  issue.
      */
-    public void DISABLEDtest42564Alt() {
+    public void test42564Alt() {
         HSSFWorkbook wb = openSample("42564-2.xls");
         writeOutAndReadBack(wb);
     }
@@ -883,10 +883,32 @@ public final class TestBugs extends TestCase {
      * Bug 28774: Excel will crash when opening xls-files with images.
      */
     public void test28774() {
-
         HSSFWorkbook wb = openSample("28774.xls");
         assertTrue("no errors reading sample xls", true);
         writeOutAndReadBack(wb);
         assertTrue("no errors writing sample xls", true);
     }
+    
+    /**
+     * Had a problem apparently, not sure what as it
+     *  works just fine...
+     */
+    public void test44891() throws Exception {
+    	HSSFWorkbook wb = openSample("44891.xls");
+        assertTrue("no errors reading sample xls", true);
+        writeOutAndReadBack(wb);
+        assertTrue("no errors writing sample xls", true);
+    }
+
+    /**
+     * Bug 44235: Ms Excel can't open save as excel file
+     *
+     * Works fine with poi-3.1-beta1.
+     */
+    public void test44235() throws Exception {
+    	HSSFWorkbook wb = openSample("44235.xls");
+        assertTrue("no errors reading sample xls", true);
+        writeOutAndReadBack(wb);
+        assertTrue("no errors writing sample xls", true);
+    }
 }

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

@@ -19,6 +19,7 @@ package org.apache.poi.hssf.usermodel;
 
 import java.io.File;
 import java.io.FileOutputStream;
+import java.util.Iterator;
 import java.util.List;
 
 import junit.framework.TestCase;
@@ -252,4 +253,29 @@ public final class TestFormulaEvaluatorBugs extends TestCase {
 		}
 		assertEquals(true, cell.getBooleanCellValue());
 	}
+	
+	public void testClassCast_bug44861() throws Exception {
+		HSSFWorkbook wb = HSSFTestDataSamples.
+			openSampleWorkbook("44861.xls");
+		
+		// Check direct
+		HSSFFormulaEvaluator.evaluateAllFormulaCells(wb);
+		
+		// And via calls
+		int numSheets = wb.getNumberOfSheets();
+		for(int i=0; i<numSheets; i++) {
+			HSSFSheet s = wb.getSheetAt(i);
+			HSSFFormulaEvaluator eval = new HSSFFormulaEvaluator(s,wb);
+			
+			for(Iterator rows = s.rowIterator(); rows.hasNext();) {
+		        HSSFRow r = (HSSFRow)rows.next();
+		        eval.setCurrentRow(r);
+		        
+		        for(Iterator cells = r.cellIterator(); cells.hasNext();) {
+		        	HSSFCell c = (HSSFCell)cells.next();
+		        	eval.evaluateFormulaCell(c);
+		        }
+			}
+		}
+	}
 }

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

@@ -0,0 +1,71 @@
+/* ====================================================================
+   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.usermodel;
+
+import org.apache.poi.hssf.HSSFTestDataSamples;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+/**
+ * @author Josh Micich
+ */
+public final class TestHSSFPatriarch extends TestCase {
+
+	public void testBasic() {
+
+		HSSFWorkbook wb = new HSSFWorkbook();
+		HSSFSheet sheet = wb.createSheet();
+
+		HSSFPatriarch patr = sheet.createDrawingPatriarch();
+
+		assertNotNull(patr);
+
+		// assert something more interesting
+	}
+
+	// TODO - fix bug 44916 (1-May-2008)
+	public void DISABLED_test44916() {
+
+		HSSFWorkbook wb = new HSSFWorkbook();
+		HSSFSheet sheet = wb.createSheet();
+
+		// 1. Create drawing patriarch
+		HSSFPatriarch patr = sheet.createDrawingPatriarch();
+
+		// 2. Try to re-get the patriarch
+		HSSFPatriarch existingPatr;
+		try {
+			existingPatr = sheet.getDrawingPatriarch();
+		} catch (NullPointerException e) {
+			throw new AssertionFailedError("Identified bug 44916");
+		}
+
+		// 3. Use patriarch
+		HSSFClientAnchor anchor = new HSSFClientAnchor(0, 0, 600, 245, (short) 1, 1, (short) 1, 2);
+		anchor.setAnchorType(3);
+		byte[] pictureData = HSSFTestDataSamples.getTestDataFileContent("logoKarmokar4.png");
+		int idx1 = wb.addPicture(pictureData, HSSFWorkbook.PICTURE_TYPE_PNG);
+		patr.createPicture(anchor, idx1);
+
+		// 4. Try to re-use patriarch later
+		existingPatr = sheet.getDrawingPatriarch();
+		assertNotNull(existingPatr);
+	}
+
+}

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

@@ -16,10 +16,6 @@
 */
 package org.apache.poi.hssf.usermodel;
 
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.io.InputStream;
-
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
@@ -36,7 +32,7 @@ public final class TestHSSFPicture extends TestCase{
         HSSFSheet sh1 = wb.createSheet();
         HSSFPatriarch p1 = sh1.createDrawingPatriarch();
 
-        byte[] pictureData = getTestDataFileContent("logoKarmokar4.png");
+        byte[] pictureData = HSSFTestDataSamples.getTestDataFileContent("logoKarmokar4.png");
         int idx1 = wb.addPicture( pictureData, HSSFWorkbook.PICTURE_TYPE_PNG );
         HSSFPicture picture1 = p1.createPicture(new HSSFClientAnchor(), idx1);
         HSSFClientAnchor anchor1 = picture1.getPreferredSize();
@@ -51,28 +47,4 @@ public final class TestHSSFPicture extends TestCase{
         assertEquals(848, anchor1.getDx2());
         assertEquals(240, anchor1.getDy2());
     }
-
-    /**
-     * Copied from org.apache.poi.hssf.usermodel.examples.OfficeDrawing
-     */
-     private static byte[] getTestDataFileContent(String fileName) {
-        ByteArrayOutputStream bos = new ByteArrayOutputStream();
-
-        try {
-            InputStream fis = HSSFTestDataSamples.openSampleFileStream(fileName);
-
-            byte[] buf = new byte[512];
-            while(true) {
-                int bytesRead = fis.read(buf);
-                if(bytesRead < 1) {
-                    break;
-                }
-                bos.write(buf, 0, bytesRead);
-            }
-            fis.close();
-        } catch (IOException e) {
-            throw new RuntimeException(e);
-        }
-        return bos.toByteArray();
-     }
 }

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

@@ -17,6 +17,7 @@
 
 package org.apache.poi.hssf.usermodel;
 
+import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
@@ -40,6 +41,23 @@ public final class TestHSSFWorkbook extends TestCase {
         assertEquals( 3, nameRecord.getIndexToSheet() );
     }
 
+    public void testCaseInsensitiveNames() {
+        HSSFWorkbook b = new HSSFWorkbook( );
+        HSSFSheet originalSheet = b.createSheet("Sheet1");
+        HSSFSheet fetchedSheet = b.getSheet("sheet1");
+        if(fetchedSheet == null) {
+            throw new AssertionFailedError("Identified bug 44892");
+        }
+        assertEquals(originalSheet, fetchedSheet);
+        try {
+            b.createSheet("sHeeT1");
+            fail("should have thrown exceptiuon due to duplicate sheet name");
+        } catch (IllegalArgumentException e) {
+            // expected during successful test
+            assertEquals("The workbook already contains a sheet of this name", e.getMessage());
+        }
+    }
+
     public void testDuplicateNames() {
         HSSFWorkbook b = new HSSFWorkbook( );
         b.createSheet("Sheet1");