Pluto
  1. Pluto
  2. PLUTO-598

Retrieving Portlet invoked servlet request attributes should first check PortletRequest attributes before using fallback to the web container

    Details

      Description

      In o.a.pluto.container.impl.HttpServletPortletRequestWrapper.getAttribute(String) a (non path related) attribute currently first is looked up from getRequest().getAttribute(String) with a fallback to the PortletRequest.getAttribute(String).
      However, this is the wrong order: first the PortletRequest.getAttribute(String) should be checked to ensure a possibly earlier set attribute which is only set with PortletRequest.setAttribute(String,Object) (and possibly cached there) is returned.

        Activity

        Hide
        Ate Douma added a comment -

        Refined handling of specific "injected" servlet container managed request attributes committed.

        Show
        Ate Douma added a comment - Refined handling of specific "injected" servlet container managed request attributes committed.
        Hide
        Eric Dalquist added a comment -

        So my primary concern here is the when the portal wants to have complete control the request attributes a portlet sees when getAttribute() is called on the PortletRequest or the ServletRequest based on the PortletRequest. The primary issue I was trying to address with PLUTO-600 was having the fallback logic in the HttpServletPortletRequestWrapper means that there is no reasonable way for a portal to influence this attribute resolution logic. My request is that if at all possible this logic should be part of the default PortletRequestContext SPI implementation so that implementing portals can still have full control over this attribute resolution.

        Show
        Eric Dalquist added a comment - So my primary concern here is the when the portal wants to have complete control the request attributes a portlet sees when getAttribute() is called on the PortletRequest or the ServletRequest based on the PortletRequest. The primary issue I was trying to address with PLUTO-600 was having the fallback logic in the HttpServletPortletRequestWrapper means that there is no reasonable way for a portal to influence this attribute resolution logic. My request is that if at all possible this logic should be part of the default PortletRequestContext SPI implementation so that implementing portals can still have full control over this attribute resolution.
        Hide
        Ate Douma added a comment -

        It turns out that while this change was needed to handle special cases on Websphere (e.g. like a forwarded portlet request invoking a servlet include request), this actually broke the same special cases on Tomcat

        Tomcat has a very peculiar handling of internal dispatcher state where it "injects" this current state within a request wrapper (org.apache.catalina.core.ApplicationHttpRequest).
        This means that these (there are actually two specific internal attributes used) attributes can only be retrieved reliably directly from the current request its parent (Tomcat "injects" the requestwrapper within the current request).
        Delegating the getAttribute call first to the PortletRequest (which by default delegates to the container SPI PortletRequestContext implementation) thus is not going to work out.
        And especially not when this PortletRequestContext itself delegates again to its wrapped servlet request ... as that one doesn't have the Tomcat injected ApplicationHttpRequest!

        Truly very complicated and tricky stuff.

        Bottom line: such internal "injected" attributes simply cannot be "managed" by the portlet container and must be retrieved from the current HttpServletPortletRequestWrapper its parent.
        Trouble is: this is servlet container specific, and at least Tomcat specific.

        So, the previous solution which always first checked the current HttpServletPortletRequestWrapper broke on Websphere, the new solution always on Tomcat.

        The only solution I can think of is providing special handling for these (Tomcat and potentially other servlet containers) internal attributes.

        I'm reopening this issue for both 2.0.3 and 2.1.0 versions to add handling for these servlet container managed attributes (meaning: not manageable by the portlet container).
        By default I'll add handling for these Tomcat/Catalina internal attribute names: "org.apache.catalina.core.DISPATCHER_TYPE" and "org.apache.catalina.core.DISPATCHER_REQUEST_PATH".
        As these are very Tomcat specific, these default "servletContainerManagedAttributes" (stored in a HashSet) will not interfere with other servlet containers.
        If other servlet containers turn out needing similar/likewise special attribute handling "protection", a static initializer method can be called (e.g. like using Spring injection) to (re)set this attributes set.

        Note: with PLUTO-600 Eric Dalquist slightly simplified the previous handling for Pluto 2.1.0, fully delegating the getAttribute call to the PortletRequest.getAttribute method.
        As should be clear from the above however that won't work either (or at all), so I'll have to revert that too and replace it with the described new solution as well.
        I'll add a comment therefore on PLUTO-600 indicating its solution is replaced by this one.

        Show
        Ate Douma added a comment - It turns out that while this change was needed to handle special cases on Websphere (e.g. like a forwarded portlet request invoking a servlet include request), this actually broke the same special cases on Tomcat Tomcat has a very peculiar handling of internal dispatcher state where it "injects" this current state within a request wrapper (org.apache.catalina.core.ApplicationHttpRequest). This means that these (there are actually two specific internal attributes used) attributes can only be retrieved reliably directly from the current request its parent (Tomcat "injects" the requestwrapper within the current request). Delegating the getAttribute call first to the PortletRequest (which by default delegates to the container SPI PortletRequestContext implementation) thus is not going to work out. And especially not when this PortletRequestContext itself delegates again to its wrapped servlet request ... as that one doesn't have the Tomcat injected ApplicationHttpRequest! Truly very complicated and tricky stuff. Bottom line: such internal "injected" attributes simply cannot be "managed" by the portlet container and must be retrieved from the current HttpServletPortletRequestWrapper its parent. Trouble is: this is servlet container specific, and at least Tomcat specific. So, the previous solution which always first checked the current HttpServletPortletRequestWrapper broke on Websphere, the new solution always on Tomcat. The only solution I can think of is providing special handling for these (Tomcat and potentially other servlet containers) internal attributes. I'm reopening this issue for both 2.0.3 and 2.1.0 versions to add handling for these servlet container managed attributes (meaning: not manageable by the portlet container). By default I'll add handling for these Tomcat/Catalina internal attribute names: "org.apache.catalina.core.DISPATCHER_TYPE" and "org.apache.catalina.core.DISPATCHER_REQUEST_PATH". As these are very Tomcat specific, these default "servletContainerManagedAttributes" (stored in a HashSet) will not interfere with other servlet containers. If other servlet containers turn out needing similar/likewise special attribute handling "protection", a static initializer method can be called (e.g. like using Spring injection) to (re)set this attributes set. Note: with PLUTO-600 Eric Dalquist slightly simplified the previous handling for Pluto 2.1.0, fully delegating the getAttribute call to the PortletRequest.getAttribute method. As should be clear from the above however that won't work either (or at all), so I'll have to revert that too and replace it with the described new solution as well. I'll add a comment therefore on PLUTO-600 indicating its solution is replaced by this one.

          People

          • Assignee:
            Ate Douma
            Reporter:
            Ate Douma
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development