From 9ba354156d28a5bf69fe4d705d9f264c8537e5a9 Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Mon, 6 Jul 2009 19:59:32 +0000 Subject: [PATCH] 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 --- src/documentation/content/xdocs/status.xml | 1 + .../apache/poi/hssf/record/BoolErrRecord.java | 66 ++++++++------ .../poi/hssf/record/AllRecordTests.java | 1 + .../poi/hssf/record/TestBoolErrRecord.java | 85 +++++++++++++++++++ 4 files changed, 128 insertions(+), 25 deletions(-) create mode 100644 src/testcases/org/apache/poi/hssf/record/TestBoolErrRecord.java diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 816028b3f..2b2da218e 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -33,6 +33,7 @@ + 47479 - Fix BoolErrRecord to tolerate incorrect format written by OOO 47448 - Allow HSSFEventFactory to handle non-zero padding at the end of the workbook stream 47456 - Support for getting OLE object data in PowerPointExtractor 47411 - Explicitly set the 1900 date system when creating XSSF workbooks diff --git a/src/java/org/apache/poi/hssf/record/BoolErrRecord.java b/src/java/org/apache/poi/hssf/record/BoolErrRecord.java index 9aa862f7d..580dc782e 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 true, 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 e7d8c7f4a..afd489313 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 000000000..2c9020839 --- /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 value field of BOOLERR records as 2 bytes instead of 1.
+ * Coincidentally, the extra byte written is zero, which is exactly the value + * required by the isError 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)); + } +}