From 6803dc89e071fac1df5aadd5d8286bfdc8b6a3bb Mon Sep 17 00:00:00 2001 From: Javen O'Neal Date: Sun, 17 Jul 2016 05:43:03 +0000 Subject: [PATCH] bug 59775: correctly create XSSFHyperlinks when target is a URL containing a hash mark; patch contributed by Guillermo Alvarez git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1753013 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/xssf/usermodel/XSSFHyperlink.java | 65 ++++++++++-------- .../poi/xssf/usermodel/TestXSSFHyperlink.java | 40 ++++++++++- test-data/spreadsheet/59775.xlsx | Bin 0 -> 5977 bytes 3 files changed, 73 insertions(+), 32 deletions(-) create mode 100644 test-data/spreadsheet/59775.xlsx diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFHyperlink.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFHyperlink.java index 4b70d50e25..b0c9a92f35 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFHyperlink.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFHyperlink.java @@ -58,44 +58,47 @@ public class XSSFHyperlink implements Hyperlink { _ctHyperlink = ctHyperlink; _externalRel = hyperlinkRel; - // Figure out the Hyperlink type and distination + // Figure out the Hyperlink type and destination - // If it has a location, it's internal - if (ctHyperlink.getLocation() != null) { - _type = Hyperlink.LINK_DOCUMENT; - _location = ctHyperlink.getLocation(); - } else { - // Otherwise it's somehow external, check - // the relation to see how - if (_externalRel == null) { - if (ctHyperlink.getId() != null) { - throw new IllegalStateException("The hyperlink for cell " + ctHyperlink.getRef() + - " references relation " + ctHyperlink.getId() + ", but that didn't exist!"); - } + if (_externalRel == null) { + // If it has a location, it's internal + if (ctHyperlink.getLocation() != null) { + _type = Hyperlink.LINK_DOCUMENT; + _location = ctHyperlink.getLocation(); + } else if (ctHyperlink.getId() != null) { + throw new IllegalStateException("The hyperlink for cell " + + ctHyperlink.getRef() + " references relation " + + ctHyperlink.getId() + ", but that didn't exist!"); + } else { // hyperlink is internal and is not related to other parts _type = Hyperlink.LINK_DOCUMENT; + } + } else { + URI target = _externalRel.getTargetURI(); + _location = target.toString(); + if (ctHyperlink.getLocation() != null) { + // URI fragment + _location += "#" + ctHyperlink.getLocation(); + } + + // Try to figure out the type + if (_location.startsWith("http://") || _location.startsWith("https://") + || _location.startsWith("ftp://")) { + _type = Hyperlink.LINK_URL; + } else if (_location.startsWith("mailto:")) { + _type = Hyperlink.LINK_EMAIL; } else { - URI target = _externalRel.getTargetURI(); - _location = target.toString(); - - // Try to figure out the type - if (_location.startsWith("http://") || _location.startsWith("https://") - || _location.startsWith("ftp://")) { - _type = Hyperlink.LINK_URL; - } else if (_location.startsWith("mailto:")) { - _type = Hyperlink.LINK_EMAIL; - } else { - _type = Hyperlink.LINK_FILE; - } + _type = Hyperlink.LINK_FILE; } + } + } - } - } - /** * Create a new XSSFHyperlink. This method is for Internal use only. - * XSSFHyperlinks can be created by XSSFCreationHelper. + * XSSFHyperlinks can be created by {@link XSSFCreationHelper}. + * See the spreadsheet quick-guide + * for an example. * * @param other the hyperlink to copy */ @@ -165,7 +168,8 @@ public class XSSFHyperlink implements Hyperlink { } /** - * Hyperlink address. Depending on the hyperlink type it can be URL, e-mail, path to a file + * Hyperlink address. Depending on the hyperlink type it can be URL, e-mail, path to a file. + * The is the hyperlink target. * * @return the address of this hyperlink */ @@ -216,6 +220,7 @@ public class XSSFHyperlink implements Hyperlink { /** * Hyperlink address. Depending on the hyperlink type it can be URL, e-mail, path to a file + * This is the hyperlink target. * * @param address - the address of this hyperlink */ diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFHyperlink.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFHyperlink.java index d5e3df294e..ec68569b73 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFHyperlink.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFHyperlink.java @@ -17,12 +17,12 @@ package org.apache.poi.xssf.usermodel; -import java.io.IOException; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; +import java.io.IOException; + import org.apache.poi.hssf.usermodel.HSSFHyperlink; import org.apache.poi.openxml4j.opc.PackageRelationship; import org.apache.poi.openxml4j.opc.PackageRelationshipCollection; @@ -31,6 +31,7 @@ import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.ss.usermodel.CreationHelper; import org.apache.poi.ss.usermodel.Hyperlink; import org.apache.poi.ss.usermodel.Row; +import org.apache.poi.ss.util.CellAddress; import org.apache.poi.ss.util.CellReference; import org.apache.poi.xssf.XSSFITestDataProvider; import org.apache.poi.xssf.XSSFTestDataSamples; @@ -292,4 +293,39 @@ public final class TestXSSFHyperlink extends BaseTestHyperlink { // Are HSSFHyperlink.label and XSSFHyperlink.tooltip the same? If so, perhaps one of these needs renamed for a consistent Hyperlink interface // assertEquals("label", xlink.getTooltip()); } + + /* bug 59775: XSSFHyperlink has wrong type if it contains a location (CTHyperlink#getLocation) + * URLs with a hash mark (#) are still URL hyperlinks, not document links + */ + @Test + public void testURLsWithHashMark() throws IOException { + XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("59775.xlsx"); + XSSFSheet sh = wb.getSheetAt(0); + CellAddress A2 = new CellAddress("A2"); + CellAddress A3 = new CellAddress("A3"); + CellAddress A4 = new CellAddress("A4"); + CellAddress A7 = new CellAddress("A7"); + + XSSFHyperlink link = sh.getHyperlink(A2); + assertEquals("address", "A2", link.getCellRef()); + assertEquals("link type", Hyperlink.LINK_URL, link.getType()); + assertEquals("link target", "http://twitter.com/#!/apacheorg", link.getAddress()); + + link = sh.getHyperlink(A3); + assertEquals("address", "A3", link.getCellRef()); + assertEquals("link type", Hyperlink.LINK_URL, link.getType()); + assertEquals("link target", "http://www.bailii.org/databases.html#ie", link.getAddress()); + + link = sh.getHyperlink(A4); + assertEquals("address", "A4", link.getCellRef()); + assertEquals("link type", Hyperlink.LINK_URL, link.getType()); + assertEquals("link target", "https://en.wikipedia.org/wiki/Apache_POI#See_also", link.getAddress()); + + link = sh.getHyperlink(A7); + assertEquals("address", "A7", link.getCellRef()); + assertEquals("link type", Hyperlink.LINK_DOCUMENT, link.getType()); + assertEquals("link target", "Sheet1", link.getAddress()); + + wb.close(); + } } diff --git a/test-data/spreadsheet/59775.xlsx b/test-data/spreadsheet/59775.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..03238bd2abc57bbb9c36a3749d7ff2ae7eccb3de GIT binary patch literal 5977 zcmai21yq#n)&)uFmXJm|1{fL?X$I+%?vA06?v(D78d?ygOA+ZVNf}B~x%E`wIE8PB{ZYF<(EGnr;;AVnLUJy~r;K}XQxsRTYAM)bQ+F^;}|KaC5a&+J*D z0D3w72z9N9d#&^)KE)mVKOH?dcn9-+yKwoXRlwrRPO*xhvA?I=7)3yu6~Kg5<`c{>{(ds)hdo{zbgHE9y5-FiUJ z3cQQ?TktT=LeV^+Wkg*YK6(P*6iMfEl|m5Qgr-!72SCO!%;Gn1W4| zoWKsw944;LE?_(M7k0L_g)U{IY^Ihoe! zyk@ASoA$fS3h>4C?7D2hg_oWHLm3cX7#Ru&ALc~k)P>DCZ8x<`z&~sFY6av#8Wd^` zP^8GWctocjs=PCeSZSg}N^5~*tujXXEnt4ltr(}Qt4rZCd}O|1uzBD^n?B5ATvBnT z38{(7mW}{+up5_O^Z=3-xcF>NYPN+=FpN(@nJlGykrsW76Q80Idy~S8`Gt$g<-FH5Jc(9Bjy4r?C5^)_Gp8AUA*w+#7j7UB>-g zq=iIycd2;tOK(L_Ri3*PgGdu~Lw;#=6E}aW|09hZgNyl~sl9A(_?wO(@uy2D!^6SR zBL1l(ICnc@s4nfKztD$pw`yojM`9VVUEj z2Wqb<>pOT8IV`3No z55vMAhS)#z7xyk&CSWJC8@NIe78Kh7INb|ZEGh57^HN+@GoT;1-tirXQC;;#LQ=Kg zQtANvZ|Ye9%Cg?y=FhxxpeGrM?zI?oxzUzt0SSy!@QlzTF?A$Q{h{{zXi;Wmrtg%M zaz%)Ng_;83`(U8(W)YGdQd(O;5_JpP$n0#2q3t=TwK>hYl7cQzllf#wQ#;!VI6!kKtG4Pp5xchH z@*#f_}Q^%HnyK#moMdzP7;*m;+)Y{BQ!g?kf6H?q~fZ`Bqt` zMp+>RFVgAg$PU?`AXjbTv>gO&I021M2juv6a z%CX(W zs>tdS8uDq{zBfTRD!lIZKO2}+?0KSt@6S~z`ELK9J36dSm#laLo%gl=BO#&pG1{JAucKx0F zq`8$nhdH0rHB?R=u{@T$xtd$(VtKHBjnFHvEhM`zou@~=B*HyJ@5%ftz2~%Oy5eNS z=-*?EAFRr<=hK6<(B?|qQio(e=c34phdpE%&O6J+Jr!cmi&c^fDtYg^uEpz45nfT+ z-ljd>$b&H6??1KBFc8wG+kdj6;#6$5OaG(S=VDW)Y`R!|pIyLp?n5k5(t-(weNqx{t|4x-(#K9 z@x6c{;qye-c9`4S(L;nA=so~5=`Ucw#QyQj)BXkB4V|1V&CFb!|HZFg51jvIS8Ia2 z90Dg!=-w5f+%Ekb8qK?ANhKrfd>N5`VEQ~pCP%`?q#u}-k%)Ptu=~PuO`v7a(?b3v zlr!F)P3JH}eFLKTwJSvgygS4`)1(7bw4_Q+hA=_OzBEj5Wji_7*;pz{ z9h3Iqtw&K7r_*M|IVsdj{U>x6v=Csl+_1O>3_KD(VvD1mEg&$R7HjWtty}Z!T4w+# zguILy1}ck6Ru%hbEH_DruJo^nUzeZc~MFOJWiW=Ws3>j4j@Ek__^lRZw4%(V{v*zBns?Mr}U8J$_h(TNbmUU!aBJ2yOj z`fFU4^V&XF^^3C#LntmJKak^!KinJp1&vDIncVeJ|2Rke`(oe{lTqNOu_pnC3l+ZF z6JiuNp;ix z5ROUOwB%5WI@eh)3BvtYvMk$^^3%uL{RtoUcJLFXi9RbYp4HM4J{3!un&fwmo29xF zio~Lw;g=uJn<1OrtNt2n<&)=OC6tI>v0O_r4G4Y#1g%fUd}1bYsK;C7t!bA$je^*V(nN&C&4aLxl91|6WGoklgkQ?}dz2V1u(5gut zX(vWp_CPv*{Ms}%M<;q{_$wWAxu%dc)~G^w5GTop_!$whkkMAUwYFMT@7UZERD{gD z1=f>^g&hS5tpIuU!08StLI} zLyu*ET5OOK5v!R;$z%DJ_sAW{NQMqF8?zNElz}MQBr$EQ@h<@YK?L)m%8E9}MKw|>8rvh0Z^lVKFjX8NRgAi7C-N2YH0JWWSFkxPsxg0HVdIoN z`Ge95srov5=11Vnag^4E?!XVVK#5_L)tWiPU=($zGGZG8F`s`#Ffe^>JuV4Rm3~dy zd$yFBMBt=RZA#6A2m?d<%QBX0vmN<2Ti*E3-HQ$la^n@=9E5s)-{~5OAL8<=A zOpugdW9v5rwoz+Tl$qYbmdz$~0$+}fg@7nvFK%@FMHJS^*mb0t_gitEj8e0LAS^vD8Vg5B`oLxL@&3;{OJ87c| za{z+Sy)%Loj(DlDF`~3FDnru@AT^?725_fcKC7esE$v)DN^uT3L#b-8KbDoG4cPd) z?#cDz$FeJk3@-x%y;$j%Y|&7@KSoUJ6sBg)Y@gFQzCgWKiX(@!ilMlKC-(x+7Y(AV z;+<&n+MgaE5Yni!`d~QGF$SIP%p)S*Ubc}jZcIJi;~fF7cp5G8_yc4FbR4@z(2S@kKZ@j& zaslT}6dK`ZG8K&oE>4DfFbJpJgrt)OkT{Ja(Zo_U>VllEZji-c0!pLE)i=he(LQP+qOC&@6eK8LX6G7YB`Lj8*H$Begp zIgW=AwXs4?EJS@qkA@m$jxiVMbCN~Hl)6ar>sJYi!a*29n=F2Y6~xE{#h@UhUMFYu zmSYSFCjAXDp{{3ky?Rd-DNR7&f%#)@y$WdBbBJPf&SyfOMAAL^(S!^%X1S)Qh5$z8 zB`N;$=3|H|h)ZL!Hbyg$(RM-6D+UZTz+;wbF~c z%0{u2d=U{;wV|46r1&bxtLOE-x;SopE4Z5(u4#Qk$hpGrKgrr~F+@;o9Ul{IYD0dg zXCY^Hl^!%wFlxHdelGgh&B^^G-J@%u>uhpp;O`f`xfIIWv7!K}ZF1(QMk9%RV&;k+ z*SMD~Wovb|1$0b@?|PZ1HO7CP!f@NMOQhGsm*Kq>H36j zgJ*O7VR^HfaBvLTfb>hI7T`i+Nuu3Rq>|)w)t4-#51h<-EGk_G1US$pRFGFF))u3Q z_xrEs^XNq;zG9YuPzRRHqy@L*rN9g)-sC5iVQ-4{jBvk^?}>JcjGUk3wnol#{%kw_ zlI5->JzSun$ePUn-I6muN4dF?TaTiG4`8=37U7@H3ij=d{P&I=t=elp2f%4PV?l)4 zvFTfrr66e5#lg+v6!)elNDeq;|yEl%ibYTo0Xh(*>9u`+H5fC>+}PqB$AoF`9j^Fnn^!5+`)7ckz>@FbQN1SlIh0)AZF(Iz532BoW!} zh5WpnuM~arA#PqVGpAWWx|G4hJRHoN^=@|H)}(%WmW@%HAEyD3M%BNMVFEg3`IHeN%BM-tVQN;>I&=8Ox{Ec79E_`jwSGR<{x$s0tZN z46m%LMSQ14l0YDo`6ZDdxMYc+pL2-*Q)m*}M5*oOy)3K+lw1jWWEAIh0=& z{y&}gd%?TA&5cgF4T}4}7ra$XzgM|C^>6l|+X#j|!T`G6ihfVMd+cwnh1;lt^%<6W l*GTz2`7T%glU7@J|8q_#$sxgzhJ!