MyFaces Orchestra
  1. MyFaces Orchestra
  2. ORCHESTRA-13

RequestParameterProviderManager fails when template URL includes EL expressions

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0
    • Fix Version/s: 1.1
    • Labels:
      None

      Description

      As reported by Jonas Esser on the mailing list, a url of form
      http://foo.example?productId=#

      {var}

      fails badly; the final url is:
      http://foo.example?productId=&contextId=1val

      This occurs in the "petstore" orchestra example, when viewing a product's details.

      The problem is that RequestParameterProvidedManager thinks the "#" is a url fragment marker. Query params go before fragment markers, eg
      http://foo.example?productId=5#anchor
      does correctly become
      http://foo.example?productId=5&contextId=1#anchor

      But #

      {...}

      is NOT an anchor. In the petstore example, the url can be found in faces-config.xml:
      <navigation-case>
      <from-outcome>ProductDetails</from-outcome>
      <to-view-id>/mops/ProductDetail.jsp?productId=#

      {param.productId}

      </to-view-id>
      <redirect/>
      </navigation-case>

        Activity

        Hide
        Simon Kitching added a comment -

        Ok, now fixed.

        This is a problem that was actually introduced after the 1.0 release. Commit r586772 to RequestParameterProviderManager added support for URLs with fragment markers (ie #anchor on the end), but this broke urls with EL expressions.

        However the fix applied for this is to instead ensure that EL expressions are evaluated before RequestParameterProviderManager runs rather than after; that seems cleaner than trying to explicitly handle JSF expression patterns in the RequestParameterProviderManager code (which is supposed to be framework-independent).

        The committed patch does have a potential side-effect: all calls to viewHandler.getActionURL now expand any EL expressions encoded in the viewId parameter. In practice, this probably never happens. One possible caller of this method is the Form component renderer, which uses this method to determine what its rendered action attribute should contain - but that should never contain an unexpanded EL expression.

        Another effect of this patch is that the UrlParameterNavigationHandler class is now completely empty; users can skip configuring this if they want. However the class has been left there to avoid breaking existing configurations.

        Show
        Simon Kitching added a comment - Ok, now fixed. This is a problem that was actually introduced after the 1.0 release. Commit r586772 to RequestParameterProviderManager added support for URLs with fragment markers (ie #anchor on the end), but this broke urls with EL expressions. However the fix applied for this is to instead ensure that EL expressions are evaluated before RequestParameterProviderManager runs rather than after; that seems cleaner than trying to explicitly handle JSF expression patterns in the RequestParameterProviderManager code (which is supposed to be framework-independent). The committed patch does have a potential side-effect: all calls to viewHandler.getActionURL now expand any EL expressions encoded in the viewId parameter. In practice, this probably never happens. One possible caller of this method is the Form component renderer, which uses this method to determine what its rendered action attribute should contain - but that should never contain an unexpanded EL expression. Another effect of this patch is that the UrlParameterNavigationHandler class is now completely empty; users can skip configuring this if they want. However the class has been left there to avoid breaking existing configurations.
        Hide
        Simon Kitching added a comment -

        I've done some debugging on this.

        After clicking on the "product details" link of the petstore, the stack trace is like this:

        RequestParameterProviderManager.encodeAndAttachParameters(String) line: 111
        RequestParameterResponseWrapper.encodeURL(String) line: 42
        ServletExternalContextImpl.encodeActionURL(String) line: 386
        UrlParameterNavigationHandler$1$1(ExternalContextWrapper).encodeActionURL(String) line: 49
        RedirectTrackerExternalContextWrapper.encodeActionURL(String) line: 53
        NavigationHandlerImpl.handleNavigation(FacesContext, String, String) line: 93
        RedirectTrackerNavigationHandler.handleNavigation(FacesContext, String, String) line: 40
        UrlParameterNavigationHandler.handleNavigation(FacesContext, String, String) line: 54
        ActionListenerImpl.processAction(ActionEvent) line: 82

        The url parameter is:
        ProductDetail.faces?productId=#

        {param.productId}

        so the encodeAndAttachParameters method then proceeds to stuff the url up.

        Here is the point at which the EL expansion is done (later):

        UrlParameterNavigationHandler.interceptRedirect(FacesContext, String) line: 91
        UrlParameterNavigationHandler$1$1.redirect(String) line: 62
        RedirectTrackerExternalContextWrapper.redirect(String) line: 222
        NavigationHandlerImpl.handleNavigation(FacesContext, String, String) line: 93
        RedirectTrackerNavigationHandler.handleNavigation(FacesContext, String, String) line: 40
        UrlParameterNavigationHandler.handleNavigation(FacesContext, String, String) line: 54
        ActionListenerImpl.processAction(ActionEvent) line: 82

        In short, the EL expansion only occurs when ExternalContext.sendRedirect is invoked by the "real" NavigationHandlerImpl. But the navigation handler invokes that only after it thinks it has built the full url to redirect to, ie after it has called encodeActionUrl. So encodeActionUrl is always called before EL expansion occurs.

        A workaround for JSF12 is probably to use $

        {...}

        form EL expressions in navigation cases, as it is the "#" that is confusing things.

        I wonder if the EL expansion can just be moved into the RequestParameterProviderManager...

        Show
        Simon Kitching added a comment - I've done some debugging on this. After clicking on the "product details" link of the petstore, the stack trace is like this: RequestParameterProviderManager.encodeAndAttachParameters(String) line: 111 RequestParameterResponseWrapper.encodeURL(String) line: 42 ServletExternalContextImpl.encodeActionURL(String) line: 386 UrlParameterNavigationHandler$1$1(ExternalContextWrapper).encodeActionURL(String) line: 49 RedirectTrackerExternalContextWrapper.encodeActionURL(String) line: 53 NavigationHandlerImpl.handleNavigation(FacesContext, String, String) line: 93 RedirectTrackerNavigationHandler.handleNavigation(FacesContext, String, String) line: 40 UrlParameterNavigationHandler.handleNavigation(FacesContext, String, String) line: 54 ActionListenerImpl.processAction(ActionEvent) line: 82 The url parameter is: ProductDetail.faces?productId=# {param.productId} so the encodeAndAttachParameters method then proceeds to stuff the url up. Here is the point at which the EL expansion is done (later): UrlParameterNavigationHandler.interceptRedirect(FacesContext, String) line: 91 UrlParameterNavigationHandler$1$1.redirect(String) line: 62 RedirectTrackerExternalContextWrapper.redirect(String) line: 222 NavigationHandlerImpl.handleNavigation(FacesContext, String, String) line: 93 RedirectTrackerNavigationHandler.handleNavigation(FacesContext, String, String) line: 40 UrlParameterNavigationHandler.handleNavigation(FacesContext, String, String) line: 54 ActionListenerImpl.processAction(ActionEvent) line: 82 In short, the EL expansion only occurs when ExternalContext.sendRedirect is invoked by the "real" NavigationHandlerImpl. But the navigation handler invokes that only after it thinks it has built the full url to redirect to, ie after it has called encodeActionUrl. So encodeActionUrl is always called before EL expansion occurs. A workaround for JSF12 is probably to use $ {...} form EL expressions in navigation cases, as it is the "#" that is confusing things. I wonder if the EL expansion can just be moved into the RequestParameterProviderManager...
        Hide
        Mario Ivankovits added a comment -

        But the RequestParameterProvidedManager should not see any url with el-expressions in it.
        The UrlParameterNavigationHandler should have replaced them all.

        If this didn't work I see two possible problems:

        1) UrlParameterNavigationHandler is not configured correctly (needs to be done manually, is not setup by orchestra)
        2) The UrlParameterNavigationHandler is broken

        Show
        Mario Ivankovits added a comment - But the RequestParameterProvidedManager should not see any url with el-expressions in it. The UrlParameterNavigationHandler should have replaced them all. If this didn't work I see two possible problems: 1) UrlParameterNavigationHandler is not configured correctly (needs to be done manually, is not setup by orchestra) 2) The UrlParameterNavigationHandler is broken

          People

          • Assignee:
            Unassigned
            Reporter:
            Simon Kitching
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development