Fix for bug 46280 - RowRecordsAggregate should allow for ContinueRecords after UnkownRecords, and PivotTable records should not get in the RowRecordsAggregate at all

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

3 files changed, 101 insertions, 1 deletions

diff --git a/src/testcases/org/apache/poi/hssf/model/AllModelTests.java b/src/testcases/org/apache/poi/hssf/model/AllModelTests.java
index f157d3a0f2..35565a7a65 100755
--- a/src/testcases/org/apache/poi/hssf/model/AllModelTests.java
+++ b/src/testcases/org/apache/poi/hssf/model/AllModelTests.java
@@ -35,6 +35,7 @@ public final class AllModelTests {
 		result.addTestSuite(TestFormulaParserEval.class);
 		result.addTestSuite(TestFormulaParserIf.class);
 		result.addTestSuite(TestOperandClassTransformer.class);
+		result.addTestSuite(TestRowBlocksReader.class);
 		result.addTestSuite(TestRVA.class);
 		result.addTestSuite(TestSheet.class);
 		result.addTestSuite(TestSheetAdditional.class);
diff --git a/src/testcases/org/apache/poi/hssf/model/TestRowBlocksReader.java b/src/testcases/org/apache/poi/hssf/model/TestRowBlocksReader.java
new file mode 100644
index 0000000000..652bea1a16
--- /dev/null
+++ b/src/testcases/org/apache/poi/hssf/model/TestRowBlocksReader.java
@@ -0,0 +1,60 @@
+/* ====================================================================

+   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.model;

+

+import java.util.Arrays;

+

+import junit.framework.AssertionFailedError;

+import junit.framework.TestCase;

+

+import org.apache.poi.hssf.record.NumberRecord;

+import org.apache.poi.hssf.record.Record;

+import org.apache.poi.hssf.record.RowRecord;

+import org.apache.poi.hssf.record.UnknownRecord;

+import org.apache.poi.hssf.record.WindowTwoRecord;

+

+/**

+ * Tests for {@link RowBlocksReader}

+ * 

+ * @author Josh Micich

+ */

+public final class TestRowBlocksReader extends TestCase {

+	public void testAbnormalPivotTableRecords_bug46280() {

+		int SXVIEW_SID = 0x00B0;

+		Record[] inRecs = {

+			new RowRecord(0),

+			new NumberRecord(),

+			// normally MSODRAWING(0x00EC) would come here before SXVIEW

+			new UnknownRecord(SXVIEW_SID, "dummydata (SXVIEW: View Definition)".getBytes()),

+			new WindowTwoRecord(),

+		};

+		RecordStream rs = new RecordStream(Arrays.asList(inRecs), 0);

+		RowBlocksReader rbr = new RowBlocksReader(rs);

+		if (rs.peekNextClass() == WindowTwoRecord.class) {

+			// Should have stopped at the SXVIEW record 

+			throw new AssertionFailedError("Identified bug 46280b");

+		}

+		RecordStream rbStream = rbr.getPlainRecordStream();

+		assertEquals(inRecs[0], rbStream.getNext());

+		assertEquals(inRecs[1], rbStream.getNext());

+		assertFalse(rbStream.hasNext());

+		assertTrue(rs.hasNext());

+		assertEquals(inRecs[2], rs.getNext());

+		assertEquals(inRecs[3], rs.getNext());

+	}

+}

diff --git a/src/testcases/org/apache/poi/hssf/record/aggregates/TestRowRecordsAggregate.java b/src/testcases/org/apache/poi/hssf/record/aggregates/TestRowRecordsAggregate.java
index 7921b2607c..6c0ae1d3ae 100644
--- a/src/testcases/org/apache/poi/hssf/record/aggregates/TestRowRecordsAggregate.java
+++ b/src/testcases/org/apache/poi/hssf/record/aggregates/TestRowRecordsAggregate.java
@@ -21,24 +21,30 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.util.Arrays;
 
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.ArrayRecord;
+import org.apache.poi.hssf.record.ContinueRecord;
 import org.apache.poi.hssf.record.FormulaRecord;
+import org.apache.poi.hssf.record.NumberRecord;
 import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RowRecord;
 import org.apache.poi.hssf.record.SharedFormulaRecord;
 import org.apache.poi.hssf.record.SharedValueRecordBase;
 import org.apache.poi.hssf.record.TableRecord;
+import org.apache.poi.hssf.record.UnknownRecord;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.hssf.usermodel.RecordInspector;
+import org.apache.poi.hssf.usermodel.RecordInspector.RecordCollector;
 import org.apache.poi.hssf.util.CellRangeAddress8Bit;
 
 /**
- * 
+ * Tests for {@link RowRecordsAggregate}
  */
 public final class TestRowRecordsAggregate extends TestCase {
 
@@ -113,4 +119,37 @@ public final class TestRowRecordsAggregate extends TestCase {
 		assertEquals(range.getFirstRow(), firstFormula.getRow());
 		assertEquals(range.getFirstColumn(), firstFormula.getColumn());
 	}
+
+	/**
+	 * This problem was noted as the overt symptom of bug 46280.  The logic for skipping {@link
+	 * UnknownRecord}s in the constructor {@link RowRecordsAggregate} did not allow for the
+	 * possibility of tailing {@link ContinueRecord}s.<br/>
+	 * The functionality change being tested here is actually not critical to the overall fix
+	 * for bug 46280, since the fix involved making sure the that offending <i>PivotTable</i>
+	 * records do not get into {@link RowRecordsAggregate}.<br/>
+	 * This fix in {@link RowRecordsAggregate} was implemented anyway since any {@link 
+	 * UnknownRecord} has the potential of being 'continued'.
+	 */
+	public void testUnknownContinue_bug46280() {
+		Record[] inRecs = {
+			new RowRecord(0),
+			new NumberRecord(),
+			new UnknownRecord(0x5555, "dummydata".getBytes()),
+			new ContinueRecord("moredummydata".getBytes()),
+		};
+		RecordStream rs = new RecordStream(Arrays.asList(inRecs), 0);
+		RowRecordsAggregate rra;
+		try {
+			rra = new RowRecordsAggregate(rs, SharedValueManager.EMPTY);
+		} catch (RuntimeException e) {
+			if (e.getMessage().startsWith("Unexpected record type")) {
+				throw new AssertionFailedError("Identified bug 46280a");
+			}
+			throw e;
+		}
+		RecordCollector rv = new RecordCollector();
+		rra.visitContainedRecords(rv);
+		Record[] outRecs = rv.getRecords();
+		assertEquals(5, outRecs.length);
+	}
 }