diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index a653d3c00..5d842b02a 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 42564 - fixed ArrayPtg to use ConstantValueParser. Fixed a few other ArrayPtg encoding issues. Follow-on from 28754 - StringPtg.toFormulaString() should escape double quotes 44929 - Improved error handling in HSSFWorkbook when attempting to read a BIFF5 file 44675 - Parameter operand classes (function metadata) required to encode SUM() etc properly. Added parse validation for number of parameters diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index f2b9fb8e1..97269693e 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 42564 - fixed ArrayPtg to use ConstantValueParser. Fixed a few other ArrayPtg encoding issues. Follow-on from 28754 - StringPtg.toFormulaString() should escape double quotes 44929 - Improved error handling in HSSFWorkbook when attempting to read a BIFF5 file 44675 - Parameter operand classes (function metadata) required to encode SUM() etc properly. Added parse validation for number of parameters 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 7d44b008f..12f26bfde 100755 --- a/src/java/org/apache/poi/hssf/record/constant/ConstantValueParser.java +++ b/src/java/org/apache/poi/hssf/record/constant/ConstantValueParser.java @@ -24,9 +24,8 @@ import org.apache.poi.util.LittleEndian; /** * To support Constant Values (2.5.7) as required by the CRN record. - * This class should probably also be used for two dimensional arrays which are encoded by + * This class is also used for two dimensional arrays which are encoded by * EXTERNALNAME (5.39) records and Array tokens.

- * TODO - code in ArrayPtg should be merged with this code. It currently supports only 2 of the constant types * * @author Josh Micich */ diff --git a/src/java/org/apache/poi/hssf/record/constant/ErrorConstant.java b/src/java/org/apache/poi/hssf/record/constant/ErrorConstant.java index 2fc79a948..3421dd4a8 100644 --- a/src/java/org/apache/poi/hssf/record/constant/ErrorConstant.java +++ b/src/java/org/apache/poi/hssf/record/constant/ErrorConstant.java @@ -47,18 +47,31 @@ public class ErrorConstant { public int getErrorCode() { return _errorCode; } + public String getText() { + if(HSSFErrorConstants.isValidCode(_errorCode)) { + return HSSFErrorConstants.getText(_errorCode); + } + return "unknown error code (" + _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; + 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); } + public String toString() { + StringBuffer sb = new StringBuffer(64); + sb.append(getClass().getName()).append(" ["); + sb.append(getText()); + sb.append("]"); + return sb.toString(); + } } diff --git a/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java b/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java index 01942be55..251009b28 100644 --- a/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java +++ b/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java @@ -17,22 +17,17 @@ package org.apache.poi.hssf.record.formula; -import org.apache.poi.util.LittleEndian; -import org.apache.poi.util.BitField; -import org.apache.poi.util.BitFieldFactory; -import org.apache.poi.util.StringUtil; - -import org.apache.poi.hssf.util.CellReference; -import org.apache.poi.hssf.usermodel.HSSFWorkbook; -import org.apache.poi.hssf.record.RecordFormatException; import org.apache.poi.hssf.record.RecordInputStream; -import org.apache.poi.hssf.record.SSTRecord; import org.apache.poi.hssf.record.UnicodeString; +import org.apache.poi.hssf.record.constant.ConstantValueParser; +import org.apache.poi.hssf.record.constant.ErrorConstant; +import org.apache.poi.hssf.usermodel.HSSFWorkbook; +import org.apache.poi.util.LittleEndian; /** * ArrayPtg - handles arrays * - * The ArrayPtg is a little wierd, the size of the Ptg when parsing initially only + * The ArrayPtg is a little weird, the size of the Ptg when parsing initially only * includes the Ptg sid and the reserved bytes. The next Ptg in the expression then follows. * It is only after the "size" of all the Ptgs is met, that the ArrayPtg data is actually * held after this. So Ptg.createParsedExpression keeps track of the number of @@ -40,209 +35,160 @@ import org.apache.poi.hssf.record.UnicodeString; * * @author Jason Height (jheight at chariot dot net dot au) */ +public class ArrayPtg extends Ptg { + public static final byte sid = 0x20; -public class ArrayPtg extends Ptg -{ - public final static byte sid = 0x20; - protected byte field_1_reserved; - protected byte field_2_reserved; - protected byte field_3_reserved; - protected byte field_4_reserved; - protected byte field_5_reserved; - protected byte field_6_reserved; - protected byte field_7_reserved; - - - protected short token_1_columns; - protected short token_2_rows; - protected Object[][] token_3_arrayValues; + private static final int RESERVED_FIELD_LEN = 7; + // TODO - fix up field visibility and subclasses + protected byte[] field_1_reserved; + // data from these fields comes after the Ptg data of all tokens in current formula + protected short token_1_columns; + protected short token_2_rows; + protected Object[] token_3_arrayValues; - protected ArrayPtg() { - //Required for clone methods - } + protected ArrayPtg() { + //Required for clone methods + } - public ArrayPtg(RecordInputStream in) - { - field_1_reserved = in.readByte(); - field_2_reserved = in.readByte(); - field_3_reserved = in.readByte(); - field_4_reserved = in.readByte(); - field_5_reserved = in.readByte(); - field_6_reserved = in.readByte(); - field_7_reserved = in.readByte(); - } - - /** - * Read in the actual token (array) values. This occurs - * AFTER the last Ptg in the expression. - * See page 304-305 of Excel97-2007BinaryFileFormat(xls)Specification.pdf - */ - public void readTokenValues(RecordInputStream in) { - token_1_columns = (short)(0x00ff & in.readByte()); - token_2_rows = in.readShort(); - - //The token_1_columns and token_2_rows do not follow the documentation. - //The number of physical rows and columns is actually +1 of these values. - //Which is not explicitly documented. - token_1_columns++; - token_2_rows++; - - token_3_arrayValues = new Object[token_1_columns][token_2_rows]; - - for (int x=0;x= token_1_columns) { + throw new IllegalArgumentException("Specified colIx (" + colIx + + ") is outside the allowed range (0.." + (token_1_columns-1) + ")"); + } + if(rowIx < 0 || rowIx >= token_2_rows) { + throw new IllegalArgumentException("Specified rowIx (" + rowIx + + ") is outside the allowed range (0.." + (token_2_rows-1) + ")"); + } + return rowIx * token_1_columns + colIx; + } - buffer.append("columns = ").append(getColumnCount()).append("\n"); - buffer.append("rows = ").append(getRowCount()).append("\n"); - for (int x=0;x 0) { + b.append(";"); + } + for (int y=0;y 0) { + b.append(","); + } + Object o = token_3_arrayValues[getValueIndex(x, y)]; + b.append(getConstantText(o)); + } + } + b.append("}"); + return b.toString(); + } + + private static String getConstantText(Object o) { - public String toFormulaString(HSSFWorkbook book) - { - StringBuffer b = new StringBuffer(); - b.append("{"); - for (int x=0;xArrayPtg + * + * @author Josh Micich + */ +public final class TestArrayPtg extends TestCase { + + private static final byte[] ENCODED_PTG_DATA = { + 0x40, 0x00, + 0x08, 0x00, + 0, 0, 0, 0, 0, 0, 0, 0, + }; + private static final byte[] ENCODED_CONSTANT_DATA = { + 2, // 3 columns + 1, 0, // 2 rows + 4, 1, 0, 0, 0, 0, 0, 0, 0, // TRUE + 2, 4, 0, 0, 65, 66, 67, 68, // "ABCD" + 2, 1, 0, 0, 69, // "E" + 1, 0, 0, 0, 0, 0, 0, 0, 0, // 0 + 4, 0, 0, 0, 0, 0, 0, 0, 0, // FALSE + 2, 2, 0, 0, 70, 71, // "FG" + }; + + /** + * Lots of problems with ArrayPtg's encoding of + */ + public void testReadWriteTokenValueBytes() { + + ArrayPtg ptg = new ArrayPtgV(new TestcaseRecordInputStream(ArrayPtgV.sid, ENCODED_PTG_DATA)); + + ptg.readTokenValues(new TestcaseRecordInputStream(0, ENCODED_CONSTANT_DATA)); + assertEquals(3, ptg.getColumnCount()); + assertEquals(2, ptg.getRowCount()); + Object[] values = ptg.token_3_arrayValues; + assertEquals(6, values.length); + + + assertEquals(Boolean.TRUE, values[0]); + assertEquals(new UnicodeString("ABCD"), values[1]); + assertEquals(new Double(0), values[3]); + assertEquals(Boolean.FALSE, values[4]); + assertEquals(new UnicodeString("FG"), values[5]); + + byte[] outBuf = new byte[ENCODED_CONSTANT_DATA.length]; + ptg.writeTokenValueBytes(outBuf, 0); + + if(outBuf[0] == 4) { + throw new AssertionFailedError("Identified bug 42564b"); + } + assertTrue(Arrays.equals(ENCODED_CONSTANT_DATA, outBuf)); + } + + /** + * make sure constant elements are stored row by row + */ + public void testElementOrdering() { + ArrayPtg ptg = new ArrayPtgV(new TestcaseRecordInputStream(ArrayPtgV.sid, ENCODED_PTG_DATA)); + ptg.readTokenValues(new TestcaseRecordInputStream(0, ENCODED_CONSTANT_DATA)); + assertEquals(3, ptg.getColumnCount()); + assertEquals(2, ptg.getRowCount()); + + assertEquals(0, ptg.getValueIndex(0, 0)); + assertEquals(1, ptg.getValueIndex(1, 0)); + assertEquals(2, ptg.getValueIndex(2, 0)); + assertEquals(3, ptg.getValueIndex(0, 1)); + assertEquals(4, ptg.getValueIndex(1, 1)); + assertEquals(5, ptg.getValueIndex(2, 1)); + } +} diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index e0a2bde24..b87510c24 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java @@ -732,7 +732,7 @@ public final class TestBugs extends TestCase { * with the NameRecord, once you get past the BOFRecord * issue. */ - public void DISABLEDtest42564Alt() { + public void test42564Alt() { HSSFWorkbook wb = openSample("42564-2.xls"); writeOutAndReadBack(wb); }