Wicket
  1. Wicket
  2. WICKET-4138

BookmarkablePageLinks not working on a forwarded page

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.0, 1.5.1, 1.5.2
    • Fix Version/s: 1.5.3
    • Component/s: wicket
    • Labels:
      None
    • Environment:
      Windows XP, Quad Core Intel, 2GB RAM

      Description

      While migrating our app from 1.4 to 1.5 we have discovered a problem with how BookmarkablePageLinks are rendered.

      The attached quickstart demonstrates the problem:

      Two pages: HomePage and OtherPage mounted at: /content/home and /content/other respectively.

      These are mounted using the encoder UrlPathPageParametersEncoder for backwards compatibility with existing 1.4 style URLs.

      A filter has been established in web.xml to forward requests to root (eg., localhost) to localhost/content/home
      [Note: I have left out the port :8080 part from all URL references so please insert when testing]

      Point browser to http://localhost and the page is forwarded to http://localhost/content/home and displays correctly (browser URL still shows http://localhost as desired) but the links do not work because they remove the 'content' segment of the URL:

      eg., Home link -> http://localhost/home - fails - should have been http://localhost/content/home

      If you type in the full URL: http://localhost/content/home

      then the home page displays and the links work correctly.

      A similar page set up works fine in 1.4.

      1. WICKET-4138.patch
        3 kB
        Martin Grigorov
      2. ForwardingFromRoot.zip
        48 kB
        Chris Colman
      3. ForwardAttributes.java
        4 kB
        Martin Grigorov

        Activity

        Hide
        Chris Colman added a comment -

        This quickstart demonstrates the bug.

        Show
        Chris Colman added a comment - This quickstart demonstrates the bug.
        Hide
        Chris Colman added a comment - - edited

        If I get Wicket to handle the "/" mount path and not the separate filter then it seems to work. Hopefully I will be able to refactor for 1.5 so that the separate filter ignores the "/" mount path and leaves it up to Wicket to handle.

        The reason the separate filter is currently handling the "/" mount path is that the app is multi tenanted and supports many virtual hosts. It uses a URL parameter strategy where the value is an organization ID like /o/123 to specify the organization ID to display. An incoming request for www.abc.com is forwarded to www.abc.com/content/home/o/123 but we should be able to determine the org ID from the domain name and let wicket perform its natural rendering of the page returned by getHomePage().

        If this works out then this issue will no longer be an issue for us.

        Show
        Chris Colman added a comment - - edited If I get Wicket to handle the "/" mount path and not the separate filter then it seems to work. Hopefully I will be able to refactor for 1.5 so that the separate filter ignores the "/" mount path and leaves it up to Wicket to handle. The reason the separate filter is currently handling the "/" mount path is that the app is multi tenanted and supports many virtual hosts. It uses a URL parameter strategy where the value is an organization ID like /o/123 to specify the organization ID to display. An incoming request for www.abc.com is forwarded to www.abc.com/content/home/o/123 but we should be able to determine the org ID from the domain name and let wicket perform its natural rendering of the page returned by getHomePage(). If this works out then this issue will no longer be an issue for us.
        Hide
        Martin Grigorov added a comment -

        Here is a minimal patch that fixes the problem.

        Show
        Martin Grigorov added a comment - Here is a minimal patch that fixes the problem.
        Hide
        Martin Grigorov added a comment -

        Igor,

        Can you take a look?
        Do we need ForwardAttributes (like ErrorAttributes) ?
        My investigation shows that UrlRenderer#getBaseUrl() should not always take into account javax.servlet.forward.request_uri. Only renderRelativeXYZ() may use it otherwise infinite redirects happen.

        Additionally should we make org.apache.wicket.protocol.http.servlet.ServletWebRequest.shouldPreserveClientUrl() aware of forward attributes ? I'm not sure.

        Show
        Martin Grigorov added a comment - Igor, Can you take a look? Do we need ForwardAttributes (like ErrorAttributes) ? My investigation shows that UrlRenderer#getBaseUrl() should not always take into account javax.servlet.forward.request_uri. Only renderRelativeXYZ() may use it otherwise infinite redirects happen. Additionally should we make org.apache.wicket.protocol.http.servlet.ServletWebRequest.shouldPreserveClientUrl() aware of forward attributes ? I'm not sure.
        Hide
        Chris Colman added a comment -

        You added ForwardAttributes but I can't see any reference to that in the code base or the patch. How is it used?

        Show
        Chris Colman added a comment - You added ForwardAttributes but I can't see any reference to that in the code base or the patch. How is it used?
        Hide
        Igor Vaynberg added a comment -

        its not used yet, it was just an idea he prototyped...

        Show
        Igor Vaynberg added a comment - its not used yet, it was just an idea he prototyped...
        Hide
        Chris Colman added a comment - - edited

        I applied the patch and now the above mentioned links in our app works fine under Wicket 1.5. Great work!

        When will this patch be committed to the trunk?

        Show
        Chris Colman added a comment - - edited I applied the patch and now the above mentioned links in our app works fine under Wicket 1.5. Great work! When will this patch be committed to the trunk?

          People

          • Assignee:
            Martin Grigorov
            Reporter:
            Chris Colman
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development