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

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.0.3-beta1" date="2008-04-??">
+           <action dev="POI-DEVELOPERS" type="fix">44792 - fixed encode/decode problems in ExternalNameRecord and CRNRecord.</action>
            <action dev="POI-DEVELOPERS" type="fix">43670, 44501 - Fix how HDGF deals with trailing data in the list of chunk headers</action>
            <action dev="POI-DEVELOPERS" type="add">30311 - More work on Conditional Formatting</action>
            <action dev="POI-DEVELOPERS" type="fix">refactored all junits' usage of HSSF.testdata.path to one place</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.0.3-beta1" date="2008-04-??">
+           <action dev="POI-DEVELOPERS" type="fix">44792 - fixed encode/decode problems in ExternalNameRecord and CRNRecord.</action>
            <action dev="POI-DEVELOPERS" type="fix">43670, 44501 - Fix how HDGF deals with trailing data in the list of chunk headers</action>
            <action dev="POI-DEVELOPERS" type="add">30311 - More work on Conditional Formatting</action>
            <action dev="POI-DEVELOPERS" type="fix">refactored all junits' usage of HSSF.testdata.path to one place</action>

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();
 	}
 

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
+		int result = 3 * 2  // 3 short fields
-			+ 2 + field_4_name.length() // nameLen and name
+			+ 2 + field_4_name.length(); // nameLen and name
-			+ 2 + getNameDefinitionSize(); // nameDefLen and nameDef
+		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();
+		if(hasFormula()) {
-		LittleEndian.putShort( data, 12 + nameLen + offset, defLen );
+			short defLen = (short) getNameDefinitionSize();
-		Ptg.serializePtgStack(toStack(field_5_name_definition), data, 14 + nameLen + offset );
+			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_2_index       = in.readShort();
-		field_3_not_used	= in.readShort();
+		field_3_not_used    = in.readShort();
-		short nameLength	= in.readShort();
+		short nameLength    = in.readShort();
-		field_4_name		= in.readCompressedUnicode(nameLength);
+		field_4_name = in.readCompressedUnicode(nameLength);
-		short formulaLen	= in.readShort();
+		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()];

src/java/org/apache/poi/hssf/record/constant/ConstantValueParser.java

@@ -20,6 +20,7 @@ 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];
+		Object[] result = new Object[nValues];
-        for (int i = 0; i < result.length; i++) {
+		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();
+		byte val = (byte)in.readLong(); // 7 bytes 'not used'
-		in.readLong(); // 8 byte 'not used' field
 		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() + "'");
+	}
 }

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. <p/>
+ * 
+ * This class is a type-safe wrapper for a 16-bit int value performing a similar job to 
+ * <tt>ErrorEval</tt>.
+ * 
+ * @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);
+	}
+}

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;
 	}
 }

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();
+		ExternalNameRecord enr = createSimpleENR(dataFDS);
-		assertEquals( "FDS", enr.getText());
+		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());
+	}
 }

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);
+	}
+}