Pluto
  1. Pluto
  2. PLUTO-530

RequestDispatcher path query string parameter handling too limited and broken with nested dispatches

    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

          • RequestDispatcher path query string parameter handling
            PLT.19.4.1 specifies that "The request dispatching mechanism is responsible for aggregating query string parameters when forwarding or
            including requests".
            This requirement has been implemented very literally in Pluto: a PortletRequestDispatcher path query string is "stored" in the request instance and on subsequent access to the parameter map from within the invoked servlet parsed and merged on top of the portlet parametermap.
            This "manual" processing and merging of query string parameters however breaks the Servlet spec requirements!
            It goes wrong if from within the dispatched servlet a subsequent dispatch is performed with additional query string parameters.
            In the current solution, this possible usage hasn't been taken into account, with as result you'll always receive the same
            parametermap on subsequent (nested) dispatches (for example from within a JSP including another JSP).
            But, even if the servletrequest.getRequestDispatcher(path) would be overridden to "fix" this problem, that still won't solve it for two reasons:
      • ServletContext.getRequestDispatcher(path) cannot be overridden, therefore still leaving a "hole" in the solution
      • there is no "returned from dispatch" callback mechanism to "revert" the current parametermap after a dispatch

      Therefore, "manual" parsing and merging of dispatcher query string parameters cannot be used to implement this spec requirement.

      However, the correct way to implement this is actually extremely simple: just let the servlet container handle it all by itself!
      There is no need to "store", parse and merge dispatcher query string parameters, and in the servletrequest(wrapper).getParameterMap(), just return super.getParameterMap() which the container will have "injected" with the additional query string parameters merged already.
      Jetspeed-2 has used this solution from the beginning (with some additional fancy cache handling) and it just works as expected.
      Changing to this solution will dramatically simplify the current implementation, especially after the PortletRequest and ServletRequest
      implementations are split up (see: PLUTO-529).

      (side note: I actually wrote a testcase for this and this spec requirement is broken in most other open-source portlet containers as well!)

        Activity

        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

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development