Merge lp:~blake-rouse/maas/fix-1484696 into lp:~maas-committers/maas/trunk

[Blake Rouse]

Regenerate the connection URL on websocket client reconnect.

I am really guessing at this one. This bug is really hard to reproduce no one really knows exactly how it is happening or what is causing it.

My guess here is that the websocket can connect faster than the cookie is updated when the entire page is loaded. So it will use the old csrftoken instead of the new one. If that happens then the retry will keep using that old token. This branch changes that so the connection url is regenerated on every connect so the new csrftoken is used.

[Andres Rodriguez (andreserl)]

The branch overall looks good to me. I think we can give it a try in trunk and see what happens, hoewver, please prepare a backport bracnh to 1.8 so we can test it against 1.8 too.

review: Approve

=== modified file 'src/maasserver/static/js/angular/factories/region.js'
--- src/maasserver/static/js/angular/factories/region.js	2015-06-26 12:47:19 +0000
+++ src/maasserver/static/js/angular/factories/region.js	2015-08-19 15:22:53 +0000
@@ -117,8 +117,8 @@
         };
 
         // Opens the websocket connection.
-        RegionConnection.prototype.connect = function(url) {
-            this.url = url;
+        RegionConnection.prototype.connect = function() {
+            this.url = this._buildUrl();
             this.autoReconnect = true;
             this.websocket = this.buildSocket(this.url);
 
@@ -142,7 +142,7 @@
                 });
                 if(self.autoReconnect) {
                     $timeout(function() {
-                        self.connect(self.url);
+                        self.connect();
                     }, self.retryTimeout);
                 }
             };
@@ -236,7 +236,7 @@
             };
             this.registerHandler("open", opened);
             this.registerHandler("error", errored);
-            this.connect(this._buildUrl());
+            this.connect();
             return defer.promise;
         };
 

=== modified file 'src/maasserver/static/js/angular/factories/tests/test_region.js'
--- src/maasserver/static/js/angular/factories/tests/test_region.js	2015-06-26 12:47:19 +0000
+++ src/maasserver/static/js/angular/factories/tests/test_region.js	2015-08-19 15:22:53 +0000
@@ -199,33 +199,37 @@
 
     describe("connect", function() {
 
-        var url = "http://test-url";
+        var url = "http://test-url", buildUrlSpy;
+        beforeEach(function() {
+            buildUrlSpy = spyOn(RegionConnection, "_buildUrl");
+            buildUrlSpy.and.returnValue(url);
+        });
 
         it("sets url", function() {
-            RegionConnection.connect(url);
+            RegionConnection.connect();
             expect(RegionConnection.url).toBe(url);
         });
 
         it("sets autoReconnect to true", function() {
             RegionConnection.autoReconnect = false;
-            RegionConnection.connect(url);
+            RegionConnection.connect();
             expect(RegionConnection.autoReconnect).toBe(true);
         });
 
         it("calls buildSocket with url", function() {
-            RegionConnection.connect(url);
+            RegionConnection.connect();
             expect(RegionConnection.buildSocket).toHaveBeenCalledWith(url);
         });
 
         it("sets websocket handlers", function() {
-            RegionConnection.connect(url);
+            RegionConnection.connect();
             expect(webSocket.onopen).not.toBeNull();
             expect(webSocket.onerror).not.toBeNull();
             expect(webSocket.onclose).not.toBeNull();
         });
 
         it("sets connect to true when onopen called", function() {
-            RegionConnection.connect(url);
+            RegionConnection.connect();
             webSocket.onopen({});
             expect(RegionConnection.connected).toBe(true);
         });
@@ -236,13 +240,13 @@
                 expect(evt).toBe(evt_obj);
                 done();
             });
-            RegionConnection.connect(url);
+            RegionConnection.connect();
             webSocket.onerror(evt_obj);
         });
 
         it("sets connect to false when onclose called", function() {
             RegionConnection.autoReconnect = false;
-            RegionConnection.connect(url);
+            RegionConnection.connect();
             webSocket.onclose({});
             expect(RegionConnection.connected).toBe(false);
         });
@@ -250,7 +254,7 @@
         it("calls close handler when onclose called", function(done) {
             var evt_obj = {};
             RegionConnection.autoReconnect = false;
-            RegionConnection.connect(url);
+            RegionConnection.connect();
             RegionConnection.registerHandler("close", function(evt) {
                 expect(evt).toBe(evt_obj);
                 done();
@@ -259,15 +263,16 @@
         });
 
         it("onclose calls connect when autoReconnect is true", function() {
-            RegionConnection.connect(url);
-            spyOn(RegionConnection, "connect");
+            RegionConnection.connect();
+            var new_url = "http://new-url";
+            buildUrlSpy.and.returnValue(new_url);
             webSocket.onclose({});
             $timeout.flush();
-            expect(RegionConnection.connect).toHaveBeenCalledWith(url);
+            expect(RegionConnection.url).toBe(new_url);
         });
 
         it("onclose sets error", function() {
-            RegionConnection.connect(url);
+            RegionConnection.connect();
             webSocket.onclose();
             $timeout.flush();
             expect(RegionConnection.error).toBe(
@@ -277,7 +282,7 @@
         it("calls onMessage when onmessage called", function() {
             var sampleData = { sample: "data" };
             spyOn(RegionConnection, "onMessage");
-            RegionConnection.connect(url);
+            RegionConnection.connect();
             webSocket.onmessage({ data: angular.toJson(sampleData) });
             expect(RegionConnection.onMessage).toHaveBeenCalledWith(
                 sampleData);