Fix for bug #44413 from Josh - Fix for circular references in INDEX, OFFSET, VLOOKUP formulas, where a cell is actually allowed to reference itself

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@628029 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.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>

src/documentation/content/xdocs/status.xml

@@ -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>

src/scratchpad/src/org/apache/poi/hssf/usermodel/EvaluationCycleDetector.java

@@ -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);
+	}
+}

src/scratchpad/src/org/apache/poi/hssf/usermodel/EvaluationCycleDetectorManager.java

@@ -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
+	}
+}

src/scratchpad/src/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java

@@ -96,8 +96,6 @@ import org.apache.poi.hssf.usermodel.HSSFSheet;
 /**
  * @author Amol S. Deshmukh < amolweb at ya hoo dot com >
  * 
- * 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 --

src/scratchpad/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java

@@ -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);
+	}
+}