fix: handle timeouts during tls negotiation for strict encryption (#…

…1564) Co-authored-by: Arthur Schreiber <[email protected]>
tediousjs · Sep 17, 2023 · 8c7e440 · 8c7e440
1 parent 4f3e210
commit 8c7e440
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 9 deletions.
diff --git a/src/connection.ts b/src/connection.ts
@@ -1931,7 +1931,8 @@ class Connection extends EventEmitter {
         });
       }, (err) => {
         this.clearConnectTimer();
-        if (err.name === 'AbortError') {
+
+        if (signal.aborted) {
           // Ignore the AbortError for now, this is still handled by the connectTimer firing
           return;
         }
@@ -2008,7 +2009,9 @@ class Connection extends EventEmitter {
     this.transitionTo(this.STATE.SENT_PRELOGIN);
   }
 
-  wrapWithTls(socket: net.Socket): Promise<tls.TLSSocket> {
+  wrapWithTls(socket: net.Socket, signal: AbortSignal): Promise<tls.TLSSocket> {
+    signal.throwIfAborted();
+
     return new Promise((resolve, reject) => {
       const secureContext = tls.createSecureContext(this.secureContextOptions);
       // If connect to an ip address directly,
@@ -2023,12 +2026,41 @@ class Connection extends EventEmitter {
         servername: this.config.options.serverName ? this.config.options.serverName : serverName,
       };
 
-      const encryptsocket = tls.connect(encryptOptions, () => {
-        encryptsocket.removeListener('error', reject);
+      const encryptsocket = tls.connect(encryptOptions);
+
+      const onAbort = () => {
+        encryptsocket.removeListener('error', onError);
+        encryptsocket.removeListener('connect', onConnect);
+
+        encryptsocket.destroy();
+
+        reject(signal.reason);
+      };
+
+      const onError = (err: Error) => {
+        signal.removeEventListener('abort', onAbort);
+
+        encryptsocket.removeListener('error', onError);
+        encryptsocket.removeListener('connect', onConnect);
+
+        encryptsocket.destroy();
+
+        reject(err);
+      };
+
+      const onConnect = () => {
+        signal.removeEventListener('abort', onAbort);
+
+        encryptsocket.removeListener('error', onError);
+        encryptsocket.removeListener('connect', onConnect);
+
         resolve(encryptsocket);
-      });
+      };
+
+      signal.addEventListener('abort', onAbort, { once: true });
 
-      encryptsocket.once('error', reject);
+      encryptsocket.on('error', onError);
+      encryptsocket.on('secureConnect', onConnect);
     });
   }
 
@@ -2047,7 +2079,7 @@ class Connection extends EventEmitter {
       if (this.config.options.encrypt === 'strict') {
         try {
           // Wrap the socket with TLS for TDS 8.0
-          socket = await this.wrapWithTls(socket);
+          socket = await this.wrapWithTls(socket, signal);
         } catch (err) {
           socket.end();
 
@@ -2059,7 +2091,7 @@ class Connection extends EventEmitter {
     })().catch((err) => {
       this.clearConnectTimer();
 
-      if (err.name === 'AbortError') {
+      if (signal.aborted) {
         return;
       }
 

diff --git a/test/unit/connection-test.ts b/test/unit/connection-test.ts
@@ -1,5 +1,5 @@
 import * as net from 'net';
-import { Connection } from '../../src/tedious';
+import { Connection, ConnectionError } from '../../src/tedious';
 import { assert } from 'chai';
 
 describe('Using `strict` encryption', function() {
@@ -42,4 +42,36 @@ describe('Using `strict` encryption', function() {
       done();
     });
   });
+
+  it('handles connection timeout when performing tls handshake', function(done) {
+    server.on('connection', (connection) => {
+      setTimeout(() => {
+        connection.destroy();
+      }, 4000);
+    });
+
+    const addressInfo = server.address() as net.AddressInfo;
+
+    const connection = new Connection({
+      server: addressInfo?.address,
+      options: {
+        port: addressInfo?.port,
+        encrypt: 'strict',
+        connectTimeout: 3000
+      }
+    });
+
+    connection.connect((err) => {
+      assert.instanceOf(err, ConnectionError);
+
+      const message = `Failed to connect to ${addressInfo?.address}:${addressInfo?.port} in 3000ms`;
+      assert.equal(err!.message, message);
+
+      connection.close();
+    });
+
+    connection.on('end', () => {
+      done();
+    });
+  });
 });