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

@@ -33,6 +33,8 @@
 
     <changes>
         <release version="3.5-beta7" date="2009-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">47498 - Fixed HyperlinkRecord to properly handle URL monikers</action>
+           <action dev="POI-DEVELOPERS" type="fix">47504 - Fixed XSSFWorkbook to read files with hyperlinks to document locations</action>
            <action dev="POI-DEVELOPERS" type="fix">47479 - Fix BoolErrRecord to tolerate incorrect format written by OOO</action>
            <action dev="POI-DEVELOPERS" type="fix">47448 - Allow HSSFEventFactory to handle non-zero padding at the end of the workbook stream</action>
            <action dev="POI-DEVELOPERS" type="add">47456 - Support for getting OLE object data in PowerPointExtractor</action>

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();
+                int length = in.readInt();
-
+                /**
-                if ((_linkOpts & HLINK_TARGET_FRAME) != 0) { 
+                 * The value of <code>length<code> be either the byte size of the url field
-                    int nChars = fieldSize/2;
+                 * (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)){

src/testcases/org/apache/poi/hssf/record/TestHyperlinkRecord.java

@@ -243,6 +243,23 @@ public final class TestHyperlinkRecord extends TestCase {
 			"43 00 32 00 00 00");
 
 
+    /**
+     * 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);
 	}
@@ -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);
+    }
 }

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

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

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" );
+        File tempFile = TempFile.createTempFile("uncalced_err", ".xls" );
-        tempFile.delete();
         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 );