Bugzilla 47479 - Fix BoolErrRecord to tolerate incorrect format written by OOO

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

4 files changed, 128 insertions, 25 deletions

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 816028b3f7..2b2da218e5 100644
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -33,6 +33,7 @@
 
     <changes>
         <release version="3.5-beta7" date="2009-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">47479 - Fix BoolErrRecord to tolerate incorrect format written by OOO</action>
            <action dev="POI-DEVELOPERS" type="fix">47448 - Allow HSSFEventFactory to handle non-zero padding at the end of the workbook stream</action>
            <action dev="POI-DEVELOPERS" type="add">47456 - Support for getting OLE object data in PowerPointExtractor</action>
            <action dev="POI-DEVELOPERS" type="fix">47411 - Explicitly set the 1900 date system when creating XSSF workbooks</action>
diff --git a/src/java/org/apache/poi/hssf/record/BoolErrRecord.java b/src/java/org/apache/poi/hssf/record/BoolErrRecord.java
index 9aa862f7d2..580dc782e4 100644
--- a/src/java/org/apache/poi/hssf/record/BoolErrRecord.java
+++ b/src/java/org/apache/poi/hssf/record/BoolErrRecord.java
@@ -29,12 +29,15 @@ import org.apache.poi.util.LittleEndianOutput;
  */
 public final class BoolErrRecord extends CellRecord {
 	public final static short sid = 0x0205;
-	private byte field_4_bBoolErr;
-	private byte field_5_fError;
+	private int _value;
+	/**
+	 * If <code>true</code>, this record represents an error cell value, otherwise this record represents a boolean cell value
+	 */
+	private boolean _isError;
 
 	/** Creates new BoolErrRecord */
 	public BoolErrRecord() {
-		// fields uninitialised 
+		// fields uninitialised
 	}
 
 	/**
@@ -42,8 +45,29 @@ public final class BoolErrRecord extends CellRecord {
 	 */
 	public BoolErrRecord(RecordInputStream in) {
 		super(in);
-		field_4_bBoolErr = in.readByte();
-		field_5_fError   = in.readByte();
+		switch (in.remaining()) {
+			case 2:
+				_value = in.readByte();
+				break;
+			case 3:
+				_value = in.readUShort();
+				break;
+			default:
+				throw new RecordFormatException("Unexpected size ("
+						+ in.remaining() + ") for BOOLERR record.");
+		}
+		int flag = in.readUByte();
+		switch (flag) {
+			case 0:
+				_isError = false;
+				break;
+			case 1:
+				_isError = true;
+				break;
+			default:
+				throw new RecordFormatException("Unexpected isError flag ("
+						+ flag + ") for BOOLERR record.");
+		}
 	}
 
 	/**
@@ -52,8 +76,8 @@ public final class BoolErrRecord extends CellRecord {
 	 * @param value   representing the boolean value
 	 */
 	public void setValue(boolean value) {
-		field_4_bBoolErr = value ? ( byte ) 1 : ( byte ) 0;
-		field_5_fError   = ( byte ) 0;
+		_value = value ? 1 : 0;
+		_isError = false;
 	}
 
 	/**
@@ -72,8 +96,8 @@ public final class BoolErrRecord extends CellRecord {
 			case ErrorConstants.ERROR_NAME:
 			case ErrorConstants.ERROR_NUM:
 			case ErrorConstants.ERROR_NA:
-				field_4_bBoolErr = value;
-				field_5_fError   = ( byte ) 1;
+				_value = value;
+				_isError = true;
 				return;
 		}
 		throw new IllegalArgumentException("Error Value can only be 0,7,15,23,29,36 or 42. It cannot be "+value);
@@ -85,7 +109,7 @@ public final class BoolErrRecord extends CellRecord {
 	 * @return boolean representing the boolean value
 	 */
 	public boolean getBooleanValue() {
-		return (field_4_bBoolErr != 0);
+		return _value != 0;
 	}
 
 	/**
@@ -94,7 +118,7 @@ public final class BoolErrRecord extends CellRecord {
 	 * @return byte representing the error value
 	 */
 	public byte getErrorValue() {
-		return field_4_bBoolErr;
+		return (byte)_value;
 	}
 
 	/**
@@ -103,14 +127,7 @@ public final class BoolErrRecord extends CellRecord {
 	 * @return boolean true if the cell holds a boolean value
 	 */
 	public boolean isBoolean() {
-		return (field_5_fError == ( byte ) 0);
-	}
-
-	/**
-	 * manually indicate this is an error rather than a boolean
-	 */
-	public void setError(boolean val) {
-		field_5_fError = (byte) (val == false ? 0 : 1);
+		return !_isError;
 	}
 
 	/**
@@ -118,9 +135,8 @@ public final class BoolErrRecord extends CellRecord {
 	 *
 	 * @return boolean true if the cell holds an error value
 	 */
-
 	public boolean isError() {
-		return field_5_fError != 0;
+		return _isError;
 	}
 
 	@Override
@@ -140,8 +156,8 @@ public final class BoolErrRecord extends CellRecord {
 	}
 	@Override
 	protected void serializeValue(LittleEndianOutput out) {
-		out.writeByte(field_4_bBoolErr);
-		out.writeByte(field_5_fError);
+		out.writeByte(_value);
+		out.writeByte(_isError ? 1 : 0);
 	}
 
 	@Override
@@ -156,8 +172,8 @@ public final class BoolErrRecord extends CellRecord {
 	public Object clone() {
 	  BoolErrRecord rec = new BoolErrRecord();
 	  copyBaseFields(rec);
-	  rec.field_4_bBoolErr = field_4_bBoolErr;
-	  rec.field_5_fError = field_5_fError;
+	  rec._value = _value;
+	  rec._isError = _isError;
 	  return rec;
 	}
 }
diff --git a/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java b/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java
index e7d8c7f4a5..afd4893137 100755
--- a/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java
+++ b/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java
@@ -43,6 +43,7 @@ public final class AllRecordTests {
 		result.addTest(AllRecordAggregateTests.suite());
 
 		result.addTestSuite(TestBOFRecord.class);
+		result.addTestSuite(TestBoolErrRecord.class);
 		result.addTestSuite(TestBoundSheetRecord.class);
 		result.addTestSuite(TestCFHeaderRecord.class);
 		result.addTestSuite(TestCFRuleRecord.class);
diff --git a/src/testcases/org/apache/poi/hssf/record/TestBoolErrRecord.java b/src/testcases/org/apache/poi/hssf/record/TestBoolErrRecord.java
new file mode 100644
index 0000000000..2c90208399
--- /dev/null
+++ b/src/testcases/org/apache/poi/hssf/record/TestBoolErrRecord.java
@@ -0,0 +1,85 @@
+/* ====================================================================
+   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;
+
+import java.util.Arrays;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.record.RecordInputStream.LeftoverDataException;
+import org.apache.poi.util.HexRead;
+/**
+ * Tests for {@link BoolErrRecord}
+ */
+public final class TestBoolErrRecord extends TestCase {
+
+	public void testError() {
+		byte[] data = HexRead.readFromString(
+				"00 00 00 00 0F 00 " + // row, col, xfIndex
+				"07 01 " // #DIV/0!, isError
+				);
+
+		RecordInputStream in = TestcaseRecordInputStream.create(BoolErrRecord.sid, data);
+		BoolErrRecord ber = new BoolErrRecord(in);
+		assertTrue(ber.isError());
+		assertEquals(7, ber.getErrorValue());
+		
+		TestcaseRecordInputStream.confirmRecordEncoding(BoolErrRecord.sid, data, ber.serialize());
+	}
+
+	/**
+	 * Bugzilla 47479 was due to an apparent error in OOO which (as of version 3.0.1) 
+	 * writes the <i>value</i> field of BOOLERR records as 2 bytes instead of 1.<br/>
+	 * Coincidentally, the extra byte written is zero, which is exactly the value 
+	 * required by the <i>isError</i> field.  This probably why Excel seems to have
+	 * no problem.  OOO does not have the same bug for error values (which wouldn't
+	 * work by the same coincidence). 
+	 */
+	public void testOooBadFormat_bug47479() {
+		byte[] data = HexRead.readFromString(
+				"05 02 09 00 " + // sid, size
+				"00 00 00 00 0F 00 " + // row, col, xfIndex
+				"01 00 00 " // extra 00 byte here
+				);
+
+		RecordInputStream in = TestcaseRecordInputStream.create(data);
+		BoolErrRecord ber = new BoolErrRecord(in);
+		boolean hasMore;
+		try {
+			hasMore = in.hasNextRecord();
+		} catch (LeftoverDataException e) {
+			if ("Initialisation of record 0x205 left 1 bytes remaining still to be read.".equals(e.getMessage())) {
+				throw new AssertionFailedError("Identified bug 47479");
+			}
+			throw e;
+		}
+		assertFalse(hasMore);
+		assertTrue(ber.isBoolean());
+		assertEquals(true, ber.getBooleanValue());
+		
+		// Check that the record re-serializes correctly
+		byte[] outData = ber.serialize();
+		byte[] expData = HexRead.readFromString(
+				"05 02 08 00 " +
+				"00 00 00 00 0F 00 " +
+				"01 00 " // normal number of data bytes
+				);
+		assertTrue(Arrays.equals(expData, outData));
+	}
+}