Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow windows users to choose their own driver. #1428

Open
ctgbarcalow opened this issue Sep 30, 2022 · 1 comment
Open

Allow windows users to choose their own driver. #1428

ctgbarcalow opened this issue Sep 30, 2022 · 1 comment

Comments

@ctgbarcalow
Copy link

ctgbarcalow commented Sep 30, 2022

Right now windows users are stuck with the 'SQL Server Native Client 11.0' driver. Why not let developers choose whether we want the ODBC driver or the Native Client? The biggest driver for me is that the SQL Server Native Client doesn't support column encryption.

Also adding ColumnEncryption to CONNECTION_STRING_PORT and CONNECTION_STRING_NAMED_INSTANCE would to be super cool too.

Expected behaviour:

Encrypted columns to return their decrypted form.

Actual behaviour:

Encrypted columns return their encrypted byte code.

Configuration:

const config = {
  server: "SERVER",
  port: 1433,
  database: "scrap",
  driver: "ODBC Driver 18 for SQL Server",
  options: {
    trustedConnection: true,
    instanceName: "DEV",
    ColumnEncryption: true
  },
};

Software versions

Here's the modified connection-pool.js necessary to make the requested changes to the code.

connection-pool.js

'use strict'

const msnodesql = require('msnodesqlv8')
const debug = require('debug')('mssql:msv8')
const BaseConnectionPool = require('../base/connection-pool')
const { IDS, INCREMENT } = require('../utils')
const shared = require('../shared')
const ConnectionError = require('../error/connection-error')
const { platform } = require('os')

const SUPPORTED_DRIVERS = ['ODBC Driver 17 for SQL Server','ODBC Driver 18 for SQL Server','SQL Server Native Client 11.0'];
const CONNECTION_DRIVER = ['darwin', 'linux'].includes(platform()) ? 'ODBC Driver 17 for SQL Server' : 'SQL Server Native Client 11.0'
// The addition of ColumnEncryption allows that feature to be used.
const CONNECTION_STRING_PORT = `Driver=#{driver};Server=#{server},#{port};Database=#{database};Uid=#{user};Pwd=#{password};Trusted_Connection=#{trusted};Encrypt=#{encrypt};ColumnEncryption=#{ColumnEncryption};` 
const CONNECTION_STRING_NAMED_INSTANCE = `Driver=#{driver};Server=#{server}\\#{instance};Database=#{database};Uid=#{user};Pwd=#{password};Trusted_Connection=#{trusted};Encrypt=#{encrypt};ColumnEncryption=#{ColumnEncryption};`

class ConnectionPool extends BaseConnectionPool {
  _poolCreate () {
    return new shared.Promise((resolve, reject) => {
      let defaultConnectionString = CONNECTION_STRING_PORT

      if (this.config.options.instanceName != null) {
        defaultConnectionString = CONNECTION_STRING_NAMED_INSTANCE
      }

      this.config.requestTimeout = this.config.requestTimeout ?? this.config.timeout ?? 15000

      const cfg = {
        conn_str: this.config.connectionString || defaultConnectionString,
        conn_timeout: (this.config.connectionTimeout ?? this.config.timeout ?? 15000) / 1000
      }

      cfg.conn_str = cfg.conn_str.replace(/#{([^}]*)}/g, (p) => {
        const key = p.substr(2, p.length - 3)

        switch (key) {
          case 'driver':
            // This allows windows users to choose thier driver
            if (this.config.driver && !SUPPORTED_DRIVERS.some(valid => valid === this.config.driver)) return reject("Unsupported driver")
            return this.config.driver ?? CONNECTION_DRIVER;
          case 'instance':
            return this.config.options.instanceName
          case 'trusted':
            return this.config.options.trustedConnection ? 'Yes' : 'No'
          case 'encrypt':
            return this.config.options.encrypt ? 'Yes' : 'No'
          default:
            return this.config[key] != null ? this.config[key] : ''
        }
      })

      const connedtionId = INCREMENT.Connection++
      debug('pool(%d): connection #%d created', IDS.get(this), connedtionId)
      debug('connection(%d): establishing', connedtionId)

      if (typeof this.config.beforeConnect === 'function') {
        this.config.beforeConnect(cfg)
      }

      msnodesql.open(cfg, (err, tds) => {
        if (err) {
          err = new ConnectionError(err)
          return reject(err)
        }

        IDS.add(tds, 'Connection', connedtionId)
        tds.setUseUTC(this.config.options.useUTC)
        debug('connection(%d): established', IDS.get(tds))
        resolve(tds)
      })
    })
  }

  _poolValidate (tds) {
    if (tds && !tds.hasError) {
      return !this.config.validateConnection || new shared.Promise((resolve) => {
        tds.query('SELECT 1;', (err) => {
          resolve(!err)
        })
      })
    }
    return false
  }

  _poolDestroy (tds) {
    return new shared.Promise((resolve, reject) => {
      if (!tds) {
        resolve()
        return
      }
      debug('connection(%d): destroying', IDS.get(tds))
      tds.close(() => {
        debug('connection(%d): destroyed', IDS.get(tds))
        resolve()
      })
    })
  }
}

module.exports = ConnectionPool
@dhensby
Copy link
Collaborator

dhensby commented Oct 1, 2022

Absolutely, this has been something I've thought about too, but given lack of demand (and lack of production environment for me to test in), I've held back.

If you're prepared to make a contribution, would you mind creating a PR? We may have to consider how we can test these various drivers in CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants