Close connection on SSL connection errors (brianc#2082)

* Close connection on SSL connection errors

Fixes brianc#2079

* Fix test

* Remove console.log

* Fix tests, implement same behavior for native client

* Fix tests

Author: brianc

packages/pg-pool/test/error-handling.js

@@ -211,14 +211,14 @@ describe('pool error handling', function () {
       const pool = new Pool({ max: 1, port: closeServer.address().port, host: 'localhost' })
       pool.connect((err) => {
         expect(err).to.be.an(Error)
-        if (err.errno) {
-          expect(err.errno).to.be('ECONNRESET')
+        if (err.code) {
+          expect(err.code).to.be('ECONNRESET')
         }
       })
       pool.connect((err) => {
         expect(err).to.be.an(Error)
-        if (err.errno) {
-          expect(err.errno).to.be('ECONNRESET')
+        if (err.code) {
+          expect(err.code).to.be('ECONNRESET')
         }
         closeServer.close(() => {
           pool.end(done)

packages/pg/lib/connection.js

@@ -85,11 +85,13 @@ Connection.prototype.connect = function (port, host) {
   this.stream.once('data', function (buffer) {
     var responseCode = buffer.toString('utf8')
     switch (responseCode) {
-      case 'N': // Server does not support SSL connections
-        return self.emit('error', new Error('The server does not support SSL connections'))
       case 'S': // Server supports SSL connections, continue with a secure connection
         break
+      case 'N': // Server does not support SSL connections
+        self.stream.end()
+        return self.emit('error', new Error('The server does not support SSL connections'))
       default: // Any other response byte, including 'E' (ErrorResponse) indicating a server error
+        self.stream.end()
         return self.emit('error', new Error('There was an error establishing an SSL connection'))
     }
     var tls = require('tls')
@@ -112,9 +114,18 @@ Connection.prototype.connect = function (port, host) {
       options.servername = host
     }
     self.stream = tls.connect(options)
-    self.attachListeners(self.stream)
     self.stream.on('error', reportStreamError)
 
+    // send SSLRequest packet
+    const buff = Buffer.alloc(8)
+    buff.writeUInt32BE(8)
+    buff.writeUInt32BE(80877103, 4)
+    if (self.stream.writable) {
+      self.stream.write(buff)
+    }
+
+    self.attachListeners(self.stream)
+
     self.emit('sslconnect')
   })
 }
@@ -345,6 +356,10 @@ Connection.prototype.end = function () {
   // 0x58 = 'X'
   this.writer.add(emptyBuffer)
   this._ending = true
+  if (!this.stream.writable) {
+    this.stream.end()
+    return
+  }
   return this.stream.write(END_BUFFER, () => {
     this.stream.end()
   })

packages/pg/lib/native/client.js

@@ -89,7 +89,10 @@ Client.prototype._connect = function (cb) {
   this.connectionParameters.getLibpqConnectionString(function (err, conString) {
     if (err) return cb(err)
     self.native.connect(conString, function (err) {
-      if (err) return cb(err)
+      if (err) {
+        self.native.end()
+        return cb(err)
+      }
 
       // set internal states to connected
       self._connected = true

packages/pg/test/integration/gh-issues/2056-tests.js

@@ -5,6 +5,7 @@ var assert = require('assert')
 
 const suite = new helper.Suite()
 
+
 suite.test('All queries should return a result array', (done) => {
   const client = new helper.pg.Client()
   client.connect()

packages/pg/test/integration/gh-issues/2079-tests.js

@@ -0,0 +1,56 @@
+
+"use strict"
+var helper = require('./../test-helper')
+var assert = require('assert')
+
+const suite = new helper.Suite()
+
+// makes a backend server that responds with a non 'S' ssl response buffer
+let makeTerminatingBackend = (byte) => {
+  const { createServer }  = require('net')
+
+  const server = createServer((socket) => {
+    // attach a listener so the socket can drain
+    // https://www.postgresql.org/docs/9.3/protocol-message-formats.html
+    socket.on('data', (buff) => {
+      const code = buff.readInt32BE(4)
+      // I don't see anything in the docs about 80877104
+      // but libpq is sending it...
+      if (code === 80877103 || code === 80877104) {
+        const packet = Buffer.from(byte, 'utf-8')
+        socket.write(packet)
+      }
+    })
+    socket.on('close', () => {
+      server.close()
+    })
+  })
+
+  server.listen()
+  const { port } = server.address()
+  return port
+}
+
+suite.test('SSL connection error allows event loop to exit', (done) => {
+  const port = makeTerminatingBackend('N')
+  const client = new helper.pg.Client({ ssl: 'require', port })
+  // since there was a connection error the client's socket should be closed
+  // and the event loop will have no refs and exit cleanly
+  client.connect((err) => {
+    assert(err instanceof Error)
+    done()
+  })
+})
+
+
+suite.test('Non "S" response code allows event loop to exit', (done) => {
+  const port = makeTerminatingBackend('X')
+  const client = new helper.pg.Client({ ssl: 'require', port })
+  // since there was a connection error the client's socket should be closed
+  // and the event loop will have no refs and exit cleanly
+  client.connect((err) => {
+    assert(err instanceof Error)
+    done()
+  })
+})
+