facelets
  1. facelets
  2. FACELETS-205

Bug in FaceletViewHandler.getRenderedViewId()

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Cannot Reproduce
    • Affects Version/s: Facelets 1.1
    • Fix Version/s: early access
    • Component/s: impl
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      205

      Description

      Hello,

      I discovered a bug in FaceletViewHandler.getRenderedViewId() today.

      Basically, the view ID calculation is incorrect due to a call to
      String.replaceFirst(). This method expects two parameters, a regular expression
      and a replacement String.

      The Facelets view handler does not escape the first argument and therefore the
      result of the string substitution is not as expected.

      Here is the code in question:

      protected String getRenderedViewId(FacesContext context, String actionId) {
      ExternalContext extCtx = context.getExternalContext();
      String viewId = actionId;
      if (extCtx.getRequestPathInfo() == null)

      { String facesSuffix = actionId.substring(actionId.lastIndexOf('.')); String viewSuffix = this.getDefaultSuffix(context); viewId = actionId.replaceFirst(facesSuffix, viewSuffix); }

      if (log.isLoggable(Level.FINE))

      { log.fine("ActionId -> ViewId: " + actionId + " -> " + viewId); }

      return viewId;
      }

      This line is the problem:

      viewId = actionId.replaceFirst(facesSuffix, viewSuffix);

      To reproduce this issue, try setting the following paramters:

      <context-param>
      <param-name>javax.faces.DEFAULT_SUFFIX</param-name>
      <param-value>.jsf</param-value>
      </context-param>
      <context-param>
      <param-name>facelets.VIEW_MAPPINGS</param-name>
      <param-value>*.jsf</param-value>
      </context-param>
      <servlet-mapping>
      <servlet-name>Faces Servlet</servlet-name>
      <url-pattern>*.jsf</url-pattern>
      </servlet-mapping>

      Create a simple JSF page as follows in a web application with the following
      directory layout:

      • SomeWebapp
      • /WEB-INF/...
      • index.jsf
      • subdir/index.jsf
      • subdir/jsfdemo/index.jsf

      Deploy this as the ROOT web application in your app server then try to access:

      http://localhost:8080/subdir/jsfdemo/index.jsf

      You should get a 404 error reporting that
      http://localhost:8080/subdir.jsfdemo/index.jsf is not found.

      This is due to the unescaped regular expression subsitution in getRenderedViewId().

      The Facelets view handler method receives the following parameters:

      • the current FacesContext
      • actionId = /subdir/jsfdemo/index.jsf

      In my setup the default suffix is ".jsf" and Facelets determines that the view
      suffix is also ".jsf".

      Next, viewId is assigned the value of actionId.replaceFirst(facesSuffix,
      viewSuffix).

      Since ".jsf" is interpreted as a regular expression, this means "any single
      character followed by jsf".

      Therefore, "/subdir/jsfdemo/index.jsf" becomes "/subdir.jsfdemo/index.jsf".

      A simple solution might be:

      String facesSuffix = actionId.substring(actionId.lastIndexOf('.'));
      String viewSuffix = this.getDefaultSuffix(context);
      viewId = actionId.replaceFirst(facesSuffix + "$", viewSuffix);

      A non-regex alternative:

      int index = actionId.lastIndexOf('.');
      String facesSuffix = actionId.substring(index);
      String viewSuffix = this.getDefaultSuffix(context);
      String mapping1 = actionId.substring(index);
      String mapping2 = mapping1.replace(facesSuffix, viewSuffix);
      viewId = actionId.replace(mapping1, mapping2);

      I have rebuilt Facelets using both of these changes and tested them both
      successfully for this use case.

      Ian


      JavaServer Faces for Dreamweaver
      http://www.jsftoolbox.com

        Activity

        Hide
        Ian Hlavats added a comment -

        Thanks for your time on this guys.

        I agree - it would be great to see this issue resolved.

        I have already seen this bug propagated to other products/frameworks derived
        from Facelets.

        Something this simple to fix should not really be a problem right?

        Show
        Ian Hlavats added a comment - Thanks for your time on this guys. I agree - it would be great to see this issue resolved. I have already seen this bug propagated to other products/frameworks derived from Facelets. Something this simple to fix should not really be a problem right?
        Hide
        Ed Burns added a comment -

        While there is no need to fix this for JSF 2.0 containers, Ryan points out that
        there is still value for prior containers. Therefore, I'm re-opening this and
        marking it LATER.

        Show
        Ed Burns added a comment - While there is no need to fix this for JSF 2.0 containers, Ryan points out that there is still value for prior containers. Therefore, I'm re-opening this and marking it LATER.
        Hide
        Ed Burns added a comment -

        While there is no need to fix this for JSF 2.0 containers, Ryan points out that
        there is still value for prior containers. Therefore, I'm re-opening this and
        marking it LATER.

        Show
        Ed Burns added a comment - While there is no need to fix this for JSF 2.0 containers, Ryan points out that there is still value for prior containers. Therefore, I'm re-opening this and marking it LATER.
        Hide
        Ed Burns added a comment -

        Re-opening.

        Show
        Ed Burns added a comment - Re-opening.
        Hide
        Ed Burns added a comment -

        This seems to work with the latest facelets HEAD code.

        Show
        Ed Burns added a comment - This seems to work with the latest facelets HEAD code.

          People

          • Assignee:
            Ed Burns
            Reporter:
            Ian Hlavats
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: