ICEfaces
  1. ICEfaces
  2. ICE-9200

ace:fileEntry - Avoid capturing all multi-part/Servlet 3.0 requests

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.3
    • Fix Version/s: EE-3.3.0.GA, 4.0.BETA, 4.0
    • Component/s: ICE-Components
    • Labels:
      None
    • Environment:
      ICEfaces ACE, ICEmobile
    • Assignee Priority:
      P1
    • Affects:
      Documentation (User Guide, Ref. Guide, etc.), Compatibility/Configuration

      Description

      FileEntryResourceHandler wraps all requests of type multipart, but this interferes with other multipart uses of the request.

        Activity

        Ted Goddard created issue -
        Ted Goddard made changes -
        Field Original Value New Value
        Assignee Mark Collette [ mark.collette ]
        Hide
        Ted Goddard added a comment - - edited

        Since FileEntry posts back to the same URL as the page it is on, it should be possible to record this URL and only wrap those matching incoming requests. A future version of FileEntry can make use of Servlet 3.0 and also not wrap the request when Servlet 3.0 API is available.

        Show
        Ted Goddard added a comment - - edited Since FileEntry posts back to the same URL as the page it is on, it should be possible to record this URL and only wrap those matching incoming requests. A future version of FileEntry can make use of Servlet 3.0 and also not wrap the request when Servlet 3.0 API is available.
        Hide
        Mark Collette added a comment -

        Within the ACE code, we could use the FileEntryRenderer, or a components system event listener, to detect the fileEntry being in a given view, and record that viewId in the session. When the FileEntryResourceHandler runs, it could see if its current viewId is in the session list, and then conditionally process the request. This would allow for a granular exclusion, so that blended apps that have ICEmobile pages and ACE pages could coexist.

        Show
        Mark Collette added a comment - Within the ACE code, we could use the FileEntryRenderer, or a components system event listener, to detect the fileEntry being in a given view, and record that viewId in the session. When the FileEntryResourceHandler runs, it could see if its current viewId is in the session list, and then conditionally process the request. This would allow for a granular exclusion, so that blended apps that have ICEmobile pages and ACE pages could coexist.
        Mark Collette made changes -
        Assignee Mark Collette [ mark.collette ] Ted Goddard [ ted.goddard ]
        Hide
        Ted Goddard added a comment -

        The problem mysteriously disappeared in testing today. This was due to testing with icefaces-ace.jar and icefaces.jar vs icefaces-ace.jar and icefaces-ee.jar. The change in .jar naming caused different ordering for the ResourceHandler, resulting in Servlet 3.0 Part extraction (in icefaces.jar/icefaces-ee.jar) occurring before or after FileEntryPhaseListener wrapped the request.

        Show
        Ted Goddard added a comment - The problem mysteriously disappeared in testing today. This was due to testing with icefaces-ace.jar and icefaces.jar vs icefaces-ace.jar and icefaces-ee.jar. The change in .jar naming caused different ordering for the ResourceHandler, resulting in Servlet 3.0 Part extraction (in icefaces.jar/icefaces-ee.jar) occurring before or after FileEntryPhaseListener wrapped the request.
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #34715 Fri May 03 17:09:24 MDT 2013 ted.goddard optional ExternalContext that will disable FileEntryResourceHandler (ICE-9200)
        Files Changed
        Commit graph ADD /icemobile/trunk/icemobile/jsf/components/component/src/org/icemobile/context/ACECompatibleExternalContextFactory.java
        Commit graph ADD /icemobile/trunk/icemobile/jsf/components/component/src/org/icemobile/context/ACECompatibleExternalContext.java
        Hide
        Ted Goddard added a comment -

        Add the following to the faces-config.xml in the ICEmobile application to disable FileEntryResourceHandler and allow uploads from ICEmobile-SX:

        <factory>
        <external-context-factory>
        org.icemobile.context.ACECompatibleExternalContextFactory
        </external-context-factory>
        </factory>

        Alternatively, icefaces.jar and icefaces-ace.jar can be named so that .jar ordering causes icefaces.jar to be processed first.

        Mark suggested that faces-config ordering would be an interesting solution to this, but the faces-config ordering mechanism requires ordering relationships to be declared within the .jar files (which already exist).

        Note that this problem is likely to occur only with the unsupported configuration that combines open source icefaces-ace.jar with icefaces-ee.jar (but this could occur if a customer added the .jar to an existing EE application).

        Show
        Ted Goddard added a comment - Add the following to the faces-config.xml in the ICEmobile application to disable FileEntryResourceHandler and allow uploads from ICEmobile-SX: <factory> <external-context-factory> org.icemobile.context.ACECompatibleExternalContextFactory </external-context-factory> </factory> Alternatively, icefaces.jar and icefaces-ace.jar can be named so that .jar ordering causes icefaces.jar to be processed first. Mark suggested that faces-config ordering would be an interesting solution to this, but the faces-config ordering mechanism requires ordering relationships to be declared within the .jar files (which already exist). Note that this problem is likely to occur only with the unsupported configuration that combines open source icefaces-ace.jar with icefaces-ee.jar (but this could occur if a customer added the .jar to an existing EE application).
        Ted Goddard made changes -
        Assignee Ted Goddard [ ted.goddard ] Mark Collette [ mark.collette ]
        Hide
        Ted Goddard added a comment -

        Assigning to Mark to complete the strategy for wrapping requests for only those pages that contain a FileEntry component in ICEfaces 3.4.

        Show
        Ted Goddard added a comment - Assigning to Mark to complete the strategy for wrapping requests for only those pages that contain a FileEntry component in ICEfaces 3.4.
        Ken Fyten made changes -
        Assignee Priority P1 [ 10010 ]
        Ken Fyten made changes -
        Fix Version/s EE-3.3.0.GA [ 10572 ]
        Hide
        Mark Collette added a comment -

        Modify the form's post url to include a parameter that will indicate that the post is relevant to the ace:fileEntry component, so that if it's omitted, the FileEntryResourceHandler will leave the request alone, which will allow any other multipart component or Servlet 3.0 part request to continue to function.

        It would make sense to use the existing hidden input of "ice.fileEntry.ajaxResponse" = "true" to be the url parameter. This might allow for removing that hidden input field of that name, so that the FileEntryResourceHandler code that relies on it would use this url parameter as well.

        Show
        Mark Collette added a comment - Modify the form's post url to include a parameter that will indicate that the post is relevant to the ace:fileEntry component, so that if it's omitted, the FileEntryResourceHandler will leave the request alone, which will allow any other multipart component or Servlet 3.0 part request to continue to function. It would make sense to use the existing hidden input of "ice.fileEntry.ajaxResponse" = "true" to be the url parameter. This might allow for removing that hidden input field of that name, so that the FileEntryResourceHandler code that relies on it would use this url parameter as well.
        Hide
        Mark Collette added a comment -

        Having to take into account what was done in ICE-6371 for portlets compatibility, since that's already stomping over the form action with a portlets compatible url.

        Show
        Mark Collette added a comment - Having to take into account what was done in ICE-6371 for portlets compatibility, since that's already stomping over the form action with a portlets compatible url.
        Hide
        Mark Collette added a comment - - edited

        For Portlets, the code was using, if present, the javax.faces.encodedURL hidden input, which h:form renders as ExternalContext.encodePartialActionURL, if that's different from ExternalContext.encodeActionURL. Which probably means that encodePartialActionURL should always be right when we're using Ajax, regardless if using Portlets or not. Had to render on the server the URL with the ice.fileEntry.ajaxResponse param, and feed that to encodePartialActionURL, since you can't assume you can append params to what you get out of encodePartialActionURL, since some Portlet containers return a block box string from it.

        Added code on the server side to look for the parameter and only process the multipart if the parameter was set.

        Then, had to address ICE-6448, where we support javascript disabled file uploads, which means that we still need to support uploads where we don't set the parameter in the javascript. This is because there's no means to override the rendered form action from the server side, short of a major kludge.

        So, introduced the context parameter org.icefaces.ace.fileEntry.requireJavascript with default value true. This means not requiring javascript will allow for ignoring the lack of the parameter. So apps can either mix upload components xor support javascript disabled.

        Wiki page for the new context-param:
        http://www.icesoft.org/wiki/display/ICE/fileEntry.requireJavascript

        Show
        Mark Collette added a comment - - edited For Portlets, the code was using, if present, the javax.faces.encodedURL hidden input, which h:form renders as ExternalContext.encodePartialActionURL, if that's different from ExternalContext.encodeActionURL. Which probably means that encodePartialActionURL should always be right when we're using Ajax, regardless if using Portlets or not. Had to render on the server the URL with the ice.fileEntry.ajaxResponse param, and feed that to encodePartialActionURL, since you can't assume you can append params to what you get out of encodePartialActionURL, since some Portlet containers return a block box string from it. Added code on the server side to look for the parameter and only process the multipart if the parameter was set. Then, had to address ICE-6448, where we support javascript disabled file uploads, which means that we still need to support uploads where we don't set the parameter in the javascript. This is because there's no means to override the rendered form action from the server side, short of a major kludge. So, introduced the context parameter org.icefaces.ace.fileEntry.requireJavascript with default value true. This means not requiring javascript will allow for ignoring the lack of the parameter. So apps can either mix upload components xor support javascript disabled. Wiki page for the new context-param: http://www.icesoft.org/wiki/display/ICE/fileEntry.requireJavascript
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #35669 Thu May 30 22:35:28 MDT 2013 mark.collette ICE-9200 : Servlet 3.0 compatibility for FileEntry
        Files Changed
        Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/impl/event/BridgeSetup.java
        Commit graph MODIFY /icefaces3/trunk/icefaces/ace/component/src/org/icefaces/ace/component/fileentry/FileEntryResourceHandler.java
        Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
        Commit graph MODIFY /icefaces3/trunk/icefaces/ace/component/src/org/icefaces/ace/component/fileentry/FileEntryFormSubmit.java
        Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/util/EnvUtils.java
        Commit graph MODIFY /icefaces3/trunk/icefaces/ace/component/resources/icefaces.ace/fileentry/fileEntry.js
        Hide
        Mark Collette added a comment - - edited

        Will need to be tested with Portlets (ICE-6371 regression risk) and in a servlet application without javascript enabled (ICE-6448 regression risk). And of course will need to test a blended application that contains separate pages or forms with mobi upload components, with other pages or forms that contain ace:fileEntry, and ensure that each now works properly, without interfering with each other.

        icefaces3 trunk
        Subversion 35669

        Show
        Mark Collette added a comment - - edited Will need to be tested with Portlets ( ICE-6371 regression risk) and in a servlet application without javascript enabled (ICE-6448 regression risk). And of course will need to test a blended application that contains separate pages or forms with mobi upload components, with other pages or forms that contain ace:fileEntry, and ensure that each now works properly, without interfering with each other. icefaces3 trunk Subversion 35669
        Mark Collette made changes -
        Affects Documentation (User Guide, Ref. Guide, etc.),Compatibility/Configuration [ 10003, 10002 ]
        Hide
        Mark Collette added a comment -

        Tested, and confirmed working, in liferay-portal-6.1.1-ce-ga2 with liferay-faces-bridge-3.1.2-ga3.

        Show
        Mark Collette added a comment - Tested, and confirmed working, in liferay-portal-6.1.1-ce-ga2 with liferay-faces-bridge-3.1.2-ga3.
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #35987 Thu Jun 06 10:22:30 MDT 2013 mark.collette ICE-9200 : Servlet 3.0 compatibility for FileEntry
        Files Changed
        Commit graph MODIFY /icefaces3/trunk/icefaces/ace/component/src/org/icefaces/ace/component/fileentry/FileEntryResourceHandler.java
        Commit graph MODIFY /icefaces3/trunk/icefaces/ace/component/src/org/icefaces/ace/component/fileentry/FileEntryPhaseListener.java
        Commit graph MODIFY /icefaces3/trunk/icefaces/ace/component/src/org/icefaces/ace/component/fileentry/FileEntryFormSubmit.java
        Hide
        Mark Collette added a comment -

        Javascript disabled mode was not working (ICE-6448), so had to fix that.

        icefaces3 trunk
        Subversion 35987

        Show
        Mark Collette added a comment - Javascript disabled mode was not working (ICE-6448), so had to fix that. icefaces3 trunk Subversion 35987
        Hide
        Mark Collette added a comment -

        Tested working in javascript disabled mode.

        Worked through these issues unrelated to fileEntry, for testing the component in portlets:

        • ResponseWriter.endElement innocuous exception on portlet add
        • Double liferay faces jars in lib
        • RenderResponsePhaseListener ClassCastException

        icefaces EE 3.3 GA tag branch
        Subversion 36033

        Show
        Mark Collette added a comment - Tested working in javascript disabled mode. Worked through these issues unrelated to fileEntry, for testing the component in portlets: ResponseWriter.endElement innocuous exception on portlet add Double liferay faces jars in lib RenderResponsePhaseListener ClassCastException icefaces EE 3.3 GA tag branch Subversion 36033
        Hide
        Mark Collette added a comment -

        Further testing of FileEntry with Portlets raised these issues of ICE-9338, ICE-9339, which seem to be unrelated to the Servlet 3.0 work, so I'm marking this as resolved now.

        Show
        Mark Collette added a comment - Further testing of FileEntry with Portlets raised these issues of ICE-9338 , ICE-9339, which seem to be unrelated to the Servlet 3.0 work, so I'm marking this as resolved now.
        Mark Collette made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Ted Goddard added a comment -

        The workaround with ACECompatibleExternalContextFactory is no longer necessary.

        Show
        Ted Goddard added a comment - The workaround with ACECompatibleExternalContextFactory is no longer necessary.
        Ken Fyten made changes -
        Summary Servlet 3.0 compatibility for FileEntry ace:fileEntry - Avoid capturing all multi-part/Servlet 3.0 requests
        Hide
        Krashan Brahmanjara added a comment -

        This change destroy behaviour of old Icefaces apps with error

        SEVERE: Error Rendering View[/zad.jspx]
        java.lang.IllegalStateException: Component ID zadFrm:ice_fileEntry_encodedURL has already been found in the view.
        at com.sun.faces.util.Util.checkIdUniqueness(Util.java:987)
        at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971)
        at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971)
        at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971)
        at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971)
        at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971)
        at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971)
        at com.sun.faces.application.view.FaceletPartialStateManagementStrategy.saveView(FaceletPartialStateManagementStrategy.java:461)
        at com.sun.faces.application.StateManagerImpl.saveView(StateManagerImpl.java:89)
        at javax.faces.application.StateManager.getViewState(StateManager.java:593)
        at com.icesoft.faces.context.CompatDOMPartialViewContext$WriteViewStateMarkup.write(CompatDOMPartialViewContext.java:65)
        at org.icefaces.impl.util.DOMUtils.printNode(DOMUtils.java:378)
        at org.icefaces.impl.util.DOMUtils.printNode(DOMUtils.java:355)
        at org.icefaces.impl.util.DOMUtils.printNodeCDATA(DOMUtils.java:286)
        at org.icefaces.impl.context.DOMPartialViewContext.processPartial(DOMPartialViewContext.java:262)
        at javax.faces.component.UIViewRoot.encodeChildren(UIViewRoot.java:1004)
        at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1896)
        at com.sun.faces.application.view.FaceletViewHandlingStrategy.renderView(FaceletViewHandlingStrategy.java:425)
        at com.sun.faces.application.view.MultiViewHandler.renderView(MultiViewHandler.java:131)
        at javax.faces.application.ViewHandlerWrapper.renderView(ViewHandlerWrapper.java:337)
        at com.sun.faces.lifecycle.RenderResponsePhase.execute(RenderResponsePhase.java:120)
        at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:101)
        at com.sun.faces.lifecycle.LifecycleImpl.render(LifecycleImpl.java:219)
        at javax.faces.webapp.FacesServlet.service(FacesServlet.java:647)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:290)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
        at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:233)
        at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:191)
        at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:563)

        Show
        Krashan Brahmanjara added a comment - This change destroy behaviour of old Icefaces apps with error SEVERE: Error Rendering View [/zad.jspx] java.lang.IllegalStateException: Component ID zadFrm:ice_fileEntry_encodedURL has already been found in the view. at com.sun.faces.util.Util.checkIdUniqueness(Util.java:987) at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971) at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971) at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971) at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971) at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971) at com.sun.faces.util.Util.checkIdUniqueness(Util.java:971) at com.sun.faces.application.view.FaceletPartialStateManagementStrategy.saveView(FaceletPartialStateManagementStrategy.java:461) at com.sun.faces.application.StateManagerImpl.saveView(StateManagerImpl.java:89) at javax.faces.application.StateManager.getViewState(StateManager.java:593) at com.icesoft.faces.context.CompatDOMPartialViewContext$WriteViewStateMarkup.write(CompatDOMPartialViewContext.java:65) at org.icefaces.impl.util.DOMUtils.printNode(DOMUtils.java:378) at org.icefaces.impl.util.DOMUtils.printNode(DOMUtils.java:355) at org.icefaces.impl.util.DOMUtils.printNodeCDATA(DOMUtils.java:286) at org.icefaces.impl.context.DOMPartialViewContext.processPartial(DOMPartialViewContext.java:262) at javax.faces.component.UIViewRoot.encodeChildren(UIViewRoot.java:1004) at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1896) at com.sun.faces.application.view.FaceletViewHandlingStrategy.renderView(FaceletViewHandlingStrategy.java:425) at com.sun.faces.application.view.MultiViewHandler.renderView(MultiViewHandler.java:131) at javax.faces.application.ViewHandlerWrapper.renderView(ViewHandlerWrapper.java:337) at com.sun.faces.lifecycle.RenderResponsePhase.execute(RenderResponsePhase.java:120) at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:101) at com.sun.faces.lifecycle.LifecycleImpl.render(LifecycleImpl.java:219) at javax.faces.webapp.FacesServlet.service(FacesServlet.java:647) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:290) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:233) at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:191) at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:563)
        Hide
        Ted Goddard added a comment -

        Please attach a simple test case that reproduces the problem and indicate which ICEfaces versions it has been tested with (and which pass and which fail).

        Show
        Ted Goddard added a comment - Please attach a simple test case that reproduces the problem and indicate which ICEfaces versions it has been tested with (and which pass and which fail).
        Hide
        Krashan Brahmanjara added a comment -

        Comment and workaround attached to ICE-9885

        Show
        Krashan Brahmanjara added a comment - Comment and workaround attached to ICE-9885
        Ken Fyten made changes -
        Fix Version/s 4.0 [ 11382 ]
        Ken Fyten made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Mark Collette
            Reporter:
            Ted Goddard
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: