From 685bd7476a58840407d2c8d6de24643f216d1cae Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Thu, 10 Apr 2008 07:06:55 +0000 Subject: [PATCH] bugzilla 44792 - fixed encode/decode problems in ExternalNameRecord and CRNRecord. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@646666 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/changes.xml | 1 + src/documentation/content/xdocs/status.xml | 1 + .../org/apache/poi/hssf/record/CRNRecord.java | 3 +- .../poi/hssf/record/ExternalNameRecord.java | 82 +++++++++++++++---- .../record/constant/ConstantValueParser.java | 73 ++++++++++++++--- .../hssf/record/constant/ErrorConstant.java | 64 +++++++++++++++ .../poi/hssf/record/AllRecordTests.java | 2 + .../hssf/record/TestExternalNameRecord.java | 62 ++++++++++++-- .../constant/TestConstantValueParser.java | 77 +++++++++++++++++ 9 files changed, 334 insertions(+), 31 deletions(-) create mode 100644 src/java/org/apache/poi/hssf/record/constant/ErrorConstant.java create mode 100644 src/testcases/org/apache/poi/hssf/record/constant/TestConstantValueParser.java diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 84c1156af9..35e5a8f015 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 44792 - fixed encode/decode problems in ExternalNameRecord and CRNRecord. 43670, 44501 - Fix how HDGF deals with trailing data in the list of chunk headers 30311 - More work on Conditional Formatting refactored all junits' usage of HSSF.testdata.path to one place diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index bb7394dd33..c102f37e86 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 44792 - fixed encode/decode problems in ExternalNameRecord and CRNRecord. 43670, 44501 - Fix how HDGF deals with trailing data in the list of chunk headers 30311 - More work on Conditional Formatting refactored all junits' usage of HSSF.testdata.path to one place diff --git a/src/java/org/apache/poi/hssf/record/CRNRecord.java b/src/java/org/apache/poi/hssf/record/CRNRecord.java index 73b9e42dfa..fe66d638b7 100755 --- a/src/java/org/apache/poi/hssf/record/CRNRecord.java +++ b/src/java/org/apache/poi/hssf/record/CRNRecord.java @@ -60,7 +60,7 @@ public final class CRNRecord extends Record { field_3_row_index = in.readShort(); int nValues = field_1_last_column_index - field_2_first_column_index + 1; field_4_constant_values = ConstantValueParser.parse(in, nValues); - } + } public String toString() { @@ -83,6 +83,7 @@ public final class CRNRecord extends Record { LittleEndian.putByte(data, 4 + offset, field_1_last_column_index); LittleEndian.putByte(data, 5 + offset, field_2_first_column_index); LittleEndian.putShort(data, 6 + offset, (short) field_3_row_index); + ConstantValueParser.encode(data, 8 + offset, field_4_constant_values); return getRecordSize(); } diff --git a/src/java/org/apache/poi/hssf/record/ExternalNameRecord.java b/src/java/org/apache/poi/hssf/record/ExternalNameRecord.java index 47be0497da..45960e91af 100755 --- a/src/java/org/apache/poi/hssf/record/ExternalNameRecord.java +++ b/src/java/org/apache/poi/hssf/record/ExternalNameRecord.java @@ -30,10 +30,12 @@ import org.apache.poi.util.StringUtil; */ public final class ExternalNameRecord extends Record { + private static final Ptg[] EMPTY_PTG_ARRAY = { }; + public final static short sid = 0x23; // as per BIFF8. (some old versions used 0x223) private static final int OPT_BUILTIN_NAME = 0x0001; - private static final int OPT_AUTOMATIC_LINK = 0x0002; + private static final int OPT_AUTOMATIC_LINK = 0x0002; // m$ doc calls this fWantAdvise private static final int OPT_PICTURE_LINK = 0x0004; private static final int OPT_STD_DOCUMENT_NAME = 0x0008; private static final int OPT_OLE_LINK = 0x0010; @@ -51,8 +53,8 @@ public final class ExternalNameRecord extends Record { super(in); } - /** - * Convenience Function to determine if the name is a built-in name + /** + * Convenience Function to determine if the name is a built-in name */ public boolean isBuiltInName() { return (field_1_option_flag & OPT_BUILTIN_NAME) != 0; @@ -102,9 +104,12 @@ public final class ExternalNameRecord extends Record { } private int getDataSize(){ - return 3 * 2 // 3 short fields - + 2 + field_4_name.length() // nameLen and name - + 2 + getNameDefinitionSize(); // nameDefLen and nameDef + int result = 3 * 2 // 3 short fields + + 2 + field_4_name.length(); // nameLen and name + if(hasFormula()) { + result += 2 + getNameDefinitionSize(); // nameDefLen and nameDef + } + return result; } /** @@ -127,9 +132,11 @@ public final class ExternalNameRecord extends Record { short nameLen = (short) field_4_name.length(); LittleEndian.putShort( data, 10 + offset, nameLen ); StringUtil.putCompressedUnicode( field_4_name, data, 12 + offset ); - short defLen = (short) getNameDefinitionSize(); - LittleEndian.putShort( data, 12 + nameLen + offset, defLen ); - Ptg.serializePtgStack(toStack(field_5_name_definition), data, 14 + nameLen + offset ); + if(hasFormula()) { + short defLen = (short) getNameDefinitionSize(); + LittleEndian.putShort( data, 12 + nameLen + offset, defLen ); + Ptg.serializePtgStack(toStack(field_5_name_definition), data, 14 + nameLen + offset ); + } return dataSize + 4; } @@ -149,13 +156,58 @@ public final class ExternalNameRecord extends Record { protected void fillFields(RecordInputStream in) { field_1_option_flag = in.readShort(); - field_2_index = in.readShort(); - field_3_not_used = in.readShort(); - short nameLength = in.readShort(); - field_4_name = in.readCompressedUnicode(nameLength); - short formulaLen = in.readShort(); + field_2_index = in.readShort(); + field_3_not_used = in.readShort(); + short nameLength = in.readShort(); + field_4_name = in.readCompressedUnicode(nameLength); + if(!hasFormula()) { + if(in.remaining() > 0) { + throw readFail("Some unread data (is formula present?)"); + } + field_5_name_definition = EMPTY_PTG_ARRAY; + return; + } + if(in.remaining() <= 0) { + throw readFail("Ran out of record data trying to read formula."); + } + short formulaLen = in.readShort(); field_5_name_definition = toPtgArray(Ptg.createParsedExpressionTokens(formulaLen, in)); } + /* + * Makes better error messages (while hasFormula() is not reliable) + * Remove this when hasFormula() is stable. + */ + private RuntimeException readFail(String msg) { + String fullMsg = msg + " fields: (option=" + field_1_option_flag + " index=" + field_2_index + + " not_used=" + field_3_not_used + " name='" + field_4_name + "')"; + return new RuntimeException(fullMsg); + } + + private boolean hasFormula() { + // TODO - determine exact conditions when formula is present + if (false) { + // "Microsoft Office Excel 97-2007 Binary File Format (.xls) Specification" + // m$'s document suggests logic like this, but bugzilla 44774 att 21790 seems to disagree + if (isStdDocumentNameIdentifier()) { + if (isOLELink()) { + // seems to be not possible according to m$ document + throw new IllegalStateException( + "flags (std-doc-name and ole-link) cannot be true at the same time"); + } + return false; + } + if (isOLELink()) { + return false; + } + return true; + } + + // This was derived by trial and error, but doesn't seem quite right + if (isAutomaticLink()) { + return false; + } + return true; + } private static Ptg[] toPtgArray(Stack s) { Ptg[] result = new Ptg[s.size()]; @@ -169,7 +221,7 @@ public final class ExternalNameRecord extends Record { } return result; } - + public short getSid() { return sid; } diff --git a/src/java/org/apache/poi/hssf/record/constant/ConstantValueParser.java b/src/java/org/apache/poi/hssf/record/constant/ConstantValueParser.java index 6ec831e4a0..7d44b008f8 100755 --- a/src/java/org/apache/poi/hssf/record/constant/ConstantValueParser.java +++ b/src/java/org/apache/poi/hssf/record/constant/ConstantValueParser.java @@ -14,12 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. ==================================================================== */ - + package org.apache.poi.hssf.record.constant; import org.apache.poi.hssf.record.RecordInputStream; import org.apache.poi.hssf.record.UnicodeString; import org.apache.poi.hssf.record.UnicodeString.UnicodeRecordStats; +import org.apache.poi.util.LittleEndian; /** * To support Constant Values (2.5.7) as required by the CRN record. @@ -30,11 +31,12 @@ import org.apache.poi.hssf.record.UnicodeString.UnicodeRecordStats; * @author Josh Micich */ public final class ConstantValueParser { - // note - value 3 seems to be unused + // note - these (non-combinable) enum values are sparse. private static final int TYPE_EMPTY = 0; private static final int TYPE_NUMBER = 1; private static final int TYPE_STRING = 2; private static final int TYPE_BOOLEAN = 4; + private static final int TYPE_ERROR_CODE = 16; // TODO - update OOO document to include this value private static final int TRUE_ENCODING = 1; private static final int FALSE_ENCODING = 0; @@ -47,11 +49,11 @@ public final class ConstantValueParser { } public static Object[] parse(RecordInputStream in, int nValues) { - Object[] result = new Object[nValues]; - for (int i = 0; i < result.length; i++) { + Object[] result = new Object[nValues]; + for (int i = 0; i < result.length; i++) { result[i] = readAConstantValue(in); } - return result; + return result; } private static Object readAConstantValue(RecordInputStream in) { @@ -66,13 +68,18 @@ public final class ConstantValueParser { return in.readUnicodeString(); case TYPE_BOOLEAN: return readBoolean(in); + case TYPE_ERROR_CODE: + int errCode = in.readUShort(); + // next 6 bytes are unused + in.readUShort(); + in.readInt(); + return ErrorConstant.valueOf(errCode); } - return null; + throw new RuntimeException("Unknown grbit value (" + grbit + ")"); } private static Object readBoolean(RecordInputStream in) { - byte val = in.readByte(); - in.readLong(); // 8 byte 'not used' field + byte val = (byte)in.readLong(); // 7 bytes 'not used' switch(val) { case FALSE_ENCODING: return Boolean.FALSE; @@ -89,7 +96,7 @@ public final class ConstantValueParser { for (int i = 0; i < values.length; i++) { result += getEncodedSize(values[i]); } - return 0; + return result; } /** @@ -100,7 +107,8 @@ public final class ConstantValueParser { return 8; } Class cls = object.getClass(); - if(cls == Boolean.class || cls == Double.class) { + + if(cls == Boolean.class || cls == Double.class || cls == ErrorConstant.class) { return 8; } UnicodeString strVal = (UnicodeString)object; @@ -108,4 +116,49 @@ public final class ConstantValueParser { strVal.getRecordSize(urs); return urs.recordSize; } + + public static void encode(byte[] data, int offset, Object[] values) { + int currentOffset = offset; + for (int i = 0; i < values.length; i++) { + currentOffset += encodeSingleValue(data, currentOffset, values[i]); + } + } + + private static int encodeSingleValue(byte[] data, int offset, Object value) { + if (value == EMPTY_REPRESENTATION) { + LittleEndian.putByte(data, offset, TYPE_EMPTY); + LittleEndian.putLong(data, offset+1, 0L); + return 9; + } + if (value instanceof Boolean) { + Boolean bVal = ((Boolean)value); + LittleEndian.putByte(data, offset, TYPE_BOOLEAN); + long longVal = bVal.booleanValue() ? 1L : 0L; + LittleEndian.putLong(data, offset+1, longVal); + return 9; + } + if (value instanceof Double) { + Double dVal = (Double) value; + LittleEndian.putByte(data, offset, TYPE_NUMBER); + LittleEndian.putDouble(data, offset+1, dVal.doubleValue()); + return 9; + } + if (value instanceof UnicodeString) { + UnicodeString usVal = (UnicodeString) value; + LittleEndian.putByte(data, offset, TYPE_STRING); + UnicodeRecordStats urs = new UnicodeRecordStats(); + usVal.serialize(urs, offset +1, data); + return 1 + urs.recordSize; + } + if (value instanceof ErrorConstant) { + ErrorConstant ecVal = (ErrorConstant) value; + LittleEndian.putByte(data, offset, TYPE_ERROR_CODE); + LittleEndian.putUShort(data, offset+1, ecVal.getErrorCode()); + LittleEndian.putUShort(data, offset+3, 0); + LittleEndian.putInt(data, offset+5, 0); + return 9; + } + + throw new IllegalStateException("Unexpected value type (" + value.getClass().getName() + "'"); + } } diff --git a/src/java/org/apache/poi/hssf/record/constant/ErrorConstant.java b/src/java/org/apache/poi/hssf/record/constant/ErrorConstant.java new file mode 100644 index 0000000000..2fc79a948f --- /dev/null +++ b/src/java/org/apache/poi/hssf/record/constant/ErrorConstant.java @@ -0,0 +1,64 @@ +/* ==================================================================== + 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.constant; + +import org.apache.poi.hssf.usermodel.HSSFErrorConstants; +/** + * Represents a constant error code value as encoded in a constant values array.

+ * + * This class is a type-safe wrapper for a 16-bit int value performing a similar job to + * ErrorEval. + * + * @author Josh Micich + */ +public class ErrorConstant { + // convenient access to name space + private static final HSSFErrorConstants EC = null; + + private static final ErrorConstant NULL = new ErrorConstant(EC.ERROR_NULL); + private static final ErrorConstant DIV_0 = new ErrorConstant(EC.ERROR_DIV_0); + private static final ErrorConstant VALUE = new ErrorConstant(EC.ERROR_VALUE); + private static final ErrorConstant REF = new ErrorConstant(EC.ERROR_REF); + private static final ErrorConstant NAME = new ErrorConstant(EC.ERROR_NAME); + private static final ErrorConstant NUM = new ErrorConstant(EC.ERROR_NUM); + private static final ErrorConstant NA = new ErrorConstant(EC.ERROR_NA); + + private final int _errorCode; + + private ErrorConstant(int errorCode) { + _errorCode = errorCode; + } + + public int getErrorCode() { + return _errorCode; + } + + public static ErrorConstant valueOf(int errorCode) { + switch (errorCode) { + case HSSFErrorConstants.ERROR_NULL: return NULL; + case HSSFErrorConstants.ERROR_DIV_0: return DIV_0; + case HSSFErrorConstants.ERROR_VALUE: return VALUE; + case HSSFErrorConstants.ERROR_REF: return REF; + case HSSFErrorConstants.ERROR_NAME: return NAME; + case HSSFErrorConstants.ERROR_NUM: return NUM; + case HSSFErrorConstants.ERROR_NA: return NA; + } + System.err.println("Warning - unexpected error code (" + errorCode + ")"); + return new ErrorConstant(errorCode); + } +} diff --git a/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java b/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java index 78128dbe19..d2a1b36841 100755 --- a/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java +++ b/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java @@ -22,6 +22,7 @@ import junit.framework.TestSuite; import org.apache.poi.hssf.record.aggregates.AllRecordAggregateTests; import org.apache.poi.hssf.record.cf.TestCellRange; +import org.apache.poi.hssf.record.constant.TestConstantValueParser; import org.apache.poi.hssf.record.formula.AllFormulaTests; /** @@ -105,6 +106,7 @@ public final class AllRecordTests { result.addTestSuite(TestUnitsRecord.class); result.addTestSuite(TestValueRangeRecord.class); result.addTestSuite(TestCellRange.class); + result.addTestSuite(TestConstantValueParser.class); return result; } } diff --git a/src/testcases/org/apache/poi/hssf/record/TestExternalNameRecord.java b/src/testcases/org/apache/poi/hssf/record/TestExternalNameRecord.java index 3c35b29a26..714c0358ff 100644 --- a/src/testcases/org/apache/poi/hssf/record/TestExternalNameRecord.java +++ b/src/testcases/org/apache/poi/hssf/record/TestExternalNameRecord.java @@ -28,13 +28,25 @@ public final class TestExternalNameRecord extends TestCase { private static final byte[] dataFDS = { 0, 0, 0, 0, 0, 0, 3, 0, 70, 68, 83, 0, 0, }; - private static ExternalNameRecord createSimpleENR() { - return new ExternalNameRecord(new TestcaseRecordInputStream((short)0x0023, dataFDS)); + + // data taken from bugzilla 44774 att 21790 + private static final byte[] dataAutoDocName = { + -22, 127, 0, 0, 0, 0, 29, 0, 39, 49, 57, 49, 50, 49, 57, 65, 87, 52, 32, 67, 111, 114, + 112, 44, 91, 87, 79, 82, 75, 79, 85, 84, 95, 80, 88, 93, 39, + }; + + // data taken from bugzilla 44774 att 21790 + private static final byte[] dataPlainName = { + 0, 0, 0, 0, 0, 0, 9, 0, 82, 97, 116, 101, 95, 68, 97, 116, 101, 9, 0, 58, 0, 0, 0, 0, 4, 0, 8, 0 + }; + + private static ExternalNameRecord createSimpleENR(byte[] data) { + return new ExternalNameRecord(new TestcaseRecordInputStream((short)0x0023, data)); } public void testBasicDeserializeReserialize() { - ExternalNameRecord enr = createSimpleENR(); - assertEquals( "FDS", enr.getText()); + ExternalNameRecord enr = createSimpleENR(dataFDS); + assertEquals("FDS", enr.getText()); try { TestcaseRecordInputStream.confirmRecordEncoding(0x0023, dataFDS, enr.serialize()); @@ -46,10 +58,50 @@ public final class TestExternalNameRecord extends TestCase { } public void testBasicSize() { - ExternalNameRecord enr = createSimpleENR(); + ExternalNameRecord enr = createSimpleENR(dataFDS); if(enr.getRecordSize() == 13) { throw new AssertionFailedError("Identified bug 44695"); } assertEquals(17, enr.getRecordSize()); } + + public void testAutoStdDocName() { + + ExternalNameRecord enr; + try { + enr = createSimpleENR(dataAutoDocName); + } catch (ArrayIndexOutOfBoundsException e) { + if(e.getMessage() == null) { + throw new AssertionFailedError("Identified bug XXXX"); + } + throw e; + } + assertEquals("'191219AW4 Corp,[WORKOUT_PX]'", enr.getText()); + assertTrue(enr.isAutomaticLink()); + assertFalse(enr.isBuiltInName()); + assertFalse(enr.isIconifiedPictureLink()); + assertFalse(enr.isInValueSection()); + assertFalse(enr.isOLELink()); + assertFalse(enr.isPicureLink()); + assertTrue(enr.isStdDocumentNameIdentifier()); + assertFalse(enr.isValue()); + + TestcaseRecordInputStream.confirmRecordEncoding(0x0023, dataAutoDocName, enr.serialize()); + } + + public void testPlainName() { + + ExternalNameRecord enr = createSimpleENR(dataPlainName); + assertEquals("Rate_Date", enr.getText()); + assertFalse(enr.isAutomaticLink()); + assertFalse(enr.isBuiltInName()); + assertFalse(enr.isIconifiedPictureLink()); + assertFalse(enr.isInValueSection()); + assertFalse(enr.isOLELink()); + assertFalse(enr.isPicureLink()); + assertFalse(enr.isStdDocumentNameIdentifier()); + assertFalse(enr.isValue()); + + TestcaseRecordInputStream.confirmRecordEncoding(0x0023, dataPlainName, enr.serialize()); + } } diff --git a/src/testcases/org/apache/poi/hssf/record/constant/TestConstantValueParser.java b/src/testcases/org/apache/poi/hssf/record/constant/TestConstantValueParser.java new file mode 100644 index 0000000000..52c6f679d7 --- /dev/null +++ b/src/testcases/org/apache/poi/hssf/record/constant/TestConstantValueParser.java @@ -0,0 +1,77 @@ +/* ==================================================================== + 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.constant; + +import java.util.Arrays; + +import junit.framework.TestCase; + +import org.apache.poi.hssf.record.RecordInputStream; +import org.apache.poi.hssf.record.TestcaseRecordInputStream; +import org.apache.poi.hssf.record.UnicodeString; +import org.apache.poi.hssf.usermodel.HSSFErrorConstants; +/** + * + * @author Josh Micich + */ +public final class TestConstantValueParser extends TestCase { + private static final Object[] SAMPLE_VALUES = { + Boolean.TRUE, + null, + new Double(1.1), + new UnicodeString("Sample text"), + ErrorConstant.valueOf(HSSFErrorConstants.ERROR_DIV_0), + }; + private static final byte[] SAMPLE_ENCODING = { + 4, 1, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, + 1, -102, -103, -103, -103, -103, -103, -15, 63, + 2, 11, 0, 0, 83, 97, 109, 112, 108, 101, 32, 116, 101, 120, 116, + 16, 7, 0, 0, 0, 0, 0, 0, 0, + }; + + public void testGetEncodedSize() { + int actual = ConstantValueParser.getEncodedSize(SAMPLE_VALUES); + assertEquals(51, actual); + } + public void testEncode() { + int size = ConstantValueParser.getEncodedSize(SAMPLE_VALUES); + byte[] data = new byte[size]; + ConstantValueParser.encode(data, 0, SAMPLE_VALUES); + + if (!Arrays.equals(data, SAMPLE_ENCODING)) { + fail("Encoding differs"); + } + } + public void testDecode() { + RecordInputStream in = new TestcaseRecordInputStream(0x0001, SAMPLE_ENCODING); + + Object[] values = ConstantValueParser.parse(in, 4); + for (int i = 0; i < values.length; i++) { + if(!isEqual(SAMPLE_VALUES[i], values[i])) { + fail("Decoded result differs"); + } + } + } + private static boolean isEqual(Object a, Object b) { + if (a == null) { + return b == null; + } + return a.equals(b); + } +} -- 2.39.5