diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 150b41ff9..a653d3c00 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + Follow-on from 28754 - StringPtg.toFormulaString() should escape double quotes 44929 - Improved error handling in HSSFWorkbook when attempting to read a BIFF5 file 44675 - Parameter operand classes (function metadata) required to encode SUM() etc properly. Added parse validation for number of parameters 44921 - allow Ptg.writeBytes() to be called on relative ref Ptgs (RefN* and AreaN*) diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index e9059493b..f2b9fb8e1 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + Follow-on from 28754 - StringPtg.toFormulaString() should escape double quotes 44929 - Improved error handling in HSSFWorkbook when attempting to read a BIFF5 file 44675 - Parameter operand classes (function metadata) required to encode SUM() etc properly. Added parse validation for number of parameters 44921 - allow Ptg.writeBytes() to be called on relative ref Ptgs (RefN* and AreaN*) diff --git a/src/java/org/apache/poi/hssf/record/formula/StringPtg.java b/src/java/org/apache/poi/hssf/record/formula/StringPtg.java index ca6fb55dc..47bd6ab6e 100644 --- a/src/java/org/apache/poi/hssf/record/formula/StringPtg.java +++ b/src/java/org/apache/poi/hssf/record/formula/StringPtg.java @@ -24,106 +24,123 @@ import org.apache.poi.util.StringUtil; import org.apache.poi.hssf.record.RecordInputStream; /** - * Number - * Stores a String value in a formula value stored in the format <length 2 bytes>char[] - * @author Werner Froidevaux + * String Stores a String value in a formula value stored in the format + * <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 final static int SIZE = 9; + public final static byte sid = 0x17; + private static final BitField fHighByte = BitFieldFactory.getInstance(0x01); + /** the character (")used in formulas to delimit string literals */ + private static final char FORMULA_DELIMITER = '"'; -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 - // Book says something totally different, so dont look there! - int field_1_length; - byte field_2_options; - BitField fHighByte = BitFieldFactory.getInstance(0x01); - private String field_3_string; + /** + * 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) - { - field_1_length = in.readByte() & 0xFF; + public StringPtg(RecordInputStream in) { + field_1_length = in.readUByte(); field_2_options = in.readByte(); if (fHighByte.isSet(field_2_options)) { - field_3_string= in.readUnicodeLEString(field_1_length); - }else { - field_3_string=in.readCompressedUnicode(field_1_length); + field_3_string = in.readUnicodeLEString(field_1_length); + } else { + 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 - * that calls this method. - * @param value : String representation of a floating point number + /** + * Create a StringPtg from a string representation of the number Number + * format is not checked, it is expected to be validated in the parser that + * calls this method. + * + * @param value : + * String representation of a floating point number */ public StringPtg(String value) { - if (value.length() >255) { - throw new IllegalArgumentException("String literals in formulas cant be bigger than 255 characters ASCII"); + if (value.length() > 255) { + throw new IllegalArgumentException( + "String literals in formulas can't be bigger than 255 characters ASCII"); } - this.field_2_options=0; - field_2_options = (byte)this.fHighByte.setBoolean(field_2_options, StringUtil.hasMultibyte(value)); - this.field_3_string=value; - this.field_1_length=value.length(); //for the moment, we support only ASCII strings in formulas we create + field_2_options = 0; + field_2_options = (byte) fHighByte.setBoolean(field_2_options, StringUtil + .hasMultibyte(value)); + field_3_string = value; + field_1_length = value.length(); // for the moment, we support only ASCII strings in formulas we create } - /* - public void setValue(String value) - { - field_1_value = value; - }*/ - - - public String getValue() - { + public String getValue() { return field_3_string; } - public void writeBytes(byte [] array, int offset) - { - array[ offset + 0 ] = sid; - array[ offset + 1 ] = (byte)field_1_length; - array[ offset + 2 ] = field_2_options; + public void writeBytes(byte[] array, int offset) { + array[offset + 0] = sid; + array[offset + 1] = (byte) field_1_length; + array[offset + 2] = field_2_options; if (fHighByte.isSet(field_2_options)) { - 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; + StringUtil.putUnicodeLE(getValue(), array, offset + 3); } else { - return field_1_length+3; + StringUtil.putCompressedUnicode(getValue(), array, offset + 3); } } - public String toFormulaString(HSSFWorkbook book) - { - return "\""+getValue()+"\""; + public int getSize() { + if (fHighByte.isSet(field_2_options)) { + return 2 * field_1_length + 3; + } else { + return field_1_length + 3; + } } + + public String toFormulaString(HSSFWorkbook 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; - } + return Ptg.CLASS_VALUE; + } - public Object clone() { - StringPtg ptg = new StringPtg(); - ptg.field_1_length = field_1_length; - ptg.field_2_options=field_2_options; - ptg.field_3_string=field_3_string; - return ptg; - } + public Object clone() { + StringPtg ptg = new StringPtg(); + ptg.field_1_length = field_1_length; + ptg.field_2_options = field_2_options; + 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(); + } } - diff --git a/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java b/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java index 2f476c892..1895705a5 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java +++ b/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 null */ 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); - fp.parse(); - Ptg[] asts = fp.getRPNPtg(); + Ptg[] asts = parseFormula(function); 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=1,\"*\",IF(4<>1,\"first\",\"second\"))",null); - fp.parse(); - Ptg[] ptgs = fp.getRPNPtg(); - assertTrue("Ptg array should not be null", ptgs !=null); + Ptg[] ptgs = parseFormula("IF(3>=1,\"*\",IF(4<>1,\"first\",\"second\"))"); 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,16 +279,15 @@ 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(){ - FormulaParser fp = new FormulaParser("CONCATENATE(\"first\",\"second\")",null); - fp.parse(); - 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() { + FormulaParser fp = new FormulaParser("CONCATENATE(\"first\",\"second\")", null); + fp.parse(); + Ptg[] ptg = fp.getRPNPtg(); + assertTrue("first ptg is string", ptg[0] instanceof StringPtg); + assertTrue("second ptg is string", ptg[1] instanceof StringPtg); + } public void testWorksheetReferences() { @@ -395,16 +371,16 @@ public final class TestFormulaParser extends TestCase { /** 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("IntPtg",(ptgs[0] instanceof IntPtg)); - assertTrue("IntPtg",(ptgs[1] instanceof IntPtg)); - assertTrue("DividePtg",(ptgs[2] instanceof DividePtg)); + FormulaParser fp = new FormulaParser("40000/2", null); + fp.parse(); + Ptg[] ptgs = fp.getRPNPtg(); + assertTrue("three tokens expected, got " + ptgs.length, ptgs.length == 3); + assertTrue("IntPtg", (ptgs[0] instanceof IntPtg)); + assertTrue("IntPtg", (ptgs[1] instanceof IntPtg)); + 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(); @@ -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,10 +812,8 @@ 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)"); - } + parseExpectedException("IF(TRUE)"); + parseExpectedException("countif(A1:B5, C1, D1)"); } private static void parseExpectedException(String formula) { @@ -896,7 +896,7 @@ public final class TestFormulaParser extends TestCase { * (e.g. COUNTIF) Excel fails to evaluate the formula, giving '#VALUE!' instead. */ public void testFuncPtgSelection() { - HSSFWorkbook book = new HSSFWorkbook(); + HSSFWorkbook book = new HSSFWorkbook(); Ptg[] ptgs; ptgs = FormulaParser.parse("countif(A1:A2, 1)", book); assertEquals(3, ptgs.length);