Tapestry
  1. Tapestry
  2. TAPESTRY-802

All Tapestry URLs should be encoded using RequestCycle.encodeURL(), except for portlets URLs

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 4.0
    • Fix Version/s: 4.1
    • Component/s: Framework, Portlet
    • Labels:
      None
    • Environment:
      WSRP4J
      Tapestry 4.0rc1

      Description

      All Tapestry URLs referencing the Tapestry servlet or context assets should be encoded using RequestCycle.encodeURL() whether the link is stateless or not.

      In portlet mode, the portlet URLs obtained by createActionURL() or createRenderURL() should NOT be encoded.

      Failure to do so causes problems when running as a WSRP producer where the URLs will be WSRP defined tokens that get re-written by the WSRP container. Action, render and resource URLs are encoded differently.

        Activity

        Hide
        Jesse Kuhnert added a comment -

        Re-factored EngineServiceLink to optionally use IRequestCycle to encode the resulting url, depending on whether or not it is specified in the constructor. (ie tapestry core doesn't pass it into constructor while PortletAssetLinkFactory does)

        Show
        Jesse Kuhnert added a comment - Re-factored EngineServiceLink to optionally use IRequestCycle to encode the resulting url, depending on whether or not it is specified in the constructor. (ie tapestry core doesn't pass it into constructor while PortletAssetLinkFactory does)
        Hide
        Jesse Kuhnert added a comment -

        Hi Rapheal,

        If you have the patience I may need to change the way these things work. I've already taken the call to encodeURL() out of the EngineServiceLink.

        I plan on refactoring things a little bit so that the portlet hivemind configuration provides it's own version of how to handle URL encoding so that everyone else is not affected. (hopefully as small a footprint as I can manage)

        Show
        Jesse Kuhnert added a comment - Hi Rapheal, If you have the patience I may need to change the way these things work. I've already taken the call to encodeURL() out of the EngineServiceLink. I plan on refactoring things a little bit so that the portlet hivemind configuration provides it's own version of how to handle URL encoding so that everyone else is not affected. (hopefully as small a footprint as I can manage)
        Hide
        Jesse Kuhnert added a comment -

        Sorry it took so long Andy, but I do finally understand your problem with the way this is done. The LinkFactory already handles encoding of session information when necessary, so using IRequestCycle.encodeURL() is absolutely not an acceptable way to do the encoding of URL's returned from link factory.

        If this is a portlet only construct then perhaps things need to be re-factored for the portlet services. I'm leaving this issue open but am about to remove encodeURL() from the normal semantics for now. (Finally ran the workbench).

        I'll try and come up with an answer for this for portlets before 4.1 is officially released.

        Show
        Jesse Kuhnert added a comment - Sorry it took so long Andy, but I do finally understand your problem with the way this is done. The LinkFactory already handles encoding of session information when necessary, so using IRequestCycle.encodeURL() is absolutely not an acceptable way to do the encoding of URL's returned from link factory. If this is a portlet only construct then perhaps things need to be re-factored for the portlet services. I'm leaving this issue open but am about to remove encodeURL() from the normal semantics for now. (Finally ran the workbench). I'll try and come up with an answer for this for portlets before 4.1 is officially released.
        Hide
        Andreas Andreou added a comment -

        Raphael, I don't disagree, I just don't see why we have to also do this in servlet URLs.

        Of course, urls for static resources (in normal servlet apps) that contain the jsessionid param
        are 100% valid but (if you ask me) really ugly. And one reason i love tapestry is for the nice, clean
        and elegant HTML it generates.

        Show
        Andreas Andreou added a comment - Raphael, I don't disagree, I just don't see why we have to also do this in servlet URLs. Of course, urls for static resources (in normal servlet apps) that contain the jsessionid param are 100% valid but (if you ask me) really ugly. And one reason i love tapestry is for the nice, clean and elegant HTML it generates.
        Hide
        Raphael Jean added a comment -

        The JSR-168 portlet spec clearly states (12.1.2):
        "Portlets may generate content with URLs referring to other resources within the portal,
        such as servlets, JSPs, images and other static files. Some portal/portlet-container
        implementations may require those URLs to contain implementation specific data
        encoded in it. Because of that, portlets should use the encodeURL method to create such
        URLs. The encodeURL method may include the session ID and other portal/portletcontainer
        specific information into the URL. If encoding is not needed, it returns the URL
        unchanged."

        Also, this is required when a tapestry portlet is running as a wsrp producer. All urls present in the html markup generated by the portlet must be encoded in a specific way, so that they can be rewritten by the wsrp consumer. Something like:
        wsrp_rewrite?wsrp-urlType=resource&wsrp-url=YOUR_URL_GOES_HERE/wsrp_rewrite

        Show
        Raphael Jean added a comment - The JSR-168 portlet spec clearly states (12.1.2): "Portlets may generate content with URLs referring to other resources within the portal, such as servlets, JSPs, images and other static files. Some portal/portlet-container implementations may require those URLs to contain implementation specific data encoded in it. Because of that, portlets should use the encodeURL method to create such URLs. The encodeURL method may include the session ID and other portal/portletcontainer specific information into the URL. If encoding is not needed, it returns the URL unchanged." Also, this is required when a tapestry portlet is running as a wsrp producer. All urls present in the html markup generated by the portlet must be encoded in a specific way, so that they can be rewritten by the wsrp consumer. Something like: wsrp_rewrite?wsrp-urlType=resource&wsrp-url=YOUR_URL_GOES_HERE/wsrp_rewrite
        Hide
        Andreas Andreou added a comment -

        Why should asset urls be encoded? Even context assets are.

        This can result in:
        <link rel="stylesheet" type="text/css" href="/workbench/css/workbench.css;jsessionid=m77nh3f6mcmr"/>

        Show
        Andreas Andreou added a comment - Why should asset urls be encoded? Even context assets are. This can result in: <link rel="stylesheet" type="text/css" href="/workbench/css/workbench.css;jsessionid=m77nh3f6mcmr"/>
        Hide
        Jesse Kuhnert added a comment -

        Patch applied.

        Show
        Jesse Kuhnert added a comment - Patch applied.
        Hide
        Jesse Kuhnert added a comment -

        Will apply this tomorrow from tap 4.0 computer.

        Show
        Jesse Kuhnert added a comment - Will apply this tomorrow from tap 4.0 computer.
        Hide
        Raphael Jean added a comment -

        Patch for tapestry 4 trunk: fixes ContextAsset, ContextAssetFactory, EngineServiceLink, PortletLink, PortletLinkFactoryImpl and related tests and hivemind config files.

        Show
        Raphael Jean added a comment - Patch for tapestry 4 trunk: fixes ContextAsset, ContextAssetFactory, EngineServiceLink, PortletLink, PortletLinkFactoryImpl and related tests and hivemind config files.

          People

          • Assignee:
            Jesse Kuhnert
            Reporter:
            Raphael Jean
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development