fix: improvements to allow to use Bun and tls (#2119)

* uplift startTls code to be compatible with current bun * more tls refactoring * lint * ci: enable tls in bun matrix * fix ssl tests * lint * bun: only run 2 basic tests for now * lint * don't enable ssl in test running against fake server * fix typo * debug failures * try bun canary * ci: add osx runner * ci: install docker for osx * ci: install docker for osx * ci: install docker for osx * ci: osx - don't mount config in docker * handle ECONNREFUSED in waitDatabaseReady helper * comment out instead of early return * explicitly install lima * use connection.end() instead of destroy * debug Ssl_cipher assertion * more flexible Ssl_cipher assertion * initialize packet header befor writing * cleanup * add compression to bun matrix * only use bun v0.6.13 for non-ssl tests
sidorares · Jul 10, 2023 · fd44a2a · fd44a2a
1 parent 4ac6a8f
commit fd44a2a
Show file tree

Hide file tree

Showing 12 changed files with 171 additions and 107 deletions.
diff --git a/.github/workflows/ci-bun.yml b/.github/workflows/ci-bun.yml
@@ -20,10 +20,19 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        bun-version: [0.5.1]
+        bun-version: [canary]
         mysql-version: ["mysql:5.7", "mysql:8.0.18", "mysql:8.0.22"]
-        use-compression: [0]
-        use-tls: [0]
+        use-compression: [0, 1]
+        use-tls: [0,1]
+        include:
+          - bun-version: "0.6.13"
+            use-compression: 1
+            use-tls: 0
+            mysql-version: "mysql:8.0.18"
+          - bun-version: "0.6.13"
+            use-compression: 1
+            use-tls: 0
+            mysql-version: "mysql:8.0.22"
 
     name: Bun ${{ matrix.bun-version }} - DB ${{ matrix.mysql-version }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}}
 
@@ -33,7 +42,7 @@ jobs:
         run: docker run -d -e MYSQL_ALLOW_EMPTY_PASSWORD=1 -e MYSQL_DATABASE=${{ env.MYSQL_DATABASE }} -v $PWD/mysqldata:/var/lib/mysql/ -v $PWD/examples/custom-conf:/etc/mysql/conf.d -v $PWD/examples/ssl/certs:/certs -p ${{ env.MYSQL_PORT }}:3306 ${{ matrix.mysql-version }}
 
       - name: Set up Bun ${{ matrix.bun-version }}
-        uses: oven-sh/setup-bun@v0.1.8
+        uses: oven-sh/setup-bun@v1
         with:
           bun-version: ${{ matrix.bun-version }}
 
@@ -57,6 +66,14 @@ jobs:
       - name: Wait mysql server is ready
         run: node tools/wait-up.js
 
-      - name: Run tests
-        # todo: run full test suite once test createServer is implemented using Bun.listen
-        run: FILTER=test-select MYSQL_PORT=3306 bun run test
+      # todo: run full test suite once test createServer is implemented using Bun.listen
+      - name: run tests
+        env:
+          MYSQL_USER: ${{ env.MYSQL_USER }}
+          MYSQL_DATABASE: ${{ env.MYSQL_DATABASE }}
+          MYSQL_PORT: ${{ env.MYSQL_PORT }}
+          MYSQL_USE_COMPRESSION: ${{ matrix.use-compression }}
+          MYSQL_USE_TLS: ${{ matrix.use-tls }}
+        run: |
+          bun test/integration/connection/test-select-1.js
+          bun test/integration/connection/test-select-ssl.js
diff --git a/.github/workflows/ci-osx.yml b/.github/workflows/ci-osx.yml
@@ -0,0 +1,81 @@
+name: CI - OSX
+
+on:
+  pull_request:
+  push:
+    branches: [ main ]
+
+  workflow_dispatch:
+
+env:
+  MYSQL_PORT: 3306
+  MYSQL_USER: root
+  MYSQL_DATABASE: test
+
+jobs:
+  tests-osx:
+    runs-on: macos-13
+    strategy:
+      fail-fast: false
+      matrix:
+        node-version: [18.x, 20.x]
+        mysql-version: ["mysql:8.0.22", "mysql:8.0.33"]
+        use-compression: [0, 1]
+        use-tls: [0]
+        mysql_connection_url_key: [""]
+        # TODO - add mariadb to the matrix. currently few tests are broken due to mariadb incompatibilities
+        include:
+          # 20.x 
+          - node-version: "20.x"
+            mysql-version: "mysql:8.0.33"
+            use-compression: 1
+            use-tls: 0
+            use-builtin-test-runner: 1
+          - node-version: "20.x"
+            mysql-version: "mysql:8.0.33"
+            use-compression: 0
+            use-tls: 1
+            use-builtin-test-runner: 1
+    env:
+      MYSQL_CONNECTION_URL: ${{ secrets[matrix.mysql_connection_url_key] }}
+
+    name: Node.js ${{ matrix.node-version }} - DB ${{ matrix.mysql-version }}${{ matrix.mysql_connection_url_key }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}}
+
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: install lima
+        run: brew install lima
+
+      - name: Setup Docker on macOS
+        uses: douglascamata/setup-docker-macos-action@v1-alpha
+
+      - name: Set up MySQL
+        if: ${{ matrix.mysql-version }}
+        run: docker run -d -e MYSQL_ALLOW_EMPTY_PASSWORD=1 -e MYSQL_DATABASE=${{ env.MYSQL_DATABASE }} -p ${{ env.MYSQL_PORT }}:3306 ${{ matrix.mysql-version }}
+
+      - name: Set up Node.js ${{ matrix.node-version }}
+        uses: actions/setup-node@v3
+        with:
+          node-version: ${{ matrix.node-version }}
+
+      - name: Cache dependencies
+        uses: actions/cache@v3
+        with:
+          path: ~/.npm
+          key: npm-${{ hashFiles('package-lock.json') }}
+          restore-keys: npm-
+
+      - name: Install npm dependencies
+        run: npm ci
+
+      - name: Wait mysql server is ready
+        if: ${{ matrix.mysql-version }}
+        run: node tools/wait-up.js
+
+      - name: Run tests
+        run: FILTER=${{matrix.filter}} MYSQL_USE_TLS=${{ matrix.use-tls }} MYSQL_USE_COMPRESSION=${{ matrix.use-compression }} npm run coverage-test
+
+      - name: Run tests with built-in node test runner
+        if: ${{ matrix.use-builtin-test-runner }}
+        run: FILTER=${{matrix.filter}} MYSQL_USE_TLS=${{ matrix.use-tls }} MYSQL_USE_COMPRESSION=${{ matrix.use-compression }} npm run test:builtin-node-runner
diff --git a/lib/connection.js b/lib/connection.js
@@ -355,62 +355,43 @@ class Connection extends EventEmitter {
     });
     const rejectUnauthorized = this.config.ssl.rejectUnauthorized;
     const verifyIdentity = this.config.ssl.verifyIdentity;
-    const host = this.config.host;
+    const servername = this.config.host;
 
     let secureEstablished = false;
-    const secureSocket = new Tls.TLSSocket(this.stream, {
-      rejectUnauthorized: rejectUnauthorized,
-      requestCert: true,
-      secureContext: secureContext,
-      isServer: false
+    this.stream.removeAllListeners('data');
+    const secureSocket = Tls.connect({
+      rejectUnauthorized,
+      requestCert: rejectUnauthorized,
+      secureContext,
+      isServer: false,
+      socket: this.stream,
+      servername
+    }, () => {
+      secureEstablished = true;
+      if (rejectUnauthorized) {
+        if (typeof servername === 'string' && verifyIdentity) {
+          const cert = secureSocket.getPeerCertificate(true);
+          const serverIdentityCheckError = Tls.checkServerIdentity(servername, cert);
+          if (serverIdentityCheckError) {
+            onSecure(serverIdentityCheckError);
+            return;
+          }
+        }
+      }
+      onSecure();
     });
-    if (typeof host === 'string') {
-      secureSocket.setServername(host);
-    }
     // error handler for secure socket
-    secureSocket.on('_tlsError', err => {
+    secureSocket.on('error', err => {
       if (secureEstablished) {
         this._handleNetworkError(err);
       } else {
         onSecure(err);
       }
     });
-    secureSocket.on('secure', () => {
-      secureEstablished = true;
-      let callbackValue = null;
-      if (rejectUnauthorized) {
-        callbackValue = secureSocket.ssl.verifyError()
-        if (!callbackValue && typeof host === 'string' && verifyIdentity) {
-          const cert = secureSocket.ssl.getPeerCertificate(true);
-          callbackValue = Tls.checkServerIdentity(host, cert)
-        }
-      }
-      onSecure(callbackValue);
-    });
     secureSocket.on('data', data => {
       this.packetParser.execute(data);
     });
-    this.write = buffer => {
-      secureSocket.write(buffer);
-    };
-    // start TLS communications
-    secureSocket._start();
-  }
-
-  pipe() {
-    if (this.stream instanceof Net.Stream) {
-      this.stream.ondata = (data, start, end) => {
-        this.packetParser.execute(data, start, end);
-      };
-    } else {
-      this.stream.on('data', data => {
-        this.packetParser.execute(
-          data.parent,
-          data.offset,
-          data.offset + data.length
-        );
-      });
-    }
+    this.write = buffer => secureSocket.write(buffer);
   }
 
   protocolError(message, code) {
@@ -948,48 +929,4 @@ class Connection extends EventEmitter {
   }
 }
 
-if (Tls.TLSSocket) {
-  // not supported
-} else {
-  Connection.prototype.startTLS = function _startTLS(onSecure) {
-    if (this.config.debug) {
-      // eslint-disable-next-line no-console
-      console.log('Upgrading connection to TLS');
-    }
-    const crypto = require('crypto');
-    const config = this.config;
-    const stream = this.stream;
-    const rejectUnauthorized = this.config.ssl.rejectUnauthorized;
-    const credentials = crypto.createCredentials({
-      key: config.ssl.key,
-      cert: config.ssl.cert,
-      passphrase: config.ssl.passphrase,
-      ca: config.ssl.ca,
-      ciphers: config.ssl.ciphers
-    });
-    const securePair = Tls.createSecurePair(
-      credentials,
-      false,
-      true,
-      rejectUnauthorized
-    );
-
-    if (stream.ondata) {
-      stream.ondata = null;
-    }
-    stream.removeAllListeners('data');
-    stream.pipe(securePair.encrypted);
-    securePair.encrypted.pipe(stream);
-    securePair.cleartext.on('data', data => {
-      this.packetParser.execute(data);
-    });
-    this.write = function(buffer) {
-      securePair.cleartext.write(buffer);
-    };
-    securePair.on('secure', () => {
-      onSecure(rejectUnauthorized ? securePair.ssl.verifyError() : null);
-    });
-  };
-}
-
 module.exports = Connection;
diff --git a/test/builtin-runner/regressions/2052.test.mjs b/test/builtin-runner/regressions/2052.test.mjs
@@ -10,6 +10,7 @@ describe(
   () => {
     it('should report 0 actual parameters when 1 placeholder is used in ORDER BY ?', (t, done) => {
       const connection = {
+        sequenceId: 1,
         constructor: {
           statementKey: () => 0,
         },
@@ -23,9 +24,10 @@ describe(
         },
         writePacket: (packet) => {
           // client -> server COM_PREPARE
+          packet.writeHeader(1);
           assert.equal(
             packet.buffer.toString('hex'),
-            '000000001673656c656374202a2066726f6d207573657273206f72646572206279203f'
+            '1f0000011673656c656374202a2066726f6d207573657273206f72646572206279203f'
           );
         },
       };
@@ -68,7 +70,9 @@ describe(
   }
 );
 
-describe('E2E Prepare result with number of parameters incorrectly reported by the server', { timeout: 1000 }, () => {
+describe('E2E Prepare result with number of parameters incorrectly reported by the server', 
+  { timeout: 1000 }, 
+  () => {
   let connection;
 
   function isNewerThan8_0_22() {

diff --git a/test/common.js b/test/common.js
@@ -12,7 +12,7 @@ const config = {
   port: process.env.MYSQL_PORT || 3306
 };
 
-if (process.env.MYSQL_USE_TLS) {
+if (process.env.MYSQL_USE_TLS === '1') {
   config.ssl = {
     rejectUnauthorized: false,
     ca: fs.readFileSync(
@@ -32,7 +32,7 @@ exports.waitDatabaseReady = function(callback) {
   const tryConnect = function() {
     const conn = exports.createConnection({ database: 'mysql', password: process.env.MYSQL_PASSWORD });
     conn.once('error', err => {
-      if (err.code !== 'PROTOCOL_CONNECTION_LOST' && err.code !== 'ETIMEDOUT') {
+      if (err.code !== 'PROTOCOL_CONNECTION_LOST' && err.code !== 'ETIMEDOUT' && err.code !== 'ECONNREFUSED') {
         console.log('Unexpected error waiting for connection', err);
         process.exit(-1);
       }
@@ -84,6 +84,7 @@ exports.createConnection = function(args) {
     typeCast: args && args.typeCast,
     namedPlaceholders: args && args.namedPlaceholders,
     connectTimeout: args && args.connectTimeout,
+    ssl: (args && args.ssl) ?? config.ssl,
   };
 
   const conn = driver.createConnection(params);
@@ -164,10 +165,12 @@ exports.createServer = function(onListening, handler) {
   const server = require('../index.js').createServer();
   server.on('connection', conn => {
     conn.on('error', () => {
-      // we are here when client drops connection
+      // server side of the connection
+      // ignore disconnects
     });
+    // remove ssl bit from the flags
     let flags = 0xffffff;
-    flags = flags ^ ClientFlags.COMPRESS;
+    flags = flags ^ (ClientFlags.COMPRESS | ClientFlags.SSL);
 
     conn.serverHandshake({
       protocolVersion: 10,

diff --git a/test/integration/connection/test-disconnects.js b/test/integration/connection/test-disconnects.js
@@ -23,7 +23,8 @@ const server = common.createServer(
       // different host provided via MYSQL_HOST that identifies a real MySQL
       // server instance.
       host: 'localhost',
-      port: server._port
+      port: server._port,
+      ssl: false
     });
     connection.query('SELECT 123', (err, _rows, _fields) => {
       if (err) {

diff --git a/test/integration/connection/test-protocol-errors.js b/test/integration/connection/test-protocol-errors.js
@@ -22,7 +22,8 @@ const server = common.createServer(
       // different host provided via MYSQL_HOST that identifies a real MySQL
       // server instance.
       host: 'localhost',
-      port: server._port
+      port: server._port,
+      ssl: false
     });
     connection.query(query, (err, _rows, _fields) => {
       if (err) {

diff --git a/test/integration/connection/test-quit.js b/test/integration/connection/test-quit.js
@@ -22,7 +22,8 @@ const server = common.createServer(
       // different host provided via MYSQL_HOST that identifies a real MySQL
       // server instance.
       host: 'localhost',
-      port: server._port
+      port: server._port,
+      ssl: false
     });
 
     connection.query(queryCli, (err, _rows, _fields) => {

diff --git a/test/integration/connection/test-select-ssl.js b/test/integration/connection/test-select-ssl.js
@@ -0,0 +1,15 @@
+'use strict';
+
+const assert = require('assert');
+const common = require('../../common');
+const connection = common.createConnection();
+
+connection.query(`SHOW STATUS LIKE 'Ssl_cipher'`, (err, rows) => {
+  assert.ifError(err);
+  if (process.env.MYSQL_USE_TLS === '1') {
+    assert.equal(rows[0].Value.length > 0, true);
+  } else {
+    assert.deepEqual(rows, [{ Variable_name: 'Ssl_cipher', Value: '' }]);
+  }
+  connection.end();
+});