]> source.dussan.org Git - poi.git/commitdiff
Fix for bug #44413 from Josh - Fix for circular references in INDEX, OFFSET, VLOOKUP...
authorNick Burch <nick@apache.org>
Fri, 15 Feb 2008 11:53:25 +0000 (11:53 +0000)
committerNick Burch <nick@apache.org>
Fri, 15 Feb 2008 11:53:25 +0000 (11:53 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@628029 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml
src/documentation/content/xdocs/status.xml
src/scratchpad/src/org/apache/poi/hssf/usermodel/EvaluationCycleDetector.java [new file with mode: 0755]
src/scratchpad/src/org/apache/poi/hssf/usermodel/EvaluationCycleDetectorManager.java [new file with mode: 0755]
src/scratchpad/src/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java
src/scratchpad/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java [new file with mode: 0755]

index d8bbb40a190bfd9c96aeccee8c564ca20c958cd3..1c91e62f85b328e9a472546f57b25773c35b888c 100644 (file)
@@ -36,6 +36,7 @@
 
                <!-- Don't forget to update status.xml too! -->
         <release version="3.1-beta1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">44413 - Fix for circular references in INDEX, OFFSET, VLOOKUP formulas, where a cell is actually allowed to reference itself</action>
            <action dev="POI-DEVELOPERS" type="fix">44403 - Fix for Mid function handling its arguments wrong</action>
            <action dev="POI-DEVELOPERS" type="add">44364 - Support for Match, NA and SumProduct functions, as well as initial function error support</action>
            <action dev="POI-DEVELOPERS" type="fix">44375 - Cope with a broken dictionary in Document Summary Information stream. RuntimeExceptions that occured when trying to read bogus data are now caught. Dictionary entries up to but not including the bogus one are preserved, the rest is ignored.</action>
index 73e2f6852fdffa35ce06d6690813750c97d8db46..67d1ddd4ce3b94ee5bef14552f121f7f0e9b8101 100644 (file)
@@ -33,6 +33,7 @@
        <!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1-beta1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">44413 - Fix for circular references in INDEX, OFFSET, VLOOKUP formulas, where a cell is actually allowed to reference itself</action>
            <action dev="POI-DEVELOPERS" type="fix">44403 - Fix for Mid function handling its arguments wrong</action>
            <action dev="POI-DEVELOPERS" type="add">44364 - Support for Match, NA and SumProduct functions, as well as initial function error support</action>
            <action dev="POI-DEVELOPERS" type="fix">44375 - Cope with a broken dictionary in Document Summary Information stream. RuntimeExceptions that occured when trying to read bogus data are now caught. Dictionary entries up to but not including the bogus one are preserved, the rest is ignored.</action>
diff --git a/src/scratchpad/src/org/apache/poi/hssf/usermodel/EvaluationCycleDetector.java b/src/scratchpad/src/org/apache/poi/hssf/usermodel/EvaluationCycleDetector.java
new file mode 100755 (executable)
index 0000000..90f5807
--- /dev/null
@@ -0,0 +1,150 @@
+/* ====================================================================
+   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 java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Instances of this class keep track of multiple dependent cell evaluations due
+ * to recursive calls to <tt>HSSFFormulaEvaluator.internalEvaluate()</tt>.
+ * The main purpose of this class is to detect an attempt to evaluate a cell
+ * that is already being evaluated. In other words, it detects circular
+ * references in spreadsheet formulas.
+ * 
+ * @author Josh Micich
+ */
+final class EvaluationCycleDetector {
+
+       /**
+        * Stores the parameters that identify the evaluation of one cell.<br/>
+        */
+       private static final class CellEvaluationFrame {
+
+               private final HSSFWorkbook _workbook;
+               private final HSSFSheet _sheet;
+               private final int _srcRowNum;
+               private final int _srcColNum;
+
+               public CellEvaluationFrame(HSSFWorkbook workbook, HSSFSheet sheet, int srcRowNum, int srcColNum) {
+                       if (workbook == null) {
+                               throw new IllegalArgumentException("workbook must not be null");
+                       }
+                       if (sheet == null) {
+                               throw new IllegalArgumentException("sheet must not be null");
+                       }
+                       _workbook = workbook;
+                       _sheet = sheet;
+                       _srcRowNum = srcRowNum;
+                       _srcColNum = srcColNum;
+               }
+
+               public boolean equals(Object obj) {
+                       CellEvaluationFrame other = (CellEvaluationFrame) obj;
+                       if (_workbook != other._workbook) {
+                               return false;
+                       }
+                       if (_sheet != other._sheet) {
+                               return false;
+                       }
+                       if (_srcRowNum != other._srcRowNum) {
+                               return false;
+                       }
+                       if (_srcColNum != other._srcColNum) {
+                               return false;
+                       }
+                       return true;
+               }
+
+               /**
+                * @return human readable string for debug purposes
+                */
+               public String formatAsString() {
+                       return "R=" + _srcRowNum + " C=" + _srcColNum + " ShIx=" + _workbook.getSheetIndex(_sheet);
+               }
+
+               public String toString() {
+                       StringBuffer sb = new StringBuffer(64);
+                       sb.append(getClass().getName()).append(" [");
+                       sb.append(formatAsString());
+                       sb.append("]");
+                       return sb.toString();
+               }
+       }
+
+       private final List _evaluationFrames;
+
+       public EvaluationCycleDetector() {
+               _evaluationFrames = new ArrayList();
+       }
+
+       /**
+        * Notifies this evaluation tracker that evaluation of the specified cell is
+        * about to start.<br/>
+        * 
+        * In the case of a <code>true</code> return code, the caller should
+        * continue evaluation of the specified cell, and also be sure to call
+        * <tt>endEvaluate()</tt> when complete.<br/>
+        * 
+        * In the case of a <code>false</code> return code, the caller should
+        * return an evaluation result of
+        * <tt>ErrorEval.CIRCULAR_REF_ERROR<tt>, and not call <tt>endEvaluate()</tt>.  
+        * <br/>
+        * @return <code>true</code> if the specified cell has not been visited yet in the current 
+        * evaluation. <code>false</code> if the specified cell is already being evaluated.
+        */
+       public boolean startEvaluate(HSSFWorkbook workbook, HSSFSheet sheet, int srcRowNum, int srcColNum) {
+               CellEvaluationFrame cef = new CellEvaluationFrame(workbook, sheet, srcRowNum, srcColNum);
+               if (_evaluationFrames.contains(cef)) {
+                       return false;
+               }
+               _evaluationFrames.add(cef);
+               return true;
+       }
+
+       /**
+        * Notifies this evaluation tracker that the evaluation of the specified
+        * cell is complete. <p/>
+        * 
+        * Every successful call to <tt>startEvaluate</tt> must be followed by a
+        * call to <tt>endEvaluate</tt> (recommended in a finally block) to enable
+        * proper tracking of which cells are being evaluated at any point in time.<p/>
+        * 
+        * Assuming a well behaved client, parameters to this method would not be
+        * required. However, they have been included to assert correct behaviour,
+        * and form more meaningful error messages.
+        */
+       public void endEvaluate(HSSFWorkbook workbook, HSSFSheet sheet, int srcRowNum, int srcColNum) {
+               int nFrames = _evaluationFrames.size();
+               if (nFrames < 1) {
+                       throw new IllegalStateException("Call to endEvaluate without matching call to startEvaluate");
+               }
+
+               nFrames--;
+               CellEvaluationFrame cefExpected = (CellEvaluationFrame) _evaluationFrames.get(nFrames);
+               CellEvaluationFrame cefActual = new CellEvaluationFrame(workbook, sheet, srcRowNum, srcColNum);
+               if (!cefActual.equals(cefExpected)) {
+                       throw new RuntimeException("Wrong cell specified. "
+                                       + "Corresponding startEvaluate() call was for cell {"
+                                       + cefExpected.formatAsString() + "} this endEvaluate() call is for cell {"
+                                       + cefActual.formatAsString() + "}");
+               }
+               // else - no problems so pop current frame 
+               _evaluationFrames.remove(nFrames);
+       }
+}
diff --git a/src/scratchpad/src/org/apache/poi/hssf/usermodel/EvaluationCycleDetectorManager.java b/src/scratchpad/src/org/apache/poi/hssf/usermodel/EvaluationCycleDetectorManager.java
new file mode 100755 (executable)
index 0000000..a06cd20
--- /dev/null
@@ -0,0 +1,46 @@
+/* ====================================================================
+   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;
+
+/**
+ * This class makes an <tt>EvaluationCycleDetector</tt> instance available to
+ * each thread via a <tt>ThreadLocal</tt> in order to avoid adding a parameter
+ * to a few protected methods within <tt>HSSFFormulaEvaluator</tt>.
+ * 
+ * @author Josh Micich
+ */
+final class EvaluationCycleDetectorManager {
+
+       ThreadLocal tl = null;
+       private static ThreadLocal _tlEvaluationTracker = new ThreadLocal() {
+               protected synchronized Object initialValue() {
+                       return new EvaluationCycleDetector();
+               }
+       };
+
+       /**
+        * @return
+        */
+       public static EvaluationCycleDetector getTracker() {
+               return (EvaluationCycleDetector) _tlEvaluationTracker.get();
+       }
+
+       private EvaluationCycleDetectorManager() {
+               // no instances of this class
+       }
+}
index 70b78dfae4a216a31b0304d5f004eb03f4ae9d25..fb0c980e84d485c8d75ba4c97a9772ff26881cb8 100644 (file)
@@ -96,8 +96,6 @@ import org.apache.poi.hssf.usermodel.HSSFSheet;
 /**
  * @author Amol S. Deshmukh &lt; amolweb at ya hoo dot com &gt;
  * 
- * Limitations: Unfortunately, cyclic references will cause stackoverflow
- * exception
  */
 public class HSSFFormulaEvaluator {
                 
@@ -353,16 +351,26 @@ public class HSSFFormulaEvaluator {
      * Dev. Note: Internal evaluate must be passed only a formula cell 
      * else a runtime exception will be thrown somewhere inside the method.
      * (Hence this is a private method.)
-     * 
-     * @param srcCell
-     * @param srcRow
-     * @param sheet
-     * @param workbook
      */
-    protected static ValueEval internalEvaluate(HSSFCell srcCell, HSSFRow srcRow, HSSFSheet sheet, HSSFWorkbook workbook) {
+    private static ValueEval internalEvaluate(HSSFCell srcCell, HSSFRow srcRow, HSSFSheet sheet, HSSFWorkbook workbook) {
         int srcRowNum = srcRow.getRowNum();
         short srcColNum = srcCell.getCellNum();
-        FormulaParser parser = new FormulaParser(srcCell.getCellFormula(), workbook.getWorkbook());
+        
+        
+        EvaluationCycleDetector tracker = EvaluationCycleDetectorManager.getTracker();
+        
+        if(!tracker.startEvaluate(workbook, sheet, srcRowNum, srcColNum)) {
+               return ErrorEval.CIRCULAR_REF_ERROR;
+        }
+        try {
+               return evaluateCell(workbook, sheet, srcRowNum, srcColNum, srcCell.getCellFormula());
+        } finally {
+               tracker.endEvaluate(workbook, sheet, srcRowNum, srcColNum);
+        }
+    }
+    private static ValueEval evaluateCell(HSSFWorkbook workbook, HSSFSheet sheet, 
+               int srcRowNum, short srcColNum, String cellFormulaText) {
+        FormulaParser parser = new FormulaParser(cellFormulaText, workbook.getWorkbook());
         parser.parse();
         Ptg[] ptgs = parser.getRPNPtg();
         // -- parsing over --
diff --git a/src/scratchpad/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java b/src/scratchpad/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java
new file mode 100755 (executable)
index 0000000..72db658
--- /dev/null
@@ -0,0 +1,125 @@
+/* ====================================================================
+   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.eval;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.usermodel.HSSFCell;
+import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator;
+import org.apache.poi.hssf.usermodel.HSSFRow;
+import org.apache.poi.hssf.usermodel.HSSFSheet;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator.CellValue;
+/**
+ * Tests HSSFFormulaEvaluator for its handling of cell formula circular references.
+ * 
+ * @author Josh Micich
+ */
+public final class TestCircularReferences extends TestCase {
+       /**
+        * Translates StackOverflowError into AssertionFailedError
+        */
+       private static CellValue evaluateWithCycles(HSSFWorkbook wb, HSSFSheet sheet, HSSFRow row, HSSFCell testCell)
+                       throws AssertionFailedError {
+               HSSFFormulaEvaluator evaluator = new HSSFFormulaEvaluator(sheet, wb);
+               evaluator.setCurrentRow(row);
+               try {
+                       return evaluator.evaluate(testCell);
+               } catch (StackOverflowError e) {
+                       throw new AssertionFailedError( "circular reference caused stack overflow error");
+               }
+       }
+       /**
+        * Makes sure that the specified evaluated cell value represents a circular reference error.
+        */
+       private static void confirmCycleErrorCode(CellValue cellValue) {
+               assertTrue(cellValue.getCellType() == HSSFCell.CELL_TYPE_ERROR);
+               assertEquals(ErrorEval.CIRCULAR_REF_ERROR.getErrorCode(), cellValue.getErrorValue());
+       }
+       
+       
+       /**
+        * ASF Bugzilla Bug 44413  
+        * "INDEX() formula cannot contain its own location in the data array range" 
+        */
+       public void testIndexFormula() {
+               
+               HSSFWorkbook wb = new HSSFWorkbook();
+               HSSFSheet sheet = wb.createSheet("Sheet1");
+               
+               short colB = 1;
+               sheet.createRow(0).createCell(colB).setCellValue(1);
+               sheet.createRow(1).createCell(colB).setCellValue(2);
+               sheet.createRow(2).createCell(colB).setCellValue(3);
+               HSSFRow row4 = sheet.createRow(3);
+               HSSFCell testCell = row4.createCell((short)0);
+               // This formula should evaluate to the contents of B2,
+               testCell.setCellFormula("INDEX(A1:B4,2,2)");
+               // However the range A1:B4 also includes the current cell A4.  If the other parameters
+               // were 4 and 1, this would represent a circular reference.  Since POI 'fully' evaluates
+               // arguments before invoking operators, POI must handle such potential cycles gracefully.
+               
+
+               CellValue cellValue = evaluateWithCycles(wb, sheet, row4, testCell);
+               
+               assertTrue(cellValue.getCellType() == HSSFCell.CELL_TYPE_NUMERIC);
+               assertEquals(2, cellValue.getNumberValue(), 0);
+       }
+
+       /**
+        * Cell A1 has formula "=A1"
+        */
+       public void testSimpleCircularReference() {
+               
+               HSSFWorkbook wb = new HSSFWorkbook();
+               HSSFSheet sheet = wb.createSheet("Sheet1");
+               
+               HSSFRow row = sheet.createRow(0);
+               HSSFCell testCell = row.createCell((short)0);
+               testCell.setCellFormula("A1");
+
+               HSSFFormulaEvaluator evaluator = new HSSFFormulaEvaluator(sheet, wb);
+               evaluator.setCurrentRow(row);
+               CellValue cellValue = evaluateWithCycles(wb, sheet, row, testCell);
+               
+               confirmCycleErrorCode(cellValue);
+       }
+
+       /**
+        * A1=B1, B1=C1, C1=D1, D1=A1
+        */
+       public void testMultiLevelCircularReference() {
+               
+               HSSFWorkbook wb = new HSSFWorkbook();
+               HSSFSheet sheet = wb.createSheet("Sheet1");
+               
+               HSSFRow row = sheet.createRow(0);
+               row.createCell((short)0).setCellFormula("B1");
+               row.createCell((short)1).setCellFormula("C1");
+               row.createCell((short)2).setCellFormula("D1");
+               HSSFCell testCell = row.createCell((short)3);
+               testCell.setCellFormula("A1");
+
+               HSSFFormulaEvaluator evaluator = new HSSFFormulaEvaluator(sheet, wb);
+               evaluator.setCurrentRow(row);
+               CellValue cellValue = evaluateWithCycles(wb, sheet, row, testCell);
+               
+               confirmCycleErrorCode(cellValue);
+       }
+}