From dcfa52b6f3342ce45855db37ea381e1d34b255ff Mon Sep 17 00:00:00 2001
From: Rohan Sikdar <rsikdar@berkeley.edu>
Date: Tue, 29 Jan 2019 15:42:47 -0800
Subject: [PATCH] fix(WebSocketSubject): fix subject failing to close socket
 (#4446)

Fix WebSocketSubject failing to close the underlying socket connection when it is unsubscribed
before the socket is in the open state. Currently, when unsubscribe is called on the subject or
when a subscription is unsubscribed there are checks on whether the socket is open (readyState = 1)
before it closes the connection. If unsubscribe is called before the connection is open, it will not
close the socket but will reset the state of the subject (_socket set to null), and it becomes
impossible to close the socket.
---
 spec/observables/dom/webSocket-spec.ts        | 28 +++++++++++++++++++
 .../observable/dom/WebSocketSubject.ts        | 13 +++++----
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/spec/observables/dom/webSocket-spec.ts b/spec/observables/dom/webSocket-spec.ts
index 4254248d84..854e9369da 100644
--- a/spec/observables/dom/webSocket-spec.ts
+++ b/spec/observables/dom/webSocket-spec.ts
@@ -143,6 +143,34 @@ describe('webSocket', () => {
       (<any>socket.close).restore();
     });
 
+    it('should close the socket when unsubscribed before socket open', () => {
+      const subject = webSocket<string>('ws://mysocket');
+      subject.subscribe();
+      subject.unsubscribe();
+      const socket = MockWebSocket.lastSocket;
+      sinon.spy(socket, 'close');
+      socket.open();
+
+      expect(socket.close).have.been.called;
+      expect(socket.readyState).to.equal(3); // closed
+
+      (<any>socket.close).restore();
+    });
+
+    it('should close the socket when subscription is cancelled before socket open', () => {
+      const subject = webSocket<string>('ws://mysocket');
+      const subscription = subject.subscribe();
+      subscription.unsubscribe();
+      const socket = MockWebSocket.lastSocket;
+      sinon.spy(socket, 'close');
+      socket.open();
+
+      expect(socket.close).have.been.called;
+      expect(socket.readyState).to.equal(3); // closed
+
+      (<any>socket.close).restore();
+    });
+
     it('should close the socket with a code and a reason when errored', () => {
       const subject = webSocket<string>('ws://mysocket');
       subject.subscribe();
diff --git a/src/internal/observable/dom/WebSocketSubject.ts b/src/internal/observable/dom/WebSocketSubject.ts
index 9dca5a256f..c9be095e68 100644
--- a/src/internal/observable/dom/WebSocketSubject.ts
+++ b/src/internal/observable/dom/WebSocketSubject.ts
@@ -184,6 +184,12 @@ export class WebSocketSubject<T> extends AnonymousSubject<T> {
     });
 
     socket.onopen = (e: Event) => {
+      const { _socket } = this;
+      if (!_socket) {
+        socket.close();
+        this._resetState();
+        return;
+      }
       const { openObserver } = this._config;
       if (openObserver) {
         openObserver.next(e);
@@ -280,14 +286,11 @@ export class WebSocketSubject<T> extends AnonymousSubject<T> {
   }
 
   unsubscribe() {
-    const { source, _socket } = this;
+    const { _socket } = this;
     if (_socket && _socket.readyState === 1) {
       _socket.close();
-      this._resetState();
     }
+    this._resetState();
     super.unsubscribe();
-    if (!source) {
-      this.destination = new ReplaySubject();
-    }
   }
 }