From db7502182e0e8d9e8135811bdb78f24901e641e6 Mon Sep 17 00:00:00 2001 From: Yegor Kozlov Date: Sun, 12 Jul 2009 07:21:04 +0000 Subject: [PATCH] Fixed HyperlinkRecord to properly handle URL monikers, see Bugzilla 47498 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@793281 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/status.xml | 2 + .../poi/hssf/record/HyperlinkRecord.java | 65 +++++++++++++------ .../poi/hssf/record/TestHyperlinkRecord.java | 39 ++++++++++- .../poi/hssf/usermodel/TestFormulas.java | 4 +- .../poi/hssf/usermodel/TestHSSFHyperlink.java | 22 +++++++ .../poi/hssf/usermodel/TestHSSFSheet.java | 6 +- 6 files changed, 111 insertions(+), 27 deletions(-) diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 2b2da218e..9955b603c 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -33,6 +33,8 @@ + 47498 - Fixed HyperlinkRecord to properly handle URL monikers + 47504 - Fixed XSSFWorkbook to read files with hyperlinks to document locations 47479 - Fix BoolErrRecord to tolerate incorrect format written by OOO 47448 - Allow HSSFEventFactory to handle non-zero padding at the end of the workbook stream 47456 - Support for getting OLE object data in PowerPointExtractor diff --git a/src/java/org/apache/poi/hssf/record/HyperlinkRecord.java b/src/java/org/apache/poi/hssf/record/HyperlinkRecord.java index f94605432..94be338f0 100644 --- a/src/java/org/apache/poi/hssf/record/HyperlinkRecord.java +++ b/src/java/org/apache/poi/hssf/record/HyperlinkRecord.java @@ -433,8 +433,13 @@ public final class HyperlinkRecord extends StandardRecord { _guid = new GUID(in); - if (in.readInt() != 0x00000002) { - throw new RecordFormatException("expected "); // TODO + /** + * streamVersion (4 bytes): An unsigned integer that specifies the version number + * of the serialization implementation used to save this structure. This value MUST equal 2. + */ + int streamVersion = in.readInt(); + if (streamVersion != 0x00000002) { + throw new RecordFormatException("Stream Version must be 0x2 but found " + streamVersion); } _linkOpts = in.readInt(); @@ -448,23 +453,38 @@ public final class HyperlinkRecord extends StandardRecord { _targetFrame = in.readUnicodeLEString(len); } - if ((_linkOpts & HLINK_UNC_PATH) != 0) { + if ((_linkOpts & HLINK_URL) != 0 && (_linkOpts & HLINK_UNC_PATH) != 0) { _moniker = null; int nChars = in.readInt(); _address = in.readUnicodeLEString(nChars); + } - } else if ((_linkOpts & HLINK_URL) != 0) { + if ((_linkOpts & HLINK_URL) != 0 && (_linkOpts & HLINK_UNC_PATH) == 0) { _moniker = new GUID(in); if(URL_MONIKER.equals(_moniker)){ - int fieldSize = in.readInt(); - - if ((_linkOpts & HLINK_TARGET_FRAME) != 0) { - int nChars = fieldSize/2; + int length = in.readInt(); + /** + * The value of length be either the byte size of the url field + * (including the terminating NULL character) or the byte size of the url field plus 24. + * If the value of this field is set to the byte size of the url field, + * then the tail bytes fields are not present. + */ + int remaining = in.remaining(); + if (length == remaining) { + int nChars = length/2; _address = in.readUnicodeLEString(nChars); } else { - int nChars = (fieldSize - TAIL_SIZE)/2; + int nChars = (length - TAIL_SIZE)/2; _address = in.readUnicodeLEString(nChars); + /** + * TODO: make sense of the remaining bytes + * According to the spec they consist of: + * 1. 16-byte GUID: This field MUST equal + * {0xF4815879, 0x1D3B, 0x487F, 0xAF, 0x2C, 0x82, 0x5D, 0xC4, 0x85, 0x27, 0x63} + * 2. Serial version, this field MUST equal 0 if present. + * 3. URI Flags + */ _uninterpretedTail = readTail(URL_TAIL, in); } } else if (FILE_MONIKER.equals(_moniker)) { @@ -476,12 +496,15 @@ public final class HyperlinkRecord extends StandardRecord { int size = in.readInt(); if (size > 0) { int charDataSize = in.readInt(); - if (in.readUShort() != 0x0003) { - throw new RecordFormatException("expected "); // TODO + + //From the spec: An optional unsigned integer that MUST be 3 if present + int optFlags = in.readUShort(); + if (optFlags != 0x0003) { + throw new RecordFormatException("Expected 0x3 but found " + optFlags); } _address = StringUtil.readUnicodeLE(in, charDataSize/2); } else { - _address = null; // TODO + _address = null; } } else if (STD_MONIKER.equals(_moniker)) { _fileOpts = in.readShort(); @@ -493,9 +516,6 @@ public final class HyperlinkRecord extends StandardRecord { _address = new String(path_bytes); } - } else { - // can happen for a plain link within the current document - _moniker = null; } if((_linkOpts & HLINK_PLACE) != 0) { @@ -525,13 +545,15 @@ public final class HyperlinkRecord extends StandardRecord { StringUtil.putUnicodeLE(_targetFrame, out); } - if ((_linkOpts & HLINK_UNC_PATH) != 0) { + if ((_linkOpts & HLINK_URL) != 0 && (_linkOpts & HLINK_UNC_PATH) != 0) { out.writeInt(_address.length()); StringUtil.putUnicodeLE(_address, out); - } else if ((_linkOpts & HLINK_URL) != 0){ + } + + if ((_linkOpts & HLINK_URL) != 0 && (_linkOpts & HLINK_UNC_PATH) == 0) { _moniker.serialize(out); if(URL_MONIKER.equals(_moniker)){ - if ((_linkOpts & HLINK_TARGET_FRAME) != 0) { + if (_uninterpretedTail == null) { out.writeInt(_address.length()*2); StringUtil.putUnicodeLE(_address, out); } else { @@ -575,15 +597,16 @@ public final class HyperlinkRecord extends StandardRecord { size += 4; // int nChars size += _targetFrame.length()*2; } - if ((_linkOpts & HLINK_UNC_PATH) != 0) { + if ((_linkOpts & HLINK_URL) != 0 && (_linkOpts & HLINK_UNC_PATH) != 0) { size += 4; // int nChars size += _address.length()*2; - } else if ((_linkOpts & HLINK_URL) != 0){ + } + if ((_linkOpts & HLINK_URL) != 0 && (_linkOpts & HLINK_UNC_PATH) == 0) { size += GUID.ENCODED_SIZE; if(URL_MONIKER.equals(_moniker)){ size += 4; //address length size += _address.length()*2; - if ((_linkOpts & HLINK_TARGET_FRAME) == 0) { + if (_uninterpretedTail != null) { size += TAIL_SIZE; } } else if (FILE_MONIKER.equals(_moniker)){ diff --git a/src/testcases/org/apache/poi/hssf/record/TestHyperlinkRecord.java b/src/testcases/org/apache/poi/hssf/record/TestHyperlinkRecord.java index 4232af7c7..3383bccbb 100644 --- a/src/testcases/org/apache/poi/hssf/record/TestHyperlinkRecord.java +++ b/src/testcases/org/apache/poi/hssf/record/TestHyperlinkRecord.java @@ -243,7 +243,24 @@ public final class TestHyperlinkRecord extends TestCase { "43 00 32 00 00 00"); - private void confirmGUID(GUID expectedGuid, GUID actualGuid) { + /** + * From Bugzilla 47498 + */ + private static byte[] data_47498 = { + 0x02, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, (byte)0xD0, (byte)0xC9, + (byte)0xEA, 0x79, (byte)0xF9, (byte)0xBA, (byte)0xCE, 0x11, (byte)0x8C, + (byte)0x82, 0x00, (byte)0xAA, 0x00, 0x4B, (byte)0xA9, 0x0B, 0x02, 0x00, + 0x00, 0x00, 0x15, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x50, 0x00, + 0x44, 0x00, 0x46, 0x00, 0x00, 0x00, (byte)0xE0, (byte)0xC9, (byte)0xEA, + 0x79, (byte)0xF9, (byte)0xBA, (byte)0xCE, (byte)0x11, (byte)0x8C, (byte)0x82, + 0x00, (byte)0xAA, 0x00, 0x4B, (byte)0xA9, 0x0B, 0x28, 0x00, 0x00, 0x00, + 0x74, 0x00, 0x65, 0x00, 0x73, 0x00, 0x74, 0x00, 0x66, 0x00, 0x6F, 0x00, + 0x6C, 0x00, 0x64, 0x00, 0x65, 0x00, 0x72, 0x00, 0x2F, 0x00, 0x74, 0x00, + 0x65, 0x00, 0x73, 0x00, 0x74, 0x00, 0x2E, 0x00, 0x50, 0x00, 0x44, 0x00, + 0x46, 0x00, 0x00, 0x00}; + + + private void confirmGUID(GUID expectedGuid, GUID actualGuid) { assertEquals(expectedGuid, actualGuid); } public void testReadURLLink(){ @@ -479,4 +496,24 @@ public final class TestHyperlinkRecord extends TestCase { assertEquals(new String(HexDump.longToHex(d4)), new String(HexDump.longToHex(g.getD4()))); } + public void test47498(){ + RecordInputStream is = TestcaseRecordInputStream.create(HyperlinkRecord.sid, data_47498); + HyperlinkRecord link = new HyperlinkRecord(is); + assertEquals(2, link.getFirstRow()); + assertEquals(2, link.getLastRow()); + assertEquals(0, link.getFirstColumn()); + assertEquals(0, link.getLastColumn()); + confirmGUID(HyperlinkRecord.STD_MONIKER, link.getGuid()); + confirmGUID(HyperlinkRecord.URL_MONIKER, link.getMoniker()); + assertEquals(2, link.getLabelOptions()); + int opts = HyperlinkRecord.HLINK_URL | HyperlinkRecord.HLINK_LABEL; + assertEquals(opts, link.getLinkOptions()); + assertEquals(0, link.getFileOptions()); + + assertEquals("PDF", link.getLabel()); + assertEquals("testfolder/test.PDF", link.getAddress()); + + byte[] ser = link.serialize(); + TestcaseRecordInputStream.confirmRecordEncoding(HyperlinkRecord.sid, data_47498, ser); + } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestFormulas.java b/src/testcases/org/apache/poi/hssf/usermodel/TestFormulas.java index 3281b5115..98ccbf039 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestFormulas.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestFormulas.java @@ -860,7 +860,7 @@ public final class TestFormulas extends TestCase { HSSFWorkbook wb = openSample("27272_1.xls"); wb.getSheetAt(0); assertEquals("Reference for named range ", "Compliance!#REF!",wb.getNameAt(0).getRefersToFormula()); - File outF = File.createTempFile("bug27272_1",".xls"); + File outF = TempFile.createTempFile("bug27272_1",".xls"); wb.write(new FileOutputStream(outF)); System.out.println("Open "+outF.getAbsolutePath()+" in Excel"); } @@ -868,7 +868,7 @@ public final class TestFormulas extends TestCase { public void test27272_2() throws Exception { HSSFWorkbook wb = openSample("27272_2.xls"); assertEquals("Reference for named range ", "LOAD.POD_HISTORIES!#REF!",wb.getNameAt(0).getRefersToFormula()); - File outF = File.createTempFile("bug27272_2",".xls"); + File outF = TempFile.createTempFile("bug27272_2",".xls"); wb.write(new FileOutputStream(outF)); System.out.println("Open "+outF.getAbsolutePath()+" in Excel"); } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFHyperlink.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFHyperlink.java index 2d5648217..454a29710 100755 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFHyperlink.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFHyperlink.java @@ -166,6 +166,28 @@ public final class TestHSSFHyperlink extends BaseTestHyperlink { assertEquals("http://poi.apache.org/hssf/", link.getAddress()); } + public void testCreate() throws Exception { + HSSFWorkbook wb = getTestDataProvider().createWorkbook(); + + HSSFHyperlink link; + HSSFCell cell; + HSSFSheet sheet = wb.createSheet("Hyperlinks"); + + cell = sheet.createRow(1).createCell(0); + cell.setCellValue("File Link"); + link = new HSSFHyperlink(HSSFHyperlink.LINK_FILE); + link.setAddress("testfolder\\test.PDF"); + cell.setHyperlink(link); + + wb = getTestDataProvider().writeOutAndReadBack(wb); + sheet = wb.getSheet("Hyperlinks"); + + cell = sheet.getRow(1).getCell(0); + link = cell.getHyperlink(); + assertNotNull(link); + assertEquals("testfolder\\test.PDF", link.getAddress()); + } + /** * Test that HSSFSheet#shiftRows moves hyperlinks, * see bugs #46445 and #29957 diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java index 2f944c5fd..ee8e0e824 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java @@ -34,6 +34,7 @@ import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.ss.util.CellRangeAddressList; import org.apache.poi.ss.usermodel.BaseTestSheet; import org.apache.poi.ddf.EscherDgRecord; +import org.apache.poi.util.TempFile; /** * Tests HSSFSheet. This test case is very incomplete at the moment. @@ -572,8 +573,7 @@ public final class TestHSSFSheet extends BaseTestSheet { assertFalse(sheet2.getForceFormulaRecalculation()); // Save and manually verify that on column C we have 0, value in template - File tempFile = new File(System.getProperty("java.io.tmpdir")+"/uncalced_err.xls" ); - tempFile.delete(); + File tempFile = TempFile.createTempFile("uncalced_err", ".xls" ); FileOutputStream fout = new FileOutputStream( tempFile ); workbook.write( fout ); fout.close(); @@ -581,7 +581,7 @@ public final class TestHSSFSheet extends BaseTestSheet { assertTrue(sheet.getForceFormulaRecalculation()); // Save and manually verify that on column C we have now 13, calculated value - tempFile = new File(System.getProperty("java.io.tmpdir")+"/uncalced_succ.xls" ); + tempFile = TempFile.createTempFile("uncalced_succ", ".xls"); tempFile.delete(); fout = new FileOutputStream( tempFile ); workbook.write( fout );