Uploaded image for project: 'PUBLIC - Liferay Faces'
  1. PUBLIC - Liferay Faces
  2. FACES-2945

The ResourceImpl.toRequestPath() method should not encode the return value

    Details

      Description

      The ResourceImpl.toRequestPath() method contains the following code:

      // In order to have Mojarra's ScriptRenderer and StylesheetRenderer function properly, this method first encodes
      // the URL returned by the wrapped resource.
      return facesContext.getExternalContext().encodeResourceURL(wrappedRequestPath);
      

      The comment implies that Mojarra's ScriptRenderer and StylesheetRenderer do not call ExternalContext.encodeResourceURL(String) with the result of calling Resource.toRequestPath(). However, the ScriptRenderer.java class in Mojarra 2.1.29-08 and the ScriptRenderer.java class in Mojarra 2.2.13 both do the encoding.

      It is therefore unnecessary and incorrect for ResourceImpl.toRequestPath() to encode the return value.

        Activity

        Hide
        kyle.stiemann Kyle Stiemann added a comment -

        Neil Griffin,
        I think we should roll this back unfortunately. Mojarra (usually) uses ExternalContext.encodeResourceURL(), but occasionally it uses Resource.getRequestPath(). Worse than that though is PrimeFaces and ICEfaces. Both libraries assume the Resource.getRequestPath() is the same as ExternalContext.encodeResourceURL() all over the place (RichFaces probably does it too, although I haven't checked yet). This change is probably too dangerous to make for those libraries and it would be better to roll it back. We have a nice check in BridgeURLResourceImpl which avoids encoding the URL twice anyway.

        Show
        kyle.stiemann Kyle Stiemann added a comment - Neil Griffin , I think we should roll this back unfortunately. Mojarra (usually) uses ExternalContext.encodeResourceURL() , but occasionally it uses Resource.getRequestPath() . Worse than that though is PrimeFaces and ICEfaces. Both libraries assume the Resource.getRequestPath() is the same as ExternalContext.encodeResourceURL() all over the place (RichFaces probably does it too, although I haven't checked yet). This change is probably too dangerous to make for those libraries and it would be better to roll it back. We have a nice check in BridgeURLResourceImpl which avoids encoding the URL twice anyway.
        Hide
        neil.griffin Neil Griffin added a comment -

        Closing this issue as "Won't Fix" but there are a variety of commits related to this issue in liferay-faces-bridge-impl.git that refactored the way we were doing things, bringing more clarity to the problems that were being solved.

        Show
        neil.griffin Neil Griffin added a comment - Closing this issue as "Won't Fix" but there are a variety of commits related to this issue in liferay-faces-bridge-impl.git that refactored the way we were doing things, bringing more clarity to the problems that were being solved.

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development

                Subcomponents