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

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.5-beta6" date="2009-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">47001 - Fixed WriteAccessRecord and LinkTable to handle unusual format written by Google Docs</action>
            <action dev="POI-DEVELOPERS" type="fix">46973 - Fixed defined names to behave better when refersToFormula is unset</action>
            <action dev="POI-DEVELOPERS" type="fix">46832 - Allow merged regions with columns greater than 255 or rows bigger than 65536 in XSSF</action>
            <action dev="POI-DEVELOPERS" type="fix">46951 - Fixed formula parser to better handle range operators and whole row/column refs.</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.5-beta6" date="2009-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">47001 - Fixed WriteAccessRecord and LinkTable to handle unusual format written by Google Docs</action>
            <action dev="POI-DEVELOPERS" type="fix">46973 - Fixed defined names to behave better when refersToFormula is unset</action>
            <action dev="POI-DEVELOPERS" type="fix">46832 - Allow merged regions with columns greater than 255 or rows bigger than 65536 in XSSF</action>
            <action dev="POI-DEVELOPERS" type="fix">46951 - Fixed formula parser to better handle range operators and whole row/column refs.</action>

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

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() {

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

src/testcases/org/apache/poi/hssf/usermodel/TestLinkTable.java → 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<Record> 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);
+	}	
 }

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;

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

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

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