Follow-on from 28754 - StringPtg.toFormulaString() should escape double quotes

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

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.1-beta2" date="2008-05-??">
+           <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>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1-beta2" date="2008-05-??">
+           <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>

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
+ * 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 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
+     * NOTE: OO doc says 16bit length, but BiffViewer says 8 Book says something
-{
+     * totally different, so don't look there!
-    public final static int  SIZE = 9;
+     */
-    public final static byte sid  = 0x17;
+    private int field_1_length;
-    //NOTE: OO doc says 16bit lenght, but BiffViewer says 8
+    private byte field_2_options;
-    // Book says something totally different, so dont look there!
+    private String field_3_string;
-    int field_1_length;
-    byte field_2_options;
-    BitField fHighByte = BitFieldFactory.getInstance(0x01);
-    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(HSSFWorkbook 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(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() {
+    public Object clone() {
-     StringPtg ptg = new StringPtg();
+        StringPtg ptg = new StringPtg();
-     ptg.field_1_length = field_1_length;
+        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;
+        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/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,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(){
+    public void testConcatenate() {
-         FormulaParser fp = new FormulaParser("CONCATENATE(\"first\",\"second\")",null);
+		FormulaParser fp = new FormulaParser("CONCATENATE(\"first\",\"second\")", null);
-         fp.parse();
+		fp.parse();
-         Ptg[] ptg = fp.getRPNPtg();
+		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()
     {
@@ -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);
+		FormulaParser fp = new FormulaParser("40000/2", null);
-		        fp.parse();
+		fp.parse();
-		        Ptg[] ptgs = fp.getRPNPtg();
+		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();
     	
@@ -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("IF(TRUE)");
+        parseExpectedException("countif(A1:B5, C1, D1)");
-            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);