Index: src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java
===================================================================
--- src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java (revision 628992)
+++ src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java (working copy)
@@ -34,7 +34,7 @@
* @author Jason Height (jheight at chariot dot net dot au)
*/
-public class ValueRecordsAggregate
+public final class ValueRecordsAggregate
extends Record
{
public final static short sid = -1000;
@@ -127,7 +127,7 @@
FormulaRecordAggregate lastFormulaAggregate = null;
- // First up, locate all the shared formulas
+ // First up, locate all the shared formulas for this sheet
List sharedFormulas = new java.util.ArrayList();
for (k = offset; k < records.size(); k++)
{
@@ -135,6 +135,10 @@
if (rec instanceof SharedFormulaRecord) {
sharedFormulas.add(rec);
}
+ if(rec instanceof EOFRecord) {
+ // End of current sheet. Ignore all subsequent shared formula records (Bugzilla 44449)
+ break;
+ }
}
// Now do the main processing sweep
@@ -156,6 +160,8 @@
// for us
boolean found = false;
for (int i=sharedFormulas.size()-1;i>=0;i--) {
+ // TODO - there is no junit test case to justify this reversed loop
+ // perhaps it could just run in the normal direction.
SharedFormulaRecord shrd = (SharedFormulaRecord)sharedFormulas.get(i);
if (shrd.isFormulaInShared(formula)) {
shrd.convertSharedFormulaRecord(formula);
@@ -164,9 +170,7 @@
}
}
if (!found) {
- //Sometimes the shared formula flag "seems" to be errornously set,
- //cant really do much about that.
- //throw new RecordFormatException("Could not find appropriate shared formula");
+ handleMissingSharedFormulaRecord(formula);
}
}
@@ -186,6 +190,24 @@
}
/**
+ * Sometimes the shared formula flag "seems" to be erroneously set, in which case there is no
+ * call to SharedFormulaRecord.convertSharedFormulaRecord and hence the
+ * parsedExpression field of this FormulaRecord will not get updated.
+ * As it turns out, this is not a problem, because in these circumstances, the existing value
+ * for parsedExpression is perfectly OK.
+ *
+ * This method may also be used for setting breakpoints to help diagnose issues regarding the
+ * abnormally-set 'shared formula' flags.
+ * (see TestValueRecordsAggregate.testSpuriousSharedFormulaFlag()).
+ *
+ * The method currently does nothing but do not delete it without finding a nice home for this
+ * comment.
+ */
+ private static void handleMissingSharedFormulaRecord(FormulaRecord formula) {
+ // could log an info message here since this is a fairly unusual occurrence.
+ }
+
+ /**
* called by the class that is responsible for writing this sucker.
* Subclasses should implement this so that their data is passed back in a
* byte array.
@@ -300,7 +322,7 @@
return rec;
}
- public class MyIterator implements Iterator {
+ private final class MyIterator implements Iterator {
short nextColumn=-1;
int nextRow,lastRow;
Index: src/testcases/org/apache/poi/hssf/record/aggregates/TestValueRecordsAggregate.java
===================================================================
--- src/testcases/org/apache/poi/hssf/record/aggregates/TestValueRecordsAggregate.java (revision 628992)
+++ src/testcases/org/apache/poi/hssf/record/aggregates/TestValueRecordsAggregate.java (working copy)
@@ -17,15 +17,30 @@
package org.apache.poi.hssf.record.aggregates;
-import junit.framework.TestCase;
-import org.apache.poi.hssf.record.*;
-
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
+import java.util.zip.CRC32;
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.record.BlankRecord;
+import org.apache.poi.hssf.record.FormulaRecord;
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.SharedFormulaRecord;
+import org.apache.poi.hssf.record.UnknownRecord;
+import org.apache.poi.hssf.record.WindowOneRecord;
+import org.apache.poi.hssf.usermodel.HSSFSheet;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+
public class TestValueRecordsAggregate extends TestCase
{
+ private static final String ABNORMAL_SHARED_FORMULA_FLAG_TEST_FILE = "AbnormalSharedFormulaFlag.xls";
ValueRecordsAggregate valueRecord = new ValueRecordsAggregate();
/**
@@ -203,4 +218,117 @@
assertEquals( 36, valueRecord.getRecordSize() );
}
+
+ /**
+ * Sometimes the 'shared formula' flag (FormulaRecord.isSharedFormula()) is set when
+ * there is no corresponding SharedFormulaRecord available. SharedFormulaRecord definitions do
+ * not span multiple sheets. They are are only defined within a sheet, and thus they do not
+ * have a sheet index field (only row and column range fields).
+ * So it is important that the code which locates the SharedFormulaRecord for each
+ * FormulaRecord does not allow matches across sheets.
+ *
+ * Prior to bugzilla 44449 (Feb 2008), POI ValueRecordsAggregate.construct(int, List)
+ * allowed SharedFormulaRecords to be erroneously used across sheets. That incorrect
+ * behaviour is shown by this test.
+ *
+ * Notes on how to produce the test spreadsheet:
+ * The setup for this test (AbnormalSharedFormulaFlag.xls) is rather fragile, insomuchas
+ * re-saving the file (either with Excel or POI) clears the flag.
+ *
+ * - A new spreadsheet was created in Excel (File | New | Blank Workbook).
+ * - Sheet3 was deleted.
+ * - Sheet2!A1 formula was set to '="second formula"', and fill-dragged through A1:A8.
+ * - Sheet1!A1 formula was set to '="first formula"', and also fill-dragged through A1:A8.
+ * - Four rows on Sheet1 "5" through "8" were deleted ('delete rows' alt-E D, not 'clear' Del).
+ * - The spreadsheet was saved as AbnormalSharedFormulaFlag.xls.
+ *
+ * Prior to the row delete action the spreadsheet has two SharedFormulaRecords. One
+ * for each sheet. To expose the bug, the shared formulas have been made to overlap.
+ * The row delete action (as described here) seems to to delete the
+ * SharedFormulaRecord from Sheet1 (but not clear the 'shared formula' flags.
+ * There are other variations on this theme to create the same effect.
+ *
+ */
+ public void testSpuriousSharedFormulaFlag() {
+ File dataDir = new File(System.getProperty("HSSF.testdata.path"));
+ File testFile = new File(dataDir, ABNORMAL_SHARED_FORMULA_FLAG_TEST_FILE);
+
+ long actualCRC = getFileCRC(testFile);
+ long expectedCRC = 2277445406L;
+ if(actualCRC != expectedCRC) {
+ System.err.println("Expected crc " + expectedCRC + " but got " + actualCRC);
+ throw failUnexpectedTestFileChange();
+ }
+ HSSFWorkbook wb;
+ try {
+ FileInputStream in = new FileInputStream(testFile);
+ wb = new HSSFWorkbook(in);
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+
+ HSSFSheet s = wb.getSheetAt(0); // Sheet1
+
+ String cellFormula;
+ cellFormula = getFormulaFromFirstCell(s, 0); // row "1"
+ // the problem is not observable in the first row of the shared formula
+ if(!cellFormula.equals("\"first formula\"")) {
+ throw new RuntimeException("Something else wrong with this test case");
+ }
+
+ // but the problem is observable in rows 2,3,4
+ cellFormula = getFormulaFromFirstCell(s, 1); // row "2"
+ if(cellFormula.equals("\"second formula\"")) {
+ throw new AssertionFailedError("found bug 44449 (Wrong SharedFormulaRecord was used).");
+ }
+ if(!cellFormula.equals("\"first formula\"")) {
+ throw new RuntimeException("Something else wrong with this test case");
+ }
+ }
+ private static String getFormulaFromFirstCell(HSSFSheet s, int rowIx) {
+ return s.getRow(rowIx).getCell((short)0).getCellFormula();
+ }
+
+ /**
+ * If someone opened this particular test file in Excel and saved it, the peculiar condition
+ * which causes the target bug would probably disappear. This test would then just succeed
+ * regardless of whether the fix was present. So a CRC check is performed to make it less easy
+ * for that to occur.
+ */
+ private static RuntimeException failUnexpectedTestFileChange() {
+ String msg = "Test file '" + ABNORMAL_SHARED_FORMULA_FLAG_TEST_FILE + "' has changed. "
+ + "This junit may not be properly testing for the target bug. "
+ + "Either revert the test file or ensure that the new version "
+ + "has the right characteristics to test the target bug.";
+ // A breakpoint in ValueRecordsAggregate.handleMissingSharedFormulaRecord(FormulaRecord)
+ // should get hit during parsing of Sheet1.
+ // If the test spreadsheet is created as directed, this condition should occur.
+ // It is easy to upset the test spreadsheet (for example re-saving will destroy the
+ // peculiar condition we are testing for).
+ throw new RuntimeException(msg);
+ }
+
+ /**
+ * gets a CRC checksum for the content of a file
+ */
+ private static long getFileCRC(File f) {
+ CRC32 crc = new CRC32();
+ byte[] buf = new byte[2048];
+ try {
+ InputStream is = new FileInputStream(f);
+ while(true) {
+ int bytesRead = is.read(buf);
+ if(bytesRead < 1) {
+ break;
+ }
+ crc.update(buf, 0, bytesRead);
+ }
+ is.close();
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+
+ return crc.getValue();
+ }
+
}