diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index d5f6e9c53..3e9214997 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -36,6 +36,7 @@ + 44297 - IntPtg must operate with unsigned short. Reading signed short results in incorrect formula calculation 44296 - Fix for reading slide background images 44293 - Avoid swapping AreaPtgs from relative to absolute 44292 - Correctly process the last paragraph in a word file diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 69190f915..dfe4e5aa6 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -33,6 +33,7 @@ + 44297 - IntPtg must operate with unsigned short. Reading signed short results in incorrect formula calculation 44296 - Fix for reading slide background images 44293 - Avoid swapping AreaPtgs from relative to absolute 44292 - Correctly process the last paragraph in a word file diff --git a/src/java/org/apache/poi/hssf/record/formula/IntPtg.java b/src/java/org/apache/poi/hssf/record/formula/IntPtg.java index 602289776..257c089df 100644 --- a/src/java/org/apache/poi/hssf/record/formula/IntPtg.java +++ b/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) { - // Need to wrap - value -= (Short.MAX_VALUE+1)*2; - } - field_1_value = (short)value; + if(value < 0 || value > (Short.MAX_VALUE + 1)*2 ) + throw new IllegalArgumentException("Unsigned short is out of range: " + value); + field_1_value = 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() diff --git a/src/scratchpad/testcases/org/apache/poi/hssf/data/44297.xls b/src/scratchpad/testcases/org/apache/poi/hssf/data/44297.xls new file mode 100755 index 000000000..bc65efd4e Binary files /dev/null and b/src/scratchpad/testcases/org/apache/poi/hssf/data/44297.xls differ diff --git a/src/scratchpad/testcases/org/apache/poi/hssf/usermodel/TestBug44297.java b/src/scratchpad/testcases/org/apache/poi/hssf/usermodel/TestBug44297.java new file mode 100755 index 000000000..ce4afd36f --- /dev/null +++ b/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); + } + +} diff --git a/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java b/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java index 9e6b53e9a..1b41d87d4 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java +++ b/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)); }