MyFaces Orchestra
  1. MyFaces Orchestra
  2. ORCHESTRA-17

RequestParameterFacesContextFactory only works with HttpServletResponse

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0, 1.1
    • Fix Version/s: 1.4
    • Labels:
      None
    • Environment:
      Liferay Portlet Container

      Description

      The following snippet wrongly casts to HttpServletResponse, therefore portlet-environments will not work:

      if (response instanceof HttpServletResponse)
      {
      HttpServletRequest httpServletRequest = (HttpServletRequest) request;

      // Wrap this request only if something else (eg a RequestParameterServletFilter) has not already wrapped it.
      if (!Boolean.TRUE.equals(httpServletRequest.getAttribute(RequestParameterServletFilter.REQUEST_PARAM_FILTER_CALLED)))
      {

      I will commit a solution very soon.

      regards,

      Martin

      1. ORCHESTRA-17.patch
        25 kB
        Bernhard Huemer

        Issue Links

          Activity

          Hide
          Martin Marinschek added a comment -

          Patch committed. The patch relies on decorating the external-context instead of the servlet-response.

          regards,

          Martin

          Show
          Martin Marinschek added a comment - Patch committed. The patch relies on decorating the external-context instead of the servlet-response. regards, Martin
          Hide
          Mario Ivankovits added a comment -

          If I understand your patch correctly this thing will no longer work with plain jsp, will it?

          Some applications mix jsp and jsf pages, even if the jsp page is just a "bridge" or something like that. I (we) wanted to support such setup.

          But probably I missed some things

          Show
          Mario Ivankovits added a comment - If I understand your patch correctly this thing will no longer work with plain jsp, will it? Some applications mix jsp and jsf pages, even if the jsp page is just a "bridge" or something like that. I (we) wanted to support such setup. But probably I missed some things
          Hide
          Martin Marinschek added a comment -

          Hi Mario,

          ok, I see - I didn't think plain JSPs where served out by the FacesServlet - we are only wrapping the FacesContext here, right, so how did this ever work with plain JSPs?

          If I understand your class comments correctly, the filter should be used for plain JSPs?

          regards,

          Martin

          Show
          Martin Marinschek added a comment - Hi Mario, ok, I see - I didn't think plain JSPs where served out by the FacesServlet - we are only wrapping the FacesContext here, right, so how did this ever work with plain JSPs? If I understand your class comments correctly, the filter should be used for plain JSPs? regards, Martin
          Hide
          Simon Kitching added a comment -

          Wrapping the ExternalContext causes problems as methods added in JSF1.2 are not correctly delegated.

          In particular, this breaks orchestra+trinidad, as reported by Bruno Aranda:

          java.lang.UnsupportedOperationException at
          >> javax.faces.context.ExternalContext.setRequest(ExternalContext.java:124)
          >> at
          >> org.apache.myfaces.trinidadinternal.config.GlobalConfiguratorImpl._isSetRequestBugPresent(GlobalConfiguratorImpl.java:490)
          >> at
          >> org.apache.myfaces.trinidadinternal.config.GlobalConfiguratorImpl.getExternalContext(GlobalConfiguratorImpl.java:309)
          >> at
          >> org.apache.myfaces.trinidadinternal.context.FacesContextFactoryImpl$CacheRenderKit.(FacesContextFactoryImpl.java:86)
          >> at
          >> org.apache.myfaces.trinidadinternal.context.FacesContextFactoryImpl.getFacesContext(FacesContextFactoryImpl.java:64)
          >> at
          >> javax.faces.webapp.FacesServlet.prepareFacesContext(FacesServlet.java:234)

          In particular, invoking setRequest on the wrapper will end up calling the ExternalContext base class method, not the implementation on the wrapped object.

          Show
          Simon Kitching added a comment - Wrapping the ExternalContext causes problems as methods added in JSF1.2 are not correctly delegated. In particular, this breaks orchestra+trinidad, as reported by Bruno Aranda: java.lang.UnsupportedOperationException at >> javax.faces.context.ExternalContext.setRequest(ExternalContext.java:124) >> at >> org.apache.myfaces.trinidadinternal.config.GlobalConfiguratorImpl._isSetRequestBugPresent(GlobalConfiguratorImpl.java:490) >> at >> org.apache.myfaces.trinidadinternal.config.GlobalConfiguratorImpl.getExternalContext(GlobalConfiguratorImpl.java:309) >> at >> org.apache.myfaces.trinidadinternal.context.FacesContextFactoryImpl$CacheRenderKit.(FacesContextFactoryImpl.java:86) >> at >> org.apache.myfaces.trinidadinternal.context.FacesContextFactoryImpl.getFacesContext(FacesContextFactoryImpl.java:64) >> at >> javax.faces.webapp.FacesServlet.prepareFacesContext(FacesServlet.java:234) In particular, invoking setRequest on the wrapper will end up calling the ExternalContext base class method, not the implementation on the wrapped object.
          Hide
          Simon Kitching added a comment -

          ORCHESTRA-17 has been reverted, in order to resolve ORCHESTRA-24

          Show
          Simon Kitching added a comment - ORCHESTRA-17 has been reverted, in order to resolve ORCHESTRA-24
          Hide
          Bernhard Huemer added a comment -

          I've created a patch that is similiar to the one Martin has provided a while ago but I've extended it a little bit as it is unlike the previous version now compatible with both JSF 1.1 and JSF 1.2 (thanks to a small "hack"). Even though I could commit it myself I'd like someone else to review it (Simon or Mario?). I've already tested it with Apache Pluto and the JSR-301 Portlet Bridge.

          Show
          Bernhard Huemer added a comment - I've created a patch that is similiar to the one Martin has provided a while ago but I've extended it a little bit as it is unlike the previous version now compatible with both JSF 1.1 and JSF 1.2 (thanks to a small "hack"). Even though I could commit it myself I'd like someone else to review it (Simon or Mario?). I've already tested it with Apache Pluto and the JSR-301 Portlet Bridge.
          Hide
          Bernhard Huemer added a comment -

          The patch I've mentioned previously. It enables one to use Orchestra in a portal environment assuming that the JSR 301 Portlet Bridge will be used.

          Show
          Bernhard Huemer added a comment - The patch I've mentioned previously. It enables one to use Orchestra in a portal environment assuming that the JSR 301 Portlet Bridge will be used.
          Hide
          Martin Marinschek added a comment -

          Hi Leonardo,

          can you please check if this can be closed (also verifying your solution with Bernhard's approach)?

          regards,

          Martin

          Show
          Martin Marinschek added a comment - Hi Leonardo, can you please check if this can be closed (also verifying your solution with Bernhard's approach)? regards, Martin
          Hide
          Leonardo Uribe added a comment -

          I have checked the patch available here and it do the same as ORCHESTRA-15, but in a different way.

          To make it work it does two things:

          1. Wrap ExternalContext.encodeActionURL and ExternalContext.encodeResourceURL to include the required params to make orchestra work.
          2. Check if it is used jsf 1.1 or jsf 1.2 to wrap ExternalContext properly.

          The problem with this approach is that set jsf 1.2 in orchestra pom.xml, so there is a risk to program with jsf 1.2 specific api and make orchestra incompatible with jsf 1.1

          The difference in ORCHESTRA-15 is that the wrapped methods that are jsf 1.2 specific are called through reflection. This gives flexibility at cost of performance, but in the long term it is preferred. My opinion is it is better to create a jsf 1.2 specific jar like in tomahawk and fix the performance issue there.

          In conclusion we can close this issue, because the solution a solution for this one was already applied on ORCHESTRA-15. Checking this stuff in deep in liferay, I have found other issue that will be committed in ORCHESTRA-15. Also, I have planned document portlet stuff better so I'll commit the remaining stuff in ORCHESTRA-15 soon

          Show
          Leonardo Uribe added a comment - I have checked the patch available here and it do the same as ORCHESTRA-15 , but in a different way. To make it work it does two things: 1. Wrap ExternalContext.encodeActionURL and ExternalContext.encodeResourceURL to include the required params to make orchestra work. 2. Check if it is used jsf 1.1 or jsf 1.2 to wrap ExternalContext properly. The problem with this approach is that set jsf 1.2 in orchestra pom.xml, so there is a risk to program with jsf 1.2 specific api and make orchestra incompatible with jsf 1.1 The difference in ORCHESTRA-15 is that the wrapped methods that are jsf 1.2 specific are called through reflection. This gives flexibility at cost of performance, but in the long term it is preferred. My opinion is it is better to create a jsf 1.2 specific jar like in tomahawk and fix the performance issue there. In conclusion we can close this issue, because the solution a solution for this one was already applied on ORCHESTRA-15 . Checking this stuff in deep in liferay, I have found other issue that will be committed in ORCHESTRA-15 . Also, I have planned document portlet stuff better so I'll commit the remaining stuff in ORCHESTRA-15 soon

            People

            • Assignee:
              Leonardo Uribe
              Reporter:
              Martin Marinschek
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development