Skip to content

Commit

Permalink
SNOW-334890 - refactor of the base httpClient
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-pmotacki committed Oct 6, 2023
1 parent e2984a5 commit 53eacf6
Show file tree
Hide file tree
Showing 9 changed files with 105 additions and 144 deletions.
4 changes: 2 additions & 2 deletions lib/authentication/auth_web.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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}`);
Expand Down
15 changes: 6 additions & 9 deletions lib/authentication/sso_url_provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lib/connection/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.getHttpClient());

try
{
Expand Down Expand Up @@ -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
{
Expand Down
2 changes: 1 addition & 1 deletion lib/connection/connection_context.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function ConnectionContext(connectionConfig, httpClient, config)
var services =
{
sf: new SfService(connectionConfig, httpClient, sfServiceConfig),
largeResultSet: new LargeResultSetService(connectionConfig, httpClient),
largeResultSet: new LargeResultSetService(connectionConfig, httpClient)
};

/**
Expand Down
179 changes: 75 additions & 104 deletions lib/http/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const Util = require('../util');
const Errors = require('../errors');
const Logger = require('../logger');
const axios = require('axios');
const URL = require('node:url').URL;

const DEFAULT_REQUEST_TIMEOUT = 360000;

Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -192,10 +112,61 @@ HttpClient.prototype.getAgent = function (url, proxy, mock) {
return null;
};

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 url = new URL(options.url);
const isHttps = url.protocol === 'https:';
const agent = this.getAgent(url, 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.
Expand Down
14 changes: 8 additions & 6 deletions lib/http/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
* 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');
const HttpsProxyAgent = require('../agent/https_proxy_ocsp_agent');
const HttpAgent = require('http').Agent;
const GlobalConfig = require('../../lib/global_config');
const Logger = require('../logger');
const Url = require("url");

/**
* Returns the delay time calculated by exponential backoff with
Expand Down Expand Up @@ -94,16 +94,18 @@ function isBypassProxy(proxy, url, bypassProxy) {
/**
* @inheritDoc
*/
NodeHttpClient.prototype.getAgent = function (url, proxy, mock) {
NodeHttpClient.prototype.getAgent = function (parsedUrl, proxy, mock) {
const agentOptions = {keepAlive: GlobalConfig.getKeepAlive()};
if (mock) {
return mock.agentClass(agentOptions);
const mockAgent = mock.agentClass(agentOptions);
if (mockAgent.protocol === parsedUrl.protocol) {
return mockAgent;
}
}

const parsedUrl = Url.parse(url);
const isHttps = parsedUrl.protocol === 'https:';
const bypassProxy = isBypassProxy(proxy, url);
const bypassProxy = isBypassProxy(proxy, parsedUrl.href);
let agent = {};
const isHttps = parsedUrl.protocol === 'https:';

if (isHttps) {
if (proxy && !bypassProxy) {
Expand Down
1 change: 0 additions & 1 deletion test/integration/ocsp_mock/https_ocsp_mock_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/

const HttpsAgent = require('https').Agent;
const Util = require('../../../lib/util');
const SocketUtil = require('../../../lib/agent/socket_util');
const Errors = require('../../../lib/errors');
const ErrorCodes = Errors.codes;
Expand Down
11 changes: 7 additions & 4 deletions test/integration/ocsp_mock/testConnectionOcspMock.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const connOption = require('../connectionOptions');
const Errors = require('../../../lib/errors');
const ErrorCodes = Errors.codes;
const HttpsMockAgent = require('./https_ocsp_mock_agent');
const Logger = require("../../../lib/logger");

function cloneConnOption(connOption)
{
Expand All @@ -20,9 +21,12 @@ function cloneConnOption(connOption)
return ret;
}

describe('Connection test with OCSP Mock', function ()
describe.only('Connection test with OCSP Mock', function ()
{
const valid = cloneConnOption(connOption.valid);

console.log(connOption.valid);

const isHttps = valid.accessUrl.startsWith("https");

function connect(errcode, connection, callback)
Expand All @@ -32,9 +36,8 @@ describe('Connection test with OCSP Mock', function ()
if (isHttps)
{
assert.equal(err.cause.code, errcode);
}
else
{
} else {
Logger.getInstance().info('Test can be run only for https protocol');
assert.ok(!err, JSON.stringify(err));
}
callback();
Expand Down
Loading

0 comments on commit 53eacf6

Please sign in to comment.