Uploaded image for project: 'Velocity Tools'
  1. Velocity Tools
  2. VELTOOLS-146

LinkTool.relative(url) and LinkTool.absolute(url) improperly handle URLs with query strings or anchors

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0.x, 2.1
    • Component/s: GenericTools
    • Labels:
      None

      Description

      On Tue, Jul 26, 2011 at 2:27 PM, Christopher Schultz <chris@christopherschultz.net> wrote:
      ...
      > We use StrutsLinkTool pretty much everywhere and had been relying on the
      > previous behavior (tools 1.4) of StrutsLinkTool.setForward(String) which
      > takes a reference to a Struts-defined "forward" which is basically just
      > a URL.
      >
      > In the past, we were able to define a URL in Struts like this:
      >
      > <forward name="some-reference" path="/path/to/resource?foo=bar" />
      >
      > Then, in the markup:
      >
      > <a href="$link.setForward('some-reference')">link text</a>
      >
      > It appears that somewhere in the modifications, the URL ha sbeen
      > sanitized and now "/path/to/resource?foo=bar" has been helpfully
      > replaced by "/path/to/resource%3Ffoo=bar".
      >
      > Was this intentional? Is there a way to get the old behavior back?

      It was unintentionally changed, in part because i wasn't aware that forwards could/would contain query strings. Basically, setForward passes the url to absolute(url) which would correctly handle absolute urls with query strings but treats relative urls as merely paths, without parsing out anchors or query strings.

      I think we should adapt both $link.relative($url) and $link.absolute($url) to accept paths with query strings and anchors. This probably will require a setFromURI(url, withoutOverriding) adapted from what absolute($url) does for actual absolute URIs.

        Activity

        Hide
        nbubna Nathan Bubna added a comment -

        Actually, instead of creating a setFromURI that doesn't override actual values with null ones, i think there is no good reason that setFromURI should ever do that. So we should just adapt setFromURI to have a lighter touch and have relative(url) and absolute(url) use that.

        Show
        nbubna Nathan Bubna added a comment - Actually, instead of creating a setFromURI that doesn't override actual values with null ones, i think there is no good reason that setFromURI should ever do that. So we should just adapt setFromURI to have a lighter touch and have relative(url) and absolute(url) use that.
        Hide
        nbubna Nathan Bubna added a comment -

        Fixed in r1151728. Also improves the $link.uri($uri) method and generally simplifies the implementation of absolute(uri).

        Show
        nbubna Nathan Bubna added a comment - Fixed in r1151728. Also improves the $link.uri($uri) method and generally simplifies the implementation of absolute(uri).

          People

          • Assignee:
            nbubna Nathan Bubna
            Reporter:
            nbubna Nathan Bubna
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development