ICEpush
  1. ICEpush
  2. PUSH-407

Add the lifetime of Browser-ID to the Set-Cookie header for Browser-ID

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: EE-4.0.0.GA, 4.1
    • Fix Version/s: EE-4.2.0.GA
    • Component/s: Push Library
    • Labels:
      None
    • Environment:
      ICEpush, Safari, iOS

      Description

      When using Safari on iOS, upon killing Safari the Browser-ID Cookie gets deleted as it is currently considered a Session Cookie. But with Cloud Push, we already want things like Browser-IDs, Push-IDs and Notify-Back-URIs to survive past the sessions. We should add the lifetime of the Browser-ID, set at the server-side, to the Set-Cookie header for the Browser-ID Cookie in order to do so.

        Activity

        Hide
        Jack Van Ooststroom added a comment -

        Sending icepush/core/src/main/java/org/icepush/Browser.java
        Sending icepush/core/src/main/java/org/icepush/LocalPushGroupManager.java
        Sending icepush/core/src/main/java/org/icepush/PushContext.java
        Sending icepush/core/src/main/java/org/icepush/servlet/BrowserDispatcher.java
        Sending icepush/util/src/main/java/org/icesoft/util/servlet/ServletContextConfiguration.java
        Transmitting file data .....
        Committed revision 48666.

        Show
        Jack Van Ooststroom added a comment - Sending icepush/core/src/main/java/org/icepush/Browser.java Sending icepush/core/src/main/java/org/icepush/LocalPushGroupManager.java Sending icepush/core/src/main/java/org/icepush/PushContext.java Sending icepush/core/src/main/java/org/icepush/servlet/BrowserDispatcher.java Sending icepush/util/src/main/java/org/icesoft/util/servlet/ServletContextConfiguration.java Transmitting file data ..... Committed revision 48666.
        Hide
        Jack Van Ooststroom added a comment -

        The Set-Cookie for header for the ice.push.browser Cookie now includes the Expires attribute, effectively changing this Cookie from a Session Cookie to a Persistent Cookie. Marking this one as FIXED.

        Show
        Jack Van Ooststroom added a comment - The Set-Cookie for header for the ice.push.browser Cookie now includes the Expires attribute, effectively changing this Cookie from a Session Cookie to a Persistent Cookie. Marking this one as FIXED.
        Hide
        Jack Van Ooststroom added a comment -

        Reopening due to the following reported UnsupportedOperationException thrown when using a Portlet environment:

        Caused by: java.lang.UnsupportedOperationException
               at org.icefaces.impl.push.servlet.ProxyHttpServletRequest.getServletContext(ProxyHttpServletRequest.java:110)
               at org.icepush.PushContext.createPushId(PushContext.java:166)
               at org.icefaces.impl.event.BridgeSetup.generateViewID(BridgeSetup.java:293)
               at org.icefaces.impl.event.BridgeSetup.assignViewID(BridgeSetup.java:283)
               at org.icefaces.impl.event.BridgeSetup.access$300(BridgeSetup.java:45)
               at org.icefaces.impl.event.BridgeSetup$AssignViewID.beforePhase(BridgeSetup.java:317)
               at com.sun.faces.lifecycle.Phase.handleBeforePhase(Phase.java:228)
               at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:99)
        

        Deryk's initial analysis:
        There are lots of unsupported methods in ProxyHttpServletRequest as we typically only implemented the minimum we needed for operating. However, the new call at org.icepush.PushContext.createPushId(PushContext.java:166) is doing this:

        cookie.setMaxAge((int)(Browser.getTimeout(request.getServletContext()) / 1000));

        This kind of servlet specific call is not portlet-friendly. Normally I’d recommend using the FacesContext API to do this kind of thing but maybe it’s not available in ICEpush? If not then it might be that we need to implement the method in the ProxyHttpServletRequest facade. However, since I’m not really sure how the Browser class is using the ServletContext, the problem could go deeper than that.

        Show
        Jack Van Ooststroom added a comment - Reopening due to the following reported UnsupportedOperationException thrown when using a Portlet environment: Caused by: java.lang.UnsupportedOperationException at org.icefaces.impl.push.servlet.ProxyHttpServletRequest.getServletContext(ProxyHttpServletRequest.java:110) at org.icepush.PushContext.createPushId(PushContext.java:166) at org.icefaces.impl.event.BridgeSetup.generateViewID(BridgeSetup.java:293) at org.icefaces.impl.event.BridgeSetup.assignViewID(BridgeSetup.java:283) at org.icefaces.impl.event.BridgeSetup.access$300(BridgeSetup.java:45) at org.icefaces.impl.event.BridgeSetup$AssignViewID.beforePhase(BridgeSetup.java:317) at com.sun.faces.lifecycle.Phase.handleBeforePhase(Phase.java:228) at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:99) Deryk's initial analysis: There are lots of unsupported methods in ProxyHttpServletRequest as we typically only implemented the minimum we needed for operating. However, the new call at org.icepush.PushContext.createPushId(PushContext.java:166) is doing this: cookie.setMaxAge((int)(Browser.getTimeout(request.getServletContext()) / 1000)); This kind of servlet specific call is not portlet-friendly. Normally I’d recommend using the FacesContext API to do this kind of thing but maybe it’s not available in ICEpush? If not then it might be that we need to implement the method in the ProxyHttpServletRequest facade. However, since I’m not really sure how the Browser class is using the ServletContext , the problem could go deeper than that.
        Hide
        Jack Van Ooststroom added a comment -

        Sending icepush-ee/core/src/main/java/org/icepush/PushContext.java
        Transmitting file data .
        Committed revision 48721.

        Show
        Jack Van Ooststroom added a comment - Sending icepush-ee/core/src/main/java/org/icepush/PushContext.java Transmitting file data . Committed revision 48721.
        Hide
        Jack Van Ooststroom added a comment -

        The ServletContext is now passed as a parameter to the PushContext constructor in order for it to be available without the need for invoking HttpServletRequest.getServletContext(). Marking this one as FIXED again.

        Show
        Jack Van Ooststroom added a comment - The ServletContext is now passed as a parameter to the PushContext constructor in order for it to be available without the need for invoking HttpServletRequest.getServletContext() . Marking this one as FIXED again.
        Hide
        Jack Van Ooststroom added a comment -

        Reopening this one again due to an NPE occurring within the Liferay Portlet environment:

        Caused by: java.lang.NullPointerException
                at com.liferay.faces.bridge.context.internal.BridgeContextImpl.getInitParameter(BridgeContextImpl.java:980)
                at com.liferay.faces.bridge.context.internal.ExternalContextImpl.getInitParameter(ExternalContextImpl.java:263)
                at javax.faces.context.ExternalContextWrapper.getInitParameter(ExternalContextWrapper.java:207)
                at org.icefaces.impl.push.servlet.ProxyServletContext.getInitParameter(ProxyServletContext.java:169)
                at org.icesoft.util.servlet.ServletContextConfiguration.getAttribute(ServletContextConfiguration.java:72)
                at org.icesoft.util.AbstractConfiguration.getAttributeAsLong(AbstractConfiguration.java:184)
                at org.icesoft.util.AbstractConfiguration.getAttributeAsLong(AbstractConfiguration.java:205)
                at org.icepush.Browser.getTimeout(Browser.java:207)
                at org.icepush.Browser.getTimeout(Browser.java:221)
                at org.icepush.PushContext.createPushId(PushContext.java:172)
                at org.icefaces.impl.event.BridgeSetup.generateViewID(BridgeSetup.java:293)
                at org.icefaces.impl.event.BridgeSetup.assignViewID(BridgeSetup.java:283)
                at org.icefaces.impl.event.BridgeSetup.access$300(BridgeSetup.java:45)
                at org.icefaces.impl.event.BridgeSetup$AssignViewID.beforePhase(BridgeSetup.java:317)
                at com.sun.faces.lifecycle.Phase.handleBeforePhase(Phase.java:228)
                at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:99)
                ... 117 more
        
        Show
        Jack Van Ooststroom added a comment - Reopening this one again due to an NPE occurring within the Liferay Portlet environment: Caused by: java.lang.NullPointerException at com.liferay.faces.bridge.context.internal.BridgeContextImpl.getInitParameter(BridgeContextImpl.java:980) at com.liferay.faces.bridge.context.internal.ExternalContextImpl.getInitParameter(ExternalContextImpl.java:263) at javax.faces.context.ExternalContextWrapper.getInitParameter(ExternalContextWrapper.java:207) at org.icefaces.impl.push.servlet.ProxyServletContext.getInitParameter(ProxyServletContext.java:169) at org.icesoft.util.servlet.ServletContextConfiguration.getAttribute(ServletContextConfiguration.java:72) at org.icesoft.util.AbstractConfiguration.getAttributeAsLong(AbstractConfiguration.java:184) at org.icesoft.util.AbstractConfiguration.getAttributeAsLong(AbstractConfiguration.java:205) at org.icepush.Browser.getTimeout(Browser.java:207) at org.icepush.Browser.getTimeout(Browser.java:221) at org.icepush.PushContext.createPushId(PushContext.java:172) at org.icefaces.impl.event.BridgeSetup.generateViewID(BridgeSetup.java:293) at org.icefaces.impl.event.BridgeSetup.assignViewID(BridgeSetup.java:283) at org.icefaces.impl.event.BridgeSetup.access$300(BridgeSetup.java:45) at org.icefaces.impl.event.BridgeSetup$AssignViewID.beforePhase(BridgeSetup.java:317) at com.sun.faces.lifecycle.Phase.handleBeforePhase(Phase.java:228) at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:99) ... 117 more
        Hide
        Jack Van Ooststroom added a comment -

        Sending icepush-ee/util/src/main/java/org/icesoft/util/servlet/ServletContextConfiguration.java
        Transmitting file data .
        Committed revision 48724.

        Show
        Jack Van Ooststroom added a comment - Sending icepush-ee/util/src/main/java/org/icesoft/util/servlet/ServletContextConfiguration.java Transmitting file data . Committed revision 48724.
        Hide
        Jack Van Ooststroom added a comment - - edited

        The ServletContextConfiguration.getAttribute(String) catches a NullPointerException and wraps it into a ConfirmationException assuming the parameter could not be found. Marking this one as FIXED once again.

        Show
        Jack Van Ooststroom added a comment - - edited The ServletContextConfiguration.getAttribute(String) catches a NullPointerException and wraps it into a ConfirmationException assuming the parameter could not be found. Marking this one as FIXED once again.
        Hide
        Carmen Cristurean added a comment - - edited

        Re-opening because the NPE can be reproduced again with EE 4.1.1.BETA Jenkins Build #2, using showcase-portlet on Liferay 6.2 g5:
        The NPE server errors have been seen randomly when adding portlet demos to a page, if the server was started with a demo already included on the page.
        If restarting the server without any demos on the page, the errors can no longer be reproduced.

        Show
        Carmen Cristurean added a comment - - edited Re-opening because the NPE can be reproduced again with EE 4.1.1.BETA Jenkins Build #2, using showcase-portlet on Liferay 6.2 g5: The NPE server errors have been seen randomly when adding portlet demos to a page, if the server was started with a demo already included on the page. If restarting the server without any demos on the page, the errors can no longer be reproduced.
        Hide
        Jack Van Ooststroom added a comment -

        I back-ported the "second-part" fix for this one to the EE 4.1.1.BETA tag.

        Show
        Jack Van Ooststroom added a comment - I back-ported the "second-part" fix for this one to the EE 4.1.1.BETA tag.
        Hide
        Carmen Cristurean added a comment - - edited

        Verified EE-4.1.1.BETA tag r.48822, and Jenkins Build #3 /showcase-portlet on Liferay 6.2 g5 on IE11, FF41, Chrome45.

        Show
        Carmen Cristurean added a comment - - edited Verified EE-4.1.1.BETA tag r.48822, and Jenkins Build #3 /showcase-portlet on Liferay 6.2 g5 on IE11, FF41, Chrome45.
        Hide
        Mircea Toma added a comment -

        Having an expiry date on the ice.push.browser cookie causes the ICEfaces applications to stop working. See ICE-11020. The problem starts occurring whenever the cookie expiry interval is lower than the one set for session expiry.
        Also the cookie expiration is enforced regardless of user or server activity, once the defined interval has elapsed the cookie is removed by the browser.

        Show
        Mircea Toma added a comment - Having an expiry date on the ice.push.browser cookie causes the ICEfaces applications to stop working. See ICE-11020 . The problem starts occurring whenever the cookie expiry interval is lower than the one set for session expiry. Also the cookie expiration is enforced regardless of user or server activity, once the defined interval has elapsed the cookie is removed by the browser.
        Hide
        Jack Van Ooststroom added a comment -

        Sending icepush/core/src/main/java/org/icepush/Browser.java
        Transmitting file data .
        Committed revision 48872.

        Show
        Jack Van Ooststroom added a comment - Sending icepush/core/src/main/java/org/icepush/Browser.java Transmitting file data . Committed revision 48872.
        Hide
        Jack Van Ooststroom added a comment -

        As for robust Cloud Push support the Browser-ID cookie's default expiry is now set to 3 years. Ensuring it survives across sessions and doesn't interfere with the current session. Marking this one as FIXED again.

        Show
        Jack Van Ooststroom added a comment - As for robust Cloud Push support the Browser-ID cookie's default expiry is now set to 3 years. Ensuring it survives across sessions and doesn't interfere with the current session. Marking this one as FIXED again.

          People

          • Assignee:
            Jack Van Ooststroom
            Reporter:
            Jack Van Ooststroom
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: