From 87b6c4b339709346c7efa6f9bcac8fecc39069fb Mon Sep 17 00:00:00 2001 From: Przemyslaw Motacki Date: Thu, 5 Oct 2023 14:54:08 +0200 Subject: [PATCH] SNOW-334890 - refactor of the base httpClient --- lib/authentication/auth_web.js | 4 +- lib/authentication/sso_url_provider.js | 15 +- lib/connection/connection.js | 4 +- lib/http/base.js | 177 ++++++++---------- lib/http/node.js | 5 +- .../authentication/authentication_test.js | 12 +- 6 files changed, 88 insertions(+), 129 deletions(-) diff --git a/lib/authentication/auth_web.js b/lib/authentication/auth_web.js index 2f383affe..a95b3a703 100644 --- a/lib/authentication/auth_web.js +++ b/lib/authentication/auth_web.js @@ -7,7 +7,7 @@ const net = require('net'); const querystring = require('querystring'); const URLUtil = require('./../../lib/url_util'); const Util = require('./../../lib/util'); -var SsoUrlProvider = require('../authentication/sso_url_provider'); +const SsoUrlProvider = require('../authentication/sso_url_provider'); /** * Creates an external browser authenticator. @@ -23,7 +23,7 @@ function auth_web(connectionConfig, httpClient, webbrowser) { const host = connectionConfig.host; const browserActionTimeout = connectionConfig.getBrowserActionTimeout(); - const ssoUrlProvider = new SsoUrlProvider(connectionConfig, httpClient); + const ssoUrlProvider = new SsoUrlProvider(httpClient); if (!Util.exists(host)) { throw new Error(`Invalid value for host: ${host}`); diff --git a/lib/authentication/sso_url_provider.js b/lib/authentication/sso_url_provider.js index 862f52a7b..d84ad242e 100644 --- a/lib/authentication/sso_url_provider.js +++ b/lib/authentication/sso_url_provider.js @@ -9,13 +9,11 @@ const { rest } = require('../global_config'); /** * Creates a new instance of an SsoUrlProvider. * - * @param {Object} connectionConfig * @param {Object} httpClient * @constructor */ -function SsoUrlProvider(connectionConfig, httpClient) { +function SsoUrlProvider(httpClient) { - Errors.assertInternal(Util.isObject(connectionConfig)); Errors.assertInternal(Util.isObject(httpClient)); const port = rest.HTTPS_PORT; @@ -54,18 +52,17 @@ function SsoUrlProvider(connectionConfig, httpClient) { } }; - const agent = httpClient.getAgent(url, connectionConfig.getProxy()); - const requestOptions = { + method: 'post', + url: url, headers: header, - requestOCSP: false, - rejectUnauthorized: true, - httpsAgent: agent + data: body, + responseType: 'json' }; // Post request to get the SSO URL - return httpClient.post(url, body, requestOptions) + return httpClient.requestAsync(requestOptions) .then((response) => { const data = response['data']['data']; return data; diff --git a/lib/connection/connection.js b/lib/connection/connection.js index 3f0733c85..c09192a82 100644 --- a/lib/connection/connection.js +++ b/lib/connection/connection.js @@ -236,7 +236,7 @@ function Connection(context) } // Get authenticator to use - var auth = Authenticator.getAuthenticator(connectionConfig, context.getServices().ssoUrlProvider); + const auth = Authenticator.getAuthenticator(connectionConfig, context.getServices().ssoUrlProvider); try { @@ -297,7 +297,7 @@ function Connection(context) var self = this; // Get authenticator to use - var auth = Authenticator.getAuthenticator(connectionConfig, context.getHttpClient()); + const auth = Authenticator.getAuthenticator(connectionConfig, context.getHttpClient()); try { diff --git a/lib/http/base.js b/lib/http/base.js index 1793fa01b..2156310f2 100644 --- a/lib/http/base.js +++ b/lib/http/base.js @@ -7,6 +7,7 @@ const Util = require('../util'); const Errors = require('../errors'); const Logger = require('../logger'); const axios = require('axios'); +const Url = require("url"); const DEFAULT_REQUEST_TIMEOUT = 360000; @@ -34,69 +35,9 @@ HttpClient.prototype.getConnectionConfig = function () * * @returns {Object} an object representing the request that was issued. */ -HttpClient.prototype.request = function (options) -{ - // validate input - Errors.assertInternal(Util.isObject(options)); - Errors.assertInternal(Util.isString(options.method)); - Errors.assertInternal(Util.isString(options.url)); - Errors.assertInternal(!Util.exists(options.headers) || - Util.isObject(options.headers)); - Errors.assertInternal(!Util.exists(options.json) || - Util.isObject(options.json)); - Errors.assertInternal(!Util.exists(options.callback) || - Util.isFunction(options.callback)); - - var headers; - var json; - var body; - var request; - - // normalize the headers - headers = normalizeHeaders(options.headers); - - // create a function to send the request - var sendRequest = async function sendRequest() - { - const url = options.url; - - const timeout = options.timeout || - this._connectionConfig.getTimeout() || - DEFAULT_REQUEST_TIMEOUT; - - Logger.getInstance().trace(`CALL ${options.method} with timeout ${timeout}: ${url}`); - // build the basic request options - - var requestOptions = - { - method: options.method, - url: url, - headers: headers, - gzip: options.gzip, - json: json, - body: body, - timeout: timeout, - requestOCSP: true, - rejectUnauthorized: true, - // we manually parse jsons or other structures from the server so they need to be text - responseType: 'text', - }; - - let mock; - if (this._connectionConfig.agentClass) - { - mock = { - agentClass: this._connectionConfig.agentClass - } - } - - // add the agent and proxy options - const agent = this.getAgent(url, this._connectionConfig.getProxy(), mock); - - requestOptions.data = requestOptions.body; - requestOptions.httpsAgent = agent; - requestOptions.retryDelay = this.constructExponentialBackoffStrategy(); - +HttpClient.prototype.request = function (options) { + const requestOptions = prepareRequestOptions.call(this, options); + let sendRequest = async function sendRequest() { request = axios.request(requestOptions).then(response => { if (Util.isFunction(options.callback)) { return options.callback(null, normalizeResponse(response), response.data); @@ -119,55 +60,34 @@ HttpClient.prototype.request = function (options) }; sendRequest = sendRequest.bind(this); - // if a request body is specified and compression is enabled, - // try to compress the request body before sending the request - json = body = options.json; - if (body) - { - var bufferUncompressed = Buffer.from(JSON.stringify(body), 'utf8'); - zlib.gzip(bufferUncompressed, null, function (err, bufferCompressed) - { - // if the compression was successful - if (!err) - { - json = undefined; // don't specify the 'json' option - - // use the compressed buffer as the body and - // set the appropriate content encoding - body = bufferCompressed; - headers['Content-Encoding'] = 'gzip'; - } - else - { - Logger.getInstance().warn('Could not compress request body.'); - } - - sendRequest(); - }); - } - else - { - if (body) - { - Logger.getInstance().trace('Request body compression disabled.'); - } - - process.nextTick(sendRequest); - } + Logger.getInstance().trace(`CALL ${requestOptions.method} with timeout ${requestOptions.timeout}: ${requestOptions.url}`); + process.nextTick(sendRequest); // return an externalized request object that only contains // methods we're comfortable exposing to the outside world return { - abort: function () - { - if (request) - { + abort: function () { + if (request) { request.abort(); } } }; }; +/** + * Issues an HTTP request. + * + * @param {Object} options + * + * @returns {Object} an object representing the request that was issued. + */ +HttpClient.prototype.requestAsync = async function (options) +{ + const requestOptions = prepareRequestOptions.call(this, options); + + return axios.request(requestOptions); +}; + /** * @abstract * Returns the module to use when making HTTP requests. Subclasses must override @@ -196,6 +116,59 @@ HttpClient.prototype.post = axios.post; module.exports = HttpClient; +function prepareRequestOptions(options) { + let headers = normalizeHeaders(options.headers) || {}; + + const timeout = options.timeout || + this._connectionConfig.getTimeout() || + DEFAULT_REQUEST_TIMEOUT; + + let data = options.data || options.json; + + if (data) { + var bufferUncompressed = Buffer.from(JSON.stringify(data), 'utf8'); + zlib.gzip(bufferUncompressed, null, function (err, bufferCompressed) { + // if the compression was successful + if (!err) { + data = bufferCompressed; + headers['Content-Encoding'] = 'gzip'; + } else { + Logger.getInstance().warn('Could not compress request data.'); + } + }); + } + let mock; + if (this._connectionConfig.agentClass) { + mock = { + agentClass: this._connectionConfig.agentClass + } + } + const backoffStrategy = this.constructExponentialBackoffStrategy(); + const requestOptions = { + method: options.method, + url: options.url, + headers: headers, + data: data, + timeout: timeout, + requestOCSP: true, + retryDelay: backoffStrategy, + rejectUnauthorized: true, + // we manually parse jsons or other structures from the server so they need to be text + responseType: options.responseType || 'text', + } + + const parsedUrl = Url.parse(url); + const isHttps = parsedUrl.protocol === 'https:'; + const agent = this.getAgent(options.url, isHttps, this._connectionConfig.getProxy(), mock); + if (isHttps) { + requestOptions.httpsAgent = agent; + } else { + requestOptions.httpAgent = agent; + } + + return requestOptions; +} + /** * Normalizes a request headers object so that we get the same behavior * regardless of whether we're using request.js or browser-request.js. diff --git a/lib/http/node.js b/lib/http/node.js index 57f873d9f..5c3d3abcb 100644 --- a/lib/http/node.js +++ b/lib/http/node.js @@ -2,7 +2,6 @@ * Copyright (c) 2015-2019 Snowflake Computing Inc. All rights reserved. */ -const Url = require('url'); const Util = require('../util'); const Base = require('./base'); const HttpsAgent = require('../agent/https_ocsp_agent'); @@ -94,14 +93,12 @@ function isBypassProxy(proxy, url, bypassProxy) { /** * @inheritDoc */ -NodeHttpClient.prototype.getAgent = function (url, proxy, mock) { +NodeHttpClient.prototype.getAgent = function (url, isHttps, proxy, mock) { const agentOptions = {keepAlive: GlobalConfig.getKeepAlive()}; if (mock) { return mock.agentClass(agentOptions); } - const parsedUrl = Url.parse(url); - const isHttps = parsedUrl.protocol === 'https:'; const bypassProxy = isBypassProxy(proxy, url); let agent = {}; diff --git a/test/unit/authentication/authentication_test.js b/test/unit/authentication/authentication_test.js index add03a563..a14a7dd2f 100644 --- a/test/unit/authentication/authentication_test.js +++ b/test/unit/authentication/authentication_test.js @@ -92,11 +92,7 @@ describe('external browser authentication', function () } }); mock('httpclient', { - getAgent: function (url, body, header) - { - return new HttpAgent(); - }, - post: async function (url, body, header) + requestAsync: async function (options) { const data = { @@ -152,11 +148,7 @@ describe('external browser authentication', function () }); mock('httpclient', { - getAgent: function (url, body, header) - { - return new HttpAgent(); - }, - post: async function (url, body, header) + requestAsync: async function (options) { const data = {