fix bug #44297: IntPtg must operate with unsigned short. Reading signed short results in incorrect formula calculation.

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

src/documentation/content/xdocs/changes.xml

@@ -36,6 +36,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.0.2-FINAL" date="2008-??-??">
+            <action dev="POI-DEVELOPERS" type="fix">44297 - IntPtg must operate with unsigned short. Reading signed short results in incorrect formula calculation</action>
             <action dev="POI-DEVELOPERS" type="fix">44296 - Fix for reading slide background images</action>
             <action dev="POI-DEVELOPERS" type="fix">44293 - Avoid swapping AreaPtgs from relative to absolute</action>
             <action dev="POI-DEVELOPERS" type="fix">44292 - Correctly process the last paragraph in a word file</action>

src/documentation/content/xdocs/status.xml

@@ -33,6 +33,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.0.2-FINAL" date="2008-??-??">
+            <action dev="POI-DEVELOPERS" type="fix">44297 - IntPtg must operate with unsigned short. Reading signed short results in incorrect formula calculation</action>
             <action dev="POI-DEVELOPERS" type="fix">44296 - Fix for reading slide background images</action>
             <action dev="POI-DEVELOPERS" type="fix">44293 - Avoid swapping AreaPtgs from relative to absolute</action>
             <action dev="POI-DEVELOPERS" type="fix">44292 - Correctly process the last paragraph in a word file</action>

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

@@ -40,7 +40,7 @@ public class IntPtg
 {
     public final static int  SIZE = 3;
     public final static byte sid  = 0x1e;
-    private short            field_1_value;
+    private int            field_1_value;
   
     private IntPtg() {
       //Required for clone methods
@@ -48,42 +48,31 @@ public class IntPtg
 
     public IntPtg(RecordInputStream in)
     {
-        setValue(in.readShort());
+        setValue(in.readUShort());
     }
     
     
     // IntPtg should be able to create itself, shouldnt have to call setValue
     public IntPtg(String formulaToken) {
-        setValue(Short.parseShort(formulaToken));
+        setValue(Integer.parseInt(formulaToken));
     }
 
     /**
      * Sets the wrapped value.
      * Normally you should call with a positive int.
      */
-    public void setValue(short value)
-    {
-        field_1_value = value;
-    }
-
-    /**
-     * Sets the unsigned value.
-     * (Handles conversion to the internal short value) 
-     */
     public void setValue(int value)
     {
-    	if(value > Short.MAX_VALUE) {
+        if(value < 0 || value > (Short.MAX_VALUE + 1)*2 )
-    		// Need to wrap
+            throw new IllegalArgumentException("Unsigned short is out of range: " + value);
-    		value -= (Short.MAX_VALUE+1)*2;
+        field_1_value = value;
-    	}
-    	field_1_value = (short)value;
     }
 
     /**
      * Returns the value as a short, which may have
      *  been wrapped into negative numbers
      */
-    public short getValue()
+    public int getValue()
     {
         return field_1_value;
     }
@@ -102,7 +91,7 @@ public class IntPtg
     public void writeBytes(byte [] array, int offset)
     {
         array[ offset + 0 ] = sid;
-        LittleEndian.putShort(array, offset + 1, getValue());
+        LittleEndian.putUShort(array, offset + 1, getValue());
     }
 
     public int getSize()

src/scratchpad/testcases/org/apache/poi/hssf/usermodel/TestBug44297.java

@@ -0,0 +1,103 @@
+package org.apache.poi.hssf.usermodel;
+/* ====================================================================
+   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.
+==================================================================== */
+
+import junit.framework.TestCase;
+
+import java.io.IOException;
+import java.io.FileInputStream;
+import java.io.File;
+
+/**
+ * Bug 44297: 32767+32768 is evaluated to -1
+ * Fix: IntPtg must operate with unsigned short. Reading signed short results in incorrect formula calculation
+ * if a formula has values in the interval [Short.MAX_VALUE, (Short.MAX_VALUE+1)*2]
+ *
+ * @author Yegor Kozlov
+ */
+
+public class TestBug44297 extends TestCase {
+    protected String cwd = System.getProperty("HSSF.testdata.path");
+
+    public void test44297() throws IOException {
+        FileInputStream in = new FileInputStream(new File(cwd, "44297.xls"));
+        HSSFWorkbook wb = new HSSFWorkbook(in);
+        in.close();
+
+        HSSFRow row;
+        HSSFCell  cell;
+
+        HSSFSheet sheet   = wb.getSheetAt(0);
+
+        HSSFFormulaEvaluator eva = new HSSFFormulaEvaluator(sheet, wb);
+
+        row = (HSSFRow)sheet.getRow(0);
+        cell = row.getCell((short)0);
+        assertEquals("31+46", cell.getCellFormula());
+        eva.setCurrentRow(row);
+        assertEquals(77, eva.evaluate(cell).getNumberValue(), 0);
+
+        row = (HSSFRow)sheet.getRow(1);
+        cell = row.getCell((short)0);
+        assertEquals("30+53", cell.getCellFormula());
+        eva.setCurrentRow(row);
+        assertEquals(83, eva.evaluate(cell).getNumberValue(), 0);
+
+        row = (HSSFRow)sheet.getRow(2);
+        cell = row.getCell((short)0);
+        assertEquals("SUM(A1:A2)", cell.getCellFormula());
+        eva.setCurrentRow(row);
+        assertEquals(160, eva.evaluate(cell).getNumberValue(), 0);
+
+        row = (HSSFRow)sheet.getRow(4);
+        cell = row.getCell((short)0);
+        assertEquals("32767+32768", cell.getCellFormula());
+        eva.setCurrentRow(row);
+        assertEquals(65535, eva.evaluate(cell).getNumberValue(), 0);
+
+        row = (HSSFRow)sheet.getRow(7);
+        cell = row.getCell((short)0);
+        assertEquals("32744+42333", cell.getCellFormula());
+        eva.setCurrentRow(row);
+        assertEquals(75077, eva.evaluate(cell).getNumberValue(), 0);
+
+        row = (HSSFRow)sheet.getRow(8);
+        cell = row.getCell((short)0);
+        assertEquals("327680.0/32768", cell.getCellFormula());
+        eva.setCurrentRow(row);
+        assertEquals(10, eva.evaluate(cell).getNumberValue(), 0);
+
+        row = (HSSFRow)sheet.getRow(9);
+        cell = row.getCell((short)0);
+        assertEquals("32767+32769", cell.getCellFormula());
+        eva.setCurrentRow(row);
+        assertEquals(65536, eva.evaluate(cell).getNumberValue(), 0);
+
+        row = (HSSFRow)sheet.getRow(10);
+        cell = row.getCell((short)0);
+        assertEquals("35000+36000", cell.getCellFormula());
+        eva.setCurrentRow(row);
+        assertEquals(71000, eva.evaluate(cell).getNumberValue(), 0);
+
+        row = (HSSFRow)sheet.getRow(11);
+        cell = row.getCell((short)0);
+        assertEquals("-1000000.0-3000000.0", cell.getCellFormula());
+        eva.setCurrentRow(row);
+        assertEquals(-4000000, eva.evaluate(cell).getNumberValue(), 0);
+    }
+
+}

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

@@ -379,15 +379,16 @@ public class TestFormulaParser extends TestCase {
 		fp = new FormulaParser("40000", null);
 		fp.parse();
 		ptg=fp.getRPNPtg();
-		assertTrue("ptg should be  Number, is "+ptg[0].getClass(), ptg[0] instanceof NumberPtg);
+		assertTrue("ptg should be  IntPtg, is "+ptg[0].getClass(), ptg[0] instanceof IntPtg);
 	}
+
 	/** bug 33160, testcase by Amol Deshmukh*/
 	public void testSimpleLongFormula() {
 		        FormulaParser fp = new FormulaParser("40000/2", null);
 		        fp.parse();
 		        Ptg[] ptgs = fp.getRPNPtg();
 		        assertTrue("three tokens expected, got "+ptgs.length,ptgs.length == 3);
-		        assertTrue("NumberPtg",(ptgs[0] instanceof NumberPtg));
+		        assertTrue("IntPtg",(ptgs[0] instanceof IntPtg));
 		        assertTrue("IntPtg",(ptgs[1] instanceof IntPtg));
 		        assertTrue("DividePtg",(ptgs[2] instanceof DividePtg));
 	}