From 9fa24eea2c441ef5ca6968ed2cf6dc403db8f722 Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Wed, 8 Apr 2009 21:01:43 +0000 Subject: [PATCH] Bugzilla 47001 - Fixed WriteAccessRecord and LinkTable to handle unusual format written by Google Docs git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@763391 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/changes.xml | 1 + src/documentation/content/xdocs/status.xml | 1 + .../org/apache/poi/hssf/model/LinkTable.java | 7 +- .../poi/hssf/record/WriteAccessRecord.java | 49 ++++++--- .../data/{42564.xls => ex42564-21435.xls} | Bin .../data/{42564-2.xls => ex42564-21503.xls} | Bin .../apache/poi/hssf/model/AllModelTests.java | 1 + .../{usermodel => model}/TestLinkTable.java | 38 ++++++- .../poi/hssf/record/AllRecordTests.java | 1 + .../hssf/record/TestWriteAccessRecord.java | 103 ++++++++++++++++++ .../poi/hssf/usermodel/AllUserModelTests.java | 1 - .../apache/poi/hssf/usermodel/TestBugs.java | 4 +- 12 files changed, 183 insertions(+), 23 deletions(-) rename src/testcases/org/apache/poi/hssf/data/{42564.xls => ex42564-21435.xls} (100%) rename src/testcases/org/apache/poi/hssf/data/{42564-2.xls => ex42564-21503.xls} (100%) rename src/testcases/org/apache/poi/hssf/{usermodel => model}/TestLinkTable.java (80%) create mode 100644 src/testcases/org/apache/poi/hssf/record/TestWriteAccessRecord.java diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 4929d519e6..e2f50b9ad6 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 47001 - Fixed WriteAccessRecord and LinkTable to handle unusual format written by Google Docs 46973 - Fixed defined names to behave better when refersToFormula is unset 46832 - Allow merged regions with columns greater than 255 or rows bigger than 65536 in XSSF 46951 - Fixed formula parser to better handle range operators and whole row/column refs. diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 8077cd8de2..99095f13f2 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 47001 - Fixed WriteAccessRecord and LinkTable to handle unusual format written by Google Docs 46973 - Fixed defined names to behave better when refersToFormula is unset 46832 - Allow merged regions with columns greater than 255 or rows bigger than 65536 in XSSF 46951 - Fixed formula parser to better handle range operators and whole row/column refs. diff --git a/src/java/org/apache/poi/hssf/model/LinkTable.java b/src/java/org/apache/poi/hssf/model/LinkTable.java index f587f5e375..f5c44ce5c9 100755 --- a/src/java/org/apache/poi/hssf/model/LinkTable.java +++ b/src/java/org/apache/poi/hssf/model/LinkTable.java @@ -161,7 +161,12 @@ final class LinkTable { if (_externalBookBlocks.length > 0) { // If any ExternalBookBlock present, there is always 1 of ExternSheetRecord - _externSheetRecord = readExtSheetRecord(rs); + if (rs.peekNextClass() != ExternSheetRecord.class) { + // not quite - if written by google docs + _externSheetRecord = null; + } else { + _externSheetRecord = readExtSheetRecord(rs); + } } else { _externSheetRecord = null; } diff --git a/src/java/org/apache/poi/hssf/record/WriteAccessRecord.java b/src/java/org/apache/poi/hssf/record/WriteAccessRecord.java index 3ff094e7f6..f87373b1ab 100644 --- a/src/java/org/apache/poi/hssf/record/WriteAccessRecord.java +++ b/src/java/org/apache/poi/hssf/record/WriteAccessRecord.java @@ -19,6 +19,7 @@ package org.apache.poi.hssf.record; import java.util.Arrays; +import org.apache.poi.util.LittleEndian; import org.apache.poi.util.LittleEndianOutput; import org.apache.poi.util.StringUtil; @@ -40,11 +41,13 @@ public final class WriteAccessRecord extends StandardRecord { private static final int DATA_SIZE = 112; private String field_1_username; /** this record is always padded to a constant length */ - private byte[] padding; + private static final byte[] PADDING = new byte[DATA_SIZE]; + static { + Arrays.fill(PADDING, PAD_CHAR); + } public WriteAccessRecord() { setUsername(""); - padding = new byte[DATA_SIZE - 3]; } public WriteAccessRecord(RecordInputStream in) { @@ -57,21 +60,33 @@ public final class WriteAccessRecord extends StandardRecord { int nChars = in.readUShort(); int is16BitFlag = in.readUByte(); - int expectedPadSize = DATA_SIZE - 3; + if (nChars > DATA_SIZE || (is16BitFlag & 0xFE) != 0) { + // String header looks wrong (probably missing) + // OOO doc says this is optional anyway. + // reconstruct data + byte[] data = new byte[3 + in.remaining()]; + LittleEndian.putUShort(data, 0, nChars); + LittleEndian.putByte(data, 2, is16BitFlag); + in.readFully(data, 3, data.length-3); + String rawValue = new String(data); + setUsername(rawValue.trim()); + return; + } + + String rawText; if ((is16BitFlag & 0x01) == 0x00) { - field_1_username = StringUtil.readCompressedUnicode(in, nChars); - expectedPadSize -= nChars; + rawText = StringUtil.readCompressedUnicode(in, nChars); } else { - field_1_username = StringUtil.readUnicodeLE(in, nChars); - expectedPadSize -= nChars * 2; + rawText = StringUtil.readUnicodeLE(in, nChars); } - padding = new byte[expectedPadSize]; + field_1_username = rawText.trim(); + + // consume padding int padSize = in.remaining(); - in.readFully(padding, 0, padSize); - if (padSize < expectedPadSize) { - // this occurs in a couple of test examples: "42564.xls", - // "bug_42794.xls" - Arrays.fill(padding, padSize, expectedPadSize, PAD_CHAR); + while (padSize > 0) { + // in some cases this seems to be garbage (non spaces) + in.readUByte(); + padSize--; } } @@ -88,8 +103,6 @@ public final class WriteAccessRecord extends StandardRecord { if (paddingSize < 0) { throw new IllegalArgumentException("Name is too long: " + username); } - padding = new byte[paddingSize]; - Arrays.fill(padding, PAD_CHAR); field_1_username = username; } @@ -109,7 +122,7 @@ public final class WriteAccessRecord extends StandardRecord { StringBuffer buffer = new StringBuffer(); buffer.append("[WRITEACCESS]\n"); - buffer.append(" .name = ").append(field_1_username.toString()).append("\n"); + buffer.append(" .name = ").append(field_1_username.toString()).append("\n"); buffer.append("[/WRITEACCESS]\n"); return buffer.toString(); } @@ -125,7 +138,9 @@ public final class WriteAccessRecord extends StandardRecord { } else { StringUtil.putCompressedUnicode(username, out); } - out.write(padding); + int encodedByteCount = 3 + username.length() * (is16bit ? 2 : 1); + int paddingSize = DATA_SIZE - encodedByteCount; + out.write(PADDING, 0, paddingSize); } protected int getDataSize() { diff --git a/src/testcases/org/apache/poi/hssf/data/42564.xls b/src/testcases/org/apache/poi/hssf/data/ex42564-21435.xls similarity index 100% rename from src/testcases/org/apache/poi/hssf/data/42564.xls rename to src/testcases/org/apache/poi/hssf/data/ex42564-21435.xls diff --git a/src/testcases/org/apache/poi/hssf/data/42564-2.xls b/src/testcases/org/apache/poi/hssf/data/ex42564-21503.xls similarity index 100% rename from src/testcases/org/apache/poi/hssf/data/42564-2.xls rename to src/testcases/org/apache/poi/hssf/data/ex42564-21503.xls diff --git a/src/testcases/org/apache/poi/hssf/model/AllModelTests.java b/src/testcases/org/apache/poi/hssf/model/AllModelTests.java index 35565a7a65..a243226867 100755 --- a/src/testcases/org/apache/poi/hssf/model/AllModelTests.java +++ b/src/testcases/org/apache/poi/hssf/model/AllModelTests.java @@ -34,6 +34,7 @@ public final class AllModelTests { result.addTestSuite(TestFormulaParser.class); result.addTestSuite(TestFormulaParserEval.class); result.addTestSuite(TestFormulaParserIf.class); + result.addTestSuite(TestLinkTable.class); result.addTestSuite(TestOperandClassTransformer.class); result.addTestSuite(TestRowBlocksReader.class); result.addTestSuite(TestRVA.class); diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestLinkTable.java b/src/testcases/org/apache/poi/hssf/model/TestLinkTable.java similarity index 80% rename from src/testcases/org/apache/poi/hssf/usermodel/TestLinkTable.java rename to src/testcases/org/apache/poi/hssf/model/TestLinkTable.java index 5a9696096b..f88cd4860f 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestLinkTable.java +++ b/src/testcases/org/apache/poi/hssf/model/TestLinkTable.java @@ -15,12 +15,20 @@ limitations under the License. ==================================================================== */ -package org.apache.poi.hssf.usermodel; +package org.apache.poi.hssf.model; + +import java.util.Arrays; +import java.util.List; import junit.framework.AssertionFailedError; import junit.framework.TestCase; import org.apache.poi.hssf.HSSFTestDataSamples; +import org.apache.poi.hssf.record.Record; +import org.apache.poi.hssf.record.SSTRecord; +import org.apache.poi.hssf.record.SupBookRecord; +import org.apache.poi.hssf.usermodel.HSSFCell; +import org.apache.poi.hssf.usermodel.HSSFWorkbook; /** * Tests for {@link LinkTable} * @@ -81,7 +89,7 @@ public final class TestLinkTable extends TestCase { The original file produces the same error. This bug was caused by a combination of invalid sheet indexes in the EXTERNSHEET - record, and eager initialisation of the extern sheet references. Note - the worbook + record, and eager initialisation of the extern sheet references. Note - the workbook has 2 sheets, but the EXTERNSHEET record refers to sheet indexes 0, 1 and 2. Offset 0x3954 (14676) @@ -114,4 +122,30 @@ public final class TestLinkTable extends TestCase { } assertEquals("Data!$A2", cellFormula); } + + /** + * This problem was visible in POI svn r763332 + * when reading the workbook of attachment 23468 from bugzilla 47001 + */ + public void testMissingExternSheetRecord_bug47001b() { + + Record[] recs = { + SupBookRecord.createAddInFunctions(), + new SSTRecord(), + }; + List recList = Arrays.asList(recs); + WorkbookRecordList wrl = new WorkbookRecordList(); + + LinkTable lt; + try { + lt = new LinkTable(recList, 0, wrl); + } catch (RuntimeException e) { + if (e.getMessage().equals("Expected an EXTERNSHEET record but got (org.apache.poi.hssf.record.SSTRecord)")) { + throw new AssertionFailedError("Identified bug 47001b"); + } + + throw e; + } + assertNotNull(lt); + } } diff --git a/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java b/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java index 3458051ee7..e7d8c7f4a5 100755 --- a/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java +++ b/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java @@ -80,6 +80,7 @@ public final class AllRecordTests { result.addTestSuite(TestTextObjectRecord.class); result.addTestSuite(TestUnicodeNameRecord.class); result.addTestSuite(TestUnicodeString.class); + result.addTestSuite(TestWriteAccessRecord.class); result.addTestSuite(TestCellRange.class); result.addTestSuite(TestConstantValueParser.class); return result; diff --git a/src/testcases/org/apache/poi/hssf/record/TestWriteAccessRecord.java b/src/testcases/org/apache/poi/hssf/record/TestWriteAccessRecord.java new file mode 100644 index 0000000000..8d170c0c4e --- /dev/null +++ b/src/testcases/org/apache/poi/hssf/record/TestWriteAccessRecord.java @@ -0,0 +1,103 @@ +/* ==================================================================== + 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 junit.framework.AssertionFailedError; +import junit.framework.TestCase; + +import org.apache.poi.util.HexRead; + +/** + * Tests for {@link WriteAccessRecord} + * + * @author Josh Micich + */ +public final class TestWriteAccessRecord extends TestCase { + + private static final String HEX_SIXTYFOUR_SPACES = "" + + "20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 " + + "20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 " + + "20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 " + + "20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20"; + + + public void testMissingStringHeader_bug47001a() { + /* + * Data taken from offset 0x0224 in + * attachment 23468 from bugzilla 47001 + */ + byte[] data = HexRead.readFromString("" + + "5C 00 70 00 " + + "4A 61 76 61 20 45 78 63 65 6C 20 41 50 49 20 76 " + + "32 2E 36 2E 34" + + "20 20 20 20 20 20 20 20 20 20 20 " + + "20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 " + + HEX_SIXTYFOUR_SPACES); + + RecordInputStream in = TestcaseRecordInputStream.create(data); + + WriteAccessRecord rec; + try { + rec = new WriteAccessRecord(in); + } catch (RecordFormatException e) { + if (e.getMessage().equals("Not enough data (0) to read requested (1) bytes")) { + throw new AssertionFailedError("Identified bug 47001a"); + } + throw e; + } + assertEquals("Java Excel API v2.6.4", rec.getUsername()); + + + byte[] expectedEncoding = HexRead.readFromString("" + + "15 00 00 4A 61 76 61 20 45 78 63 65 6C 20 41 50 " + + "49 20 76 32 2E 36 2E 34" + + "20 20 20 20 20 20 20 20 " + + "20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 " + + HEX_SIXTYFOUR_SPACES); + + TestcaseRecordInputStream.confirmRecordEncoding(WriteAccessRecord.sid, expectedEncoding, rec.serialize()); + } + + public void testShortRecordWrittenByMSAccess() { + /* + * Data taken from two example files + * ex42564-21435.xls + * bug_42794.xls (from bug 42794 attachment 20429) + * In both cases, this data is found at offset 0x0C1C. + */ + byte[] data = HexRead.readFromString("" + + "5C 00 39 00 " + + "36 00 00 41 20 73 61 74 69 73 66 69 65 64 20 4D " + + "69 63 72 6F 73 6F 66 74 20 4F 66 66 69 63 65 39 " + + "20 55 73 65 72" + + "20 20 20 20 20 20 20 20 20 20 20 " + + "20 20 20 20 20 20 20 20 20"); + + RecordInputStream in = TestcaseRecordInputStream.create(data); + WriteAccessRecord rec = new WriteAccessRecord(in); + assertEquals("A satisfied Microsoft Office9 User", rec.getUsername()); + byte[] expectedEncoding = HexRead.readFromString("" + + "22 00 00 41 20 73 61 74 69 73 66 69 65 64 20 4D " + + "69 63 72 6F 73 6F 66 74 20 4F 66 66 69 63 65 39 " + + "20 55 73 65 72" + + "20 20 20 20 20 20 20 20 20 20 20 " + + HEX_SIXTYFOUR_SPACES); + + TestcaseRecordInputStream.confirmRecordEncoding(WriteAccessRecord.sid, expectedEncoding, rec.serialize()); + } +} diff --git a/src/testcases/org/apache/poi/hssf/usermodel/AllUserModelTests.java b/src/testcases/org/apache/poi/hssf/usermodel/AllUserModelTests.java index 6b3043c072..eec05641af 100755 --- a/src/testcases/org/apache/poi/hssf/usermodel/AllUserModelTests.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/AllUserModelTests.java @@ -59,7 +59,6 @@ public class AllUserModelTests { result.addTestSuite(TestHSSFSheet.class); result.addTestSuite(TestHSSFTextbox.class); result.addTestSuite(TestHSSFWorkbook.class); - result.addTestSuite(TestLinkTable.class); result.addTestSuite(TestHSSFName.class); result.addTestSuite(TestOLE2Embeding.class); result.addTestSuite(TestPOIFSProperties.class); diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index 44063e3c7a..d81441c003 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java @@ -586,7 +586,7 @@ public final class TestBugs extends BaseTestBugzillaIssues { * when reading the BOFRecord */ public void test42564() { - HSSFWorkbook wb = openSample("42564.xls"); + HSSFWorkbook wb = openSample("ex42564-21435.xls"); writeOutAndReadBack(wb); } @@ -596,7 +596,7 @@ public final class TestBugs extends BaseTestBugzillaIssues { * issue. */ public void test42564Alt() { - HSSFWorkbook wb = openSample("42564-2.xls"); + HSSFWorkbook wb = openSample("ex42564-21503.xls"); writeOutAndReadBack(wb); } -- 2.39.5