Pluto
  1. Pluto
  2. PLUTO-529

PortletRequest/PortletResponse implementations extending HttpServletRequest/Response wrappers causes "indentity" problems when accessed from servlets

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.0
    • Fix Version/s: 2.0.0
    • Component/s: portlet container
    • Labels:
      None

      Description

      Below copied (in part) from email discussion on the Pluto dev list, see also: http://www.nabble.com/More-required-Pluto-2.0-SPI-and-implementation-refactoring-issues-td21973310.html

          • InternalPortletRequest/Response implementations (and subclasses thereof) extending HttpServletRequest/ResponseWrapper
            This solution (dating back from Pluto 1.0 implementation) has a very tricky but serious flaw.
            By using a single instance HttpServletRequestWrapper instance for both the PortletRequest and dispatched ServletRequest,
            a dispatched servlet retrieving the current PortletRequest (or Response) using HttpServletRequest.getAttribute("javax.portlet.request") as
            specified by the Portlet specification (PLT.19.3.2), will actually return the current HttpServletRequestWrapper itself again.
            So far, nothing wrong yet.
            But, as the InternalPortletRequestImpl (which is the real implementation class) also maintains internal instance state concerning its dispatched state and based upon that decides how overlapping methods need to behave, the PortletRequest object
            retrieved like this from within a servlet environment actually behaves as a dispatched ServletRequest.
            This is not compliant with the Portlet specification, even if the current JSR-286 TCK doesn't (properly) test against this.
            The only solution to solve this is not using a piggy back solution for the dispatched ServletRequest/Response objects, but use independent
            instances for the PortletRequest/Response and wrap these within the dispatched ServletRequest/Response objects.

      This is a rather big change, but really required.

      On the bright side, doing so will result in a much more readable/maintainable solution as the current implementation has to maintain some tricky state flags to keep track of its "identity". Getting rid of all that and moving the dispatched servlet specific handling in separate
      classes will make this much easier and transparent to deal with.

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        4s 1 Ate Douma 19/Feb/09 15:02
        In Progress In Progress Resolved Resolved
        22d 12h 22m 1 Ate Douma 14/Mar/09 03:25
        Mark Thomas made changes -
        Workflow Default workflow, editable Closed status [ 12564879 ] jira [ 12585964 ]
        Mark Thomas made changes -
        Workflow jira [ 12452968 ] Default workflow, editable Closed status [ 12564879 ]
        Ate Douma made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Ate Douma added a comment -

        All fixed and implemented with the recent big bang refactoring and final commits r753592 and r753593

        Show
        Ate Douma added a comment - All fixed and implemented with the recent big bang refactoring and final commits r753592 and r753593
        Ate Douma made changes -
        Description Below copied (in part) from email discussion on the Pluto dev list, see also: http://www.nabble.com/More-required-Pluto-2.0-SPI-and-implementation-refactoring-issues-td21973310.html

        *** InternalPortletRequest/Response implementations (and subclasses thereof) extending HttpServletRequest/ResponseWrapper
        This solution (dating back from Pluto 1.0 implementation) has a very tricky but serious flaw.
        By using a single instance HttpServletRequestWrapper instance for both the PortletRequest and dispatched ServletRequest,
        a dispatched servlet retrieving the current PortletRequest (or Response) using HttpServletRequest.getAttribute("javax.portlet.request") as
        specified by the Portlet specification (PLT.19.3.2), will actually return the *current* HttpServletRequestWrapper itself again.
        So far, nothing wrong yet.
        But, as the InternalPortletRequestImpl (which is the real implementation class) also maintains internal instance state concerning its dispatched state and based upon that decides how overlapping methods need to behave, the PortletRequest object
        retrieved like this from within a servlet environment actually behaves as a dispatched ServletRequest.
        This is *not* compliant with the Portlet specification, even if the current JSR-286 TCK doesn't (properly) test against this.
        The only solution to solve this is *not* using a piggy back solution for the dispatched ServletRequest/Response objects, but use independent
        instances for the PortletRequest/Response and wrap these within the dispatched ServletRequest/Response objects.

        This is a rather big change, but really required.

        On the bright side, doing so will result in a much more readable/maintainable solution as the current implementation has to maintain some tricky state flags to keep track of its "identity". Getting rid of all that and moving the dispatched servlet specific handling in separate
        classes will make this much easier and transparent to deal with.

        (side note: I actually wrote a test case for this and this spec requirement is broken in most other open-source portlet containers as well!)
        Below copied (in part) from email discussion on the Pluto dev list, see also: http://www.nabble.com/More-required-Pluto-2.0-SPI-and-implementation-refactoring-issues-td21973310.html

        *** InternalPortletRequest/Response implementations (and subclasses thereof) extending HttpServletRequest/ResponseWrapper
        This solution (dating back from Pluto 1.0 implementation) has a very tricky but serious flaw.
        By using a single instance HttpServletRequestWrapper instance for both the PortletRequest and dispatched ServletRequest,
        a dispatched servlet retrieving the current PortletRequest (or Response) using HttpServletRequest.getAttribute("javax.portlet.request") as
        specified by the Portlet specification (PLT.19.3.2), will actually return the *current* HttpServletRequestWrapper itself again.
        So far, nothing wrong yet.
        But, as the InternalPortletRequestImpl (which is the real implementation class) also maintains internal instance state concerning its dispatched state and based upon that decides how overlapping methods need to behave, the PortletRequest object
        retrieved like this from within a servlet environment actually behaves as a dispatched ServletRequest.
        This is *not* compliant with the Portlet specification, even if the current JSR-286 TCK doesn't (properly) test against this.
        The only solution to solve this is *not* using a piggy back solution for the dispatched ServletRequest/Response objects, but use independent
        instances for the PortletRequest/Response and wrap these within the dispatched ServletRequest/Response objects.

        This is a rather big change, but really required.

        On the bright side, doing so will result in a much more readable/maintainable solution as the current implementation has to maintain some tricky state flags to keep track of its "identity". Getting rid of all that and moving the dispatched servlet specific handling in separate
        classes will make this much easier and transparent to deal with.
        Ate Douma made changes -
        Field Original Value New Value
        Status Open [ 1 ] In Progress [ 3 ]
        Ate Douma created issue -

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development