Invalid Snowflake account produces invalid URL, unhandledPromiseRejection, and swallows error

[markandrus]

Reproduction

You can put this test.js file in the root of the repo to reproduce:

// test.js

const snowflake = require('.') // or require('snowflake-sdk')

process.on('unhandledRejection', err => {
  // This should not execute, but it does, because the error is not handled by snowflake-sdk.
  console.error('unhandledRejection', err)
})

const connection = snowflake.createConnection({
  account: 'This produces an invalid URL',
  database: 'Snowflake Database',
  password: 'Snowflake Password',
  role: 'Snowflake Role',
  schema: 'Snowflake Schema',
  username: 'Snowflake User',
  warehouse: 'Snowflake Warehouse'
})

connection.connect(err => {
  // This should execute, but it does not.
  console.error('connection.connect', err)
})

$ node test.js
(node:316) NOTE: We are formalizing our plans to enter AWS SDK for JavaScript (v2) into maintenance mode in 2023.

Please migrate your code to use AWS SDK for JavaScript (v3).
For more information, check the migration guide at https://a.co/7PzMCcy
(Use `node --trace-warnings ...` to show where the warning was created)
unhandledRejection TypeError [ERR_INVALID_URL]: Invalid URL
    at new NodeError (node:internal/errors:400:5)
    at URL.onParseError (node:internal/url:565:9)
    at new URL (node:internal/url:645:5)
    at Object.requestWithCallback (snowflake-connector-nodejs/node_modules/urllib/lib/urllib.js:233:33)
    at Object.request (snowflake-connector-nodejs/node_modules/urllib/lib/urllib.js:154:20)
    at NodeHttpClient.sendRequest (snowflake-connector-nodejs/lib/http/base.js:98:33)
    at Gzip.cb (snowflake-connector-nodejs/lib/http/base.js:145:7)
    at Gzip.zlibBufferOnEnd (node:zlib:161:10)
    at Gzip.emit (node:events:513:28)
    at Gzip.emit (node:domain:489:12) {
  input: 'https://This produces an invalid URL.snowflakecomputing.com/session/v1/login-request?requestId=e6c95558-2d57-408b-b6a1-ac08e3f483af&warehouse=Snowflake%20Warehouse&databaseName=Snowflake%20Database&schemaName=Snowflake%20Schema&roleName=Snowflake%20Role',
  code: 'ERR_INVALID_URL'
}

Solutions

1. Improve URL parsing

snowflake-sdk should probably ensure the URLs it constructs are valid, and fail early (perhaps with a synchronous exception) if not. I don't know all the places in the snowflake-sdk that construct URLs, but my particular error arises (in part) here:

    if (dotPos < 0 && Util.isString(options.region) && options.region.length > 0)
    {
      options.accessUrl = Util.format('https://%s.%s.snowflakecomputing.com', options.account, options.region);
    }
    else
    {
      options.accessUrl = Util.format('https://%s.snowflakecomputing.com', options.account);
    }

Notice that the URLs aren't validated, they're just formatted strings, so the resulting accessUrl is

https://This produces an invalid URL.snowflakecomputing.com/session/v1/login-request?requestId=e6c95558-2d57-408b-b6a1-ac08e3f483af&warehouse=Snowflake%20Warehouse&databaseName=Snowflake%20Database&schemaName=Snowflake%20Schema&roleName=Snowflake%20Role

2. try/catch around require('urllib').request

We could also put a try/catch here in lib/http/base.js like this:

  try {
    request = require('urllib').request(requestOptions.url, requestOptions, async function (err, data, response)
      {
        // if a callback has been specified, normalize the
        // response before passing it to the callback
        if (Util.isFunction(options.callback))
        {
          if (err)
          {
            await options.callback(err, normalizeResponse(null), null);
          }
          else
          {
            response.body = response.data.toString();
            await options.callback(null, normalizeResponse(response), data.toString());
          }
        }
      });
  } catch (err) {
    if (Util.isFunction(options.callback))
    {
      // This will lead to retries, which we do not want in the case of INVALID_URL.
      await options.callback(err, normalizeResponse(null), null);
    }
  }

But this will lead to retries, which we do not want in the case of INVALID_URL. Instead, we'd like to error early.

[sfc-gh-dszmolka]

hi and thank you for raising this issue! it is surely not a bug - it's the responsibility of the user to provide the account as per the documented format. (either regionless: myorg-myaccount , or the locator version e.g. myaccount.us-east-2.aws)

but, i agree there's also room for improvement or at least handle these user errors more gracefully, so we'll take a look. appreciate your inputs for the possible ways forward !

[markandrus]

@sfc-gh-dszmolka thank you for your response. In our application, we receive the account from the end user and pass it along to snowflake-sdk. It is untrusted input, so is there a function in snowflake-sdk that can validate the account string?

[sfc-gh-dszmolka]

at this moment of writing, I'm not aware of such validation for account (except it needing to be a string) so if it's really coming from an upper layer, at this moment I guess you'll need to validate it at the application layer before passing it on to snowflake-sdk.

[sfc-gh-dszmolka]

should be resolved with #692 , review in progress

[sfc-gh-dszmolka]

#692 merged into main and will be part of the next upcoming release. will update this issue once the release is out.

[sfc-gh-dszmolka]

node.js driver version 1.9.1 released with the fix and is available on npm. thank you all for bearing with us !