ICEpush
  1. ICEpush
  2. PUSH-276

Set missing browserID cookie while ice.push.createPushId is used

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0.BETA
    • Fix Version/s: 4.0.BETA, 4.0
    • Component/s: None
    • Labels:
      None
    • Environment:
      supported browsers
    • Assignee Priority:
      P1

      Description

      Make sure the browserID is set first when ice.push.createPushId function is used and then create the pushID.
      1. localmongo.html
        3 kB
        Ted Goddard
      2. mongochat.html
        3 kB
        Ted Goddard

        Activity

        Hide
        Mircea Toma added a comment - - edited

        Modified ICEpush to set ice.push.browser cookie whenever ice.push.createPushId is invoked or a blocking request is issued.

        Show
        Mircea Toma added a comment - - edited Modified ICEpush to set ice.push.browser cookie whenever ice.push.createPushId is invoked or a blocking request is issued.
        Hide
        Ted Goddard added a comment -

        createPushId goes into infinite loop:

        I tried disabling the apparently recursive code, but this causes the
        ID not to be set at all.

        +++ core/src/main/javascript/application.js (working copy)
        @@ -189,7 +189,7 @@
        condition(OK, function(response) {
        if (isXMLResponse(response))

        { deserializeAndExecute(commandDispatcher, contentAsDOM(response).documentElement); - id = namespace.push.createPushId(); +// id = namespace.push.createPushId(); }

        else

        { id = contentAsText(response); }
        Show
        Ted Goddard added a comment - createPushId goes into infinite loop: I tried disabling the apparently recursive code, but this causes the ID not to be set at all. +++ core/src/main/javascript/application.js (working copy) @@ -189,7 +189,7 @@ condition(OK, function(response) { if (isXMLResponse(response)) { deserializeAndExecute(commandDispatcher, contentAsDOM(response).documentElement); - id = namespace.push.createPushId(); +// id = namespace.push.createPushId(); } else { id = contentAsText(response); }
        Hide
        Ted Goddard added a comment -

        To reproduce, build pushservice in samples. This application includes a header filter to support cross-domain invocation. Run pushservice on one host and load the mongochat page from another. Change mongochat (attached) JavaScript links to point to the pushservice server.

        Show
        Ted Goddard added a comment - To reproduce, build pushservice in samples. This application includes a header filter to support cross-domain invocation. Run pushservice on one host and load the mongochat page from another. Change mongochat (attached) JavaScript links to point to the pushservice server.
        Hide
        Mircea Toma added a comment -

        Added mechanism that avoids having the bridge going into infinite loop when browser ID cookie cannot be set.

        Show
        Mircea Toma added a comment - Added mechanism that avoids having the bridge going into infinite loop when browser ID cookie cannot be set.
        Hide
        Mircea Toma added a comment - - edited

        It looks like that creating a cross-domain cookie through javascript is not really feasible. So the createPushId or listen responses will need to set the cookie through the normal Set-Cookie header mechanism. In order to have that working XHR requests need to have the withCredentials property set to true.

        Show
        Mircea Toma added a comment - - edited It looks like that creating a cross-domain cookie through javascript is not really feasible. So the createPushId or listen responses will need to set the cookie through the normal Set-Cookie header mechanism. In order to have that working XHR requests need to have the withCredentials property set to true.
        Hide
        Ted Goddard added a comment -

        I don't believe we can use a cookie for the browserID in this case – it will need to be stored by javascript and included in each request, preferably in the body since new headers require custom header configuration. In order to share the browserID across all browser windows, we may need to use local storage.

        Cross-site Cookie permissions are not well supported across different browsers.

        Show
        Ted Goddard added a comment - I don't believe we can use a cookie for the browserID in this case – it will need to be stored by javascript and included in each request, preferably in the body since new headers require custom header configuration. In order to share the browserID across all browser windows, we may need to use local storage. Cross-site Cookie permissions are not well supported across different browsers.
        Hide
        Mircea Toma added a comment - - edited

        Modified ICEpush bridge to store its corresponding ID into the local storage by default (to ensure easy sharing between windows). Alo modified the bridge issued requests to send the browser ID as parameter.

        Tested pushservice application, with these latest changes, successfully.

        Show
        Mircea Toma added a comment - - edited Modified ICEpush bridge to store its corresponding ID into the local storage by default (to ensure easy sharing between windows). Alo modified the bridge issued requests to send the browser ID as parameter. Tested pushservice application, with these latest changes, successfully.
        Hide
        Ted Goddard added a comment -

        An infinite loop in create-push-id. This can be seen with pushservice deployed from the trunk to labs

        http://labs.icesoft.com:8080/pushservice

        It is likely necessary to test with pushservice running on one machine and the mongochat page hosted on another machine.

        Show
        Ted Goddard added a comment - An infinite loop in create-push-id. This can be seen with pushservice deployed from the trunk to labs http://labs.icesoft.com:8080/pushservice It is likely necessary to test with pushservice running on one machine and the mongochat page hosted on another machine.
        Hide
        Ted Goddard added a comment -

        Can we eliminate the header and just use the parameter for BrowserID?

        Will this support IE 7 since it does not have local storage?

        Show
        Ted Goddard added a comment - Can we eliminate the header and just use the parameter for BrowserID? Will this support IE 7 since it does not have local storage?
        Hide
        Mircea Toma added a comment -

        The header is still there for backward compatibility. I remember Jack or you needed it for some feature.

        Anyway, all requests now have ice.push.browser parameter which contains the browser ID. As for the infinite loop, I am pretty sure that cannot occur anymore because I introduced a safety mechanism which stops the createPushId requests if they loop more than 3 times.

        To me it looks like you do not have the latest code.

        Show
        Mircea Toma added a comment - The header is still there for backward compatibility. I remember Jack or you needed it for some feature. Anyway, all requests now have ice.push.browser parameter which contains the browser ID. As for the infinite loop, I am pretty sure that cannot occur anymore because I introduced a safety mechanism which stops the createPushId requests if they loop more than 3 times. To me it looks like you do not have the latest code.
        Hide
        Mircea Toma added a comment -

        Browsers that do not have localStorage will still use a cookie to store the browser ID. These browsers cannot make cross-domain requests anyway since they do not implement XHR2.

        Show
        Mircea Toma added a comment - Browsers that do not have localStorage will still use a cookie to store the browser ID. These browsers cannot make cross-domain requests anyway since they do not implement XHR2.
        Hide
        Ted Goddard added a comment -

        Is this the checkin for this feature?

        r38038 | mircea.toma | 2013-09-16 14:26:22 -0600 (Mon, 16 Sep 2013) | 1 line

        PUSH-276 Modified ICEpush bridge to store its corresponding ID into the local storage by default (to ensure easy sharing between windows). Alo modified the bridge issued requests to send the browser ID as parameter.

        Show
        Ted Goddard added a comment - Is this the checkin for this feature? r38038 | mircea.toma | 2013-09-16 14:26:22 -0600 (Mon, 16 Sep 2013) | 1 line PUSH-276 Modified ICEpush bridge to store its corresponding ID into the local storage by default (to ensure easy sharing between windows). Alo modified the bridge issued requests to send the browser ID as parameter.
        Hide
        Ted Goddard added a comment -

        Could the infinite loop be caused by the browser having all cookies cleared?

        Show
        Ted Goddard added a comment - Could the infinite loop be caused by the browser having all cookies cleared?
        Hide
        Jack Van Ooststroom added a comment -

        I looked a bit at the server-side code, but I'm a bit confused now.

        With my earlier changes I tried to consolidate the various locations where we were trying to get the Browser-ID into one single location, namely the Browser.class. It has a public static method getBrowserID(request), which in its turn could get it from the Header or the Cookie. This is now changed to getting it from the Header or the Parameter, not the Cookie anymore.

        However, the new CheckBrowserIDServlet doesn't use this API and only checks if the Parameter is there.

        In addition, the PushContext.createPushID(...) still uses the Browser.getBrowserID(request) method, which now checks for the Header and the Parameter. However, if the Browser-ID isn't found it sets it still as a Cookie.

        I came up with the following questions:

        1. Which possibilities does ICEpush needs to provide in regards to ICEpush in general (not just as a service)? Parameter? Header? Cookie? Or any combination of the three?
        2. How does the new Browser-ID need to be communicated back to the client? Through <browser id=" {browser-id}

          " />? Through Set-Cookie? Or both?

        3. Shouldn't CheckBrowserIDServlet use Browser.getBrowserID(request) as well? We could even make the sub-methods getBrowserIDFromCookie(...), getBrowserIDFromHeader(...) and getBrowserIDFromParameter(...) public as well and still have getBrowserID(...) public too to provide choice and/or different strategy.
        Show
        Jack Van Ooststroom added a comment - I looked a bit at the server-side code, but I'm a bit confused now. With my earlier changes I tried to consolidate the various locations where we were trying to get the Browser-ID into one single location, namely the Browser.class. It has a public static method getBrowserID(request), which in its turn could get it from the Header or the Cookie. This is now changed to getting it from the Header or the Parameter, not the Cookie anymore. However, the new CheckBrowserIDServlet doesn't use this API and only checks if the Parameter is there. In addition, the PushContext.createPushID(...) still uses the Browser.getBrowserID(request) method, which now checks for the Header and the Parameter. However, if the Browser-ID isn't found it sets it still as a Cookie. I came up with the following questions: Which possibilities does ICEpush needs to provide in regards to ICEpush in general (not just as a service)? Parameter? Header? Cookie? Or any combination of the three? How does the new Browser-ID need to be communicated back to the client? Through <browser id=" {browser-id} " />? Through Set-Cookie? Or both? Shouldn't CheckBrowserIDServlet use Browser.getBrowserID(request) as well? We could even make the sub-methods getBrowserIDFromCookie(...), getBrowserIDFromHeader(...) and getBrowserIDFromParameter(...) public as well and still have getBrowserID(...) public too to provide choice and/or different strategy.
        Hide
        Mircea Toma added a comment -

        To Ted:

        Yes, since revision 38038 the fix is available.
        No, clearing of the cookies hasn't caused me any problem while testing the fix.

        Show
        Mircea Toma added a comment - To Ted: Yes, since revision 38038 the fix is available. No, clearing of the cookies hasn't caused me any problem while testing the fix.
        Hide
        Mircea Toma added a comment -

        To Jack:

        1. Parameter should be enough.
        2. Yes, through <browser id="

        {browser-id}

        " /> message.
        3. I don't know, I'm not familiar with what Browser object is responsible for. Again, Browser.getBrowserIDFromParameter should suffice.

        Show
        Mircea Toma added a comment - To Jack: 1. Parameter should be enough. 2. Yes, through <browser id=" {browser-id} " /> message. 3. I don't know, I'm not familiar with what Browser object is responsible for. Again, Browser.getBrowserIDFromParameter should suffice.
        Hide
        Ted Goddard added a comment -

        localhost configuration for pushservice

        Show
        Ted Goddard added a comment - localhost configuration for pushservice
        Hide
        Ted Goddard added a comment -

        Steps to reproduce:

        copy pushservice.war built from ICEpush trunk to apache-tomcat/webapps
        copy attached localmongo.html to apache-tomcat/webapps/ROOT

        load http://localhost:8080/localmongo.html in chrome and inspect element to display network activity.
        Press shift-reload
        observe infinite loop of create-push-id

        (This may have something to do with behavior under shift reload.)

        Show
        Ted Goddard added a comment - Steps to reproduce: copy pushservice.war built from ICEpush trunk to apache-tomcat/webapps copy attached localmongo.html to apache-tomcat/webapps/ROOT load http://localhost:8080/localmongo.html in chrome and inspect element to display network activity. Press shift-reload observe infinite loop of create-push-id (This may have something to do with behavior under shift reload.)
        Hide
        Mircea Toma added a comment -

        I cannot reproduce the infinite loop in Chrome. We'll have to cross-check our deployments.

        Show
        Mircea Toma added a comment - I cannot reproduce the infinite loop in Chrome. We'll have to cross-check our deployments.
        Hide
        Ted Goddard added a comment -

        Can you attach your .war file? I will deploy to labs for testing.

        Show
        Ted Goddard added a comment - Can you attach your .war file? I will deploy to labs for testing.
        Hide
        Mircea Toma added a comment -

        Attached pushservice.war that works as expected on my local deployment.

        Show
        Mircea Toma added a comment - Attached pushservice.war that works as expected on my local deployment.
        Hide
        Ted Goddard added a comment -

        It appears that this problem is transient – after restarting the server, I had good results with the browser I tested with, but then observed a storm of traffic for a connected mobile device. Please leave your server running locally over a period of time and test it periodically with a number of browsers while keeping an eye on the network activity. I will monitor the labs deployment to look for the problem again.

        Show
        Ted Goddard added a comment - It appears that this problem is transient – after restarting the server, I had good results with the browser I tested with, but then observed a storm of traffic for a connected mobile device. Please leave your server running locally over a period of time and test it periodically with a number of browsers while keeping an eye on the network activity. I will monitor the labs deployment to look for the problem again.
        Hide
        Ted Goddard added a comment -

        Restarting tomcat and clearing the work directory seems to temporarily fix the problem. So, this appears to be a transient state that either the push client or the server enters (but it is a very severe failure since it could exhaust either a mobile user's bandwidth cap or result in a high network use bill from amazon for us.)

        Show
        Ted Goddard added a comment - Restarting tomcat and clearing the work directory seems to temporarily fix the problem. So, this appears to be a transient state that either the push client or the server enters (but it is a very severe failure since it could exhaust either a mobile user's bandwidth cap or result in a high network use bill from amazon for us.)
        Hide
        Mircea Toma added a comment -

        The infinite loop issue cannot be reproduced any more, most probably it was caused by browser caching. We can open this issue again if the problem re-occurs.

        Show
        Mircea Toma added a comment - The infinite loop issue cannot be reproduced any more, most probably it was caused by browser caching. We can open this issue again if the problem re-occurs.

          People

          • Assignee:
            Mircea Toma
            Reporter:
            Mircea Toma
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: