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

Snow 1846395 improve url data sanitization #980

Merged
merged 58 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
5d74303
SNOW-1631790-Transport-Layer - Base HTTP client implemented
sfc-gh-fpawlowski Nov 17, 2024
4b2c001
SNOW-1631790-Transport-Layer - Base http client logging splitted into…
sfc-gh-fpawlowski Nov 17, 2024
df6f773
SNOW-1631790-Transport-Layer - Removed absent parameters from the doc…
sfc-gh-fpawlowski Nov 18, 2024
7725d96
SNOW-1631790-Transport-Layer - Improved readability by renaming varia…
sfc-gh-fpawlowski Nov 18, 2024
a858cb3
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver - Added guid to re…
sfc-gh-fpawlowski Nov 19, 2024
91628b1
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver - Guid logging imp…
sfc-gh-fpawlowski Nov 19, 2024
c79c2fc
SNOW-1631790-Transport-Layer - missing logs added for browser client
sfc-gh-fpawlowski Nov 19, 2024
45c1bbe
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: mock client adjus…
sfc-gh-fpawlowski Nov 19, 2024
e12d8ba
SNOW-1631790-Transport-Layer: Partial Identification of the HttpClien…
sfc-gh-fpawlowski Nov 22, 2024
b45999d
SNOW-1631790-Transport-Layer: Proxy description added to logs
sfc-gh-fpawlowski Nov 22, 2024
1d57d58
SNOW-1631790-Transport-Layer: Proxy redundant calls to getter eliminated
sfc-gh-fpawlowski Nov 22, 2024
b734f52
Merge branch 'master' into SNOW-1631790-Transport-Layer
sfc-gh-fpawlowski Nov 24, 2024
bcc9628
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Guid generated on…
sfc-gh-fpawlowski Nov 24, 2024
54d48e6
SNOW-1631790-Transport-Layer: Masking Tokens for '%' signs and parame…
sfc-gh-fpawlowski Nov 26, 2024
fc5531c
Merge branch 'master' into SNOW-1631790-Transport-Layer
sfc-gh-fpawlowski Nov 28, 2024
8e8998b
Merge branch 'master' into SNOW-1631790-Transport-Layer
sfc-gh-fpawlowski Nov 29, 2024
be0d198
Merge branch 'SNOW-1631790-Transport-Layer' into SNOW-1801434-Add-GUI…
sfc-gh-fpawlowski Dec 3, 2024
eb94b72
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: IncludeGuid extra…
sfc-gh-fpawlowski Dec 3, 2024
8d53d1a
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Added HttpClientW…
sfc-gh-fpawlowski Dec 4, 2024
cf39674
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Fixed tests for GUID
sfc-gh-fpawlowski Dec 4, 2024
6029097
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Formatting fix.
sfc-gh-fpawlowski Dec 4, 2024
adb13c4
Merge branch 'master' into SNOW-1631790-Transport-Layer
sfc-gh-fpawlowski Dec 4, 2024
69aee7b
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Tests with Interc…
sfc-gh-fpawlowski Dec 4, 2024
9674391
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Core instance inj…
sfc-gh-fpawlowski Dec 4, 2024
29a75a8
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: IncludeGuid added…
sfc-gh-fpawlowski Dec 4, 2024
8acb7aa
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Reverted extendin…
sfc-gh-fpawlowski Dec 4, 2024
691ede4
Merge branch 'SNOW-1631790-Transport-Layer' into SNOW-1801434-Add-GUI…
sfc-gh-fpawlowski Dec 5, 2024
fe9578b
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Removed unused co…
sfc-gh-fpawlowski Dec 5, 2024
895e356
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Fixed httpClient …
sfc-gh-fpawlowski Dec 5, 2024
d109f10
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Fixed httpClient …
sfc-gh-fpawlowski Dec 5, 2024
08cc997
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Removed old comments
sfc-gh-fpawlowski Dec 5, 2024
7fd05b9
Merge branch 'SNOW-1631790-Transport-Layer' into SNOW-1846395-Improve…
sfc-gh-fpawlowski Dec 5, 2024
86ff23f
Merge branch 'SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver' int…
sfc-gh-fpawlowski Dec 5, 2024
e9e96a2
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Add guid in async…
sfc-gh-fpawlowski Dec 5, 2024
a034e34
Merge branch 'SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver' int…
sfc-gh-fpawlowski Dec 5, 2024
e23851b
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Merged master
sfc-gh-fpawlowski Dec 5, 2024
31496c8
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: Rolled back to se…
sfc-gh-fpawlowski Dec 6, 2024
a8ddcfe
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: IncludeGuid chang…
sfc-gh-fpawlowski Dec 6, 2024
c05d219
Merge branch 'master' into SNOW-1801434-Add-GUID-to-request-in-NODE.j…
sfc-gh-fpawlowski Dec 6, 2024
9cf3a54
SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver: ExcludeGuid setti…
sfc-gh-fpawlowski Dec 6, 2024
e31141a
Merge branch 'SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver' int…
sfc-gh-fpawlowski Dec 6, 2024
f046ae4
SNOW-1846395-Improve-URL-data-sanitization: Made queryID available in…
sfc-gh-fpawlowski Dec 5, 2024
2395fad
SNOW-1846395-Improve-URL-data-sanitization: Added description functions
sfc-gh-fpawlowski Dec 6, 2024
b713232
SNOW-1846395-Improve-URL-data-sanitization: Added describing attribut…
sfc-gh-fpawlowski Dec 6, 2024
b68433b
SNOW-1846395-Improve-URL-data-sanitization: Added describing attribut…
sfc-gh-fpawlowski Dec 6, 2024
a9894e4
SNOW-1846395-Improve-URL-data-sanitization: Added describing attribut…
sfc-gh-fpawlowski Dec 6, 2024
9ca67ec
SNOW-1846395-Improve-URL-data-sanitization: Added describing attribut…
sfc-gh-fpawlowski Dec 6, 2024
c218e95
SNOW-1846395-Improve-URL-data-sanitization: Made undefined attributes…
sfc-gh-fpawlowski Dec 6, 2024
43fa436
SNOW-1846395-Improve-URL-data-sanitization: Formatting fixed
sfc-gh-fpawlowski Dec 6, 2024
aec74ec
Merge branch 'master' into SNOW-1801434-Add-GUID-to-request-in-NODE.j…
sfc-gh-fpawlowski Dec 6, 2024
d0f2e1f
Merge branch 'SNOW-1801434-Add-GUID-to-request-in-NODE.js-driver' int…
sfc-gh-fpawlowski Dec 6, 2024
0d15b8f
SNOW-1846395-Improve-URL-data-sanitization: Cleaned code exports
sfc-gh-fpawlowski Dec 6, 2024
2e640f3
SNOW-1846395-Improve-URL-data-sanitization: Tests draft
sfc-gh-fpawlowski Dec 8, 2024
6dfea7c
SNOW-1846395-Improve-URL-data-sanitization: Tests draft
sfc-gh-fpawlowski Dec 8, 2024
68d6ddb
SNOW-1846395-Improve-URL-data-sanitization: Tests added
sfc-gh-fpawlowski Dec 8, 2024
7bc95ea
SNOW-1846395-Improve-URL-data-sanitization: Comments added
sfc-gh-fpawlowski Dec 8, 2024
16e34f3
SNOW-1846395-Improve-URL-data-sanitization: Tests fixed
sfc-gh-fpawlowski Dec 8, 2024
1010afb
SNOW-1846395-Improve-URL-data-sanitization: Tests fixed
sfc-gh-fpawlowski Dec 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,14 @@ declare module 'snowflake-sdk' {
*/
requestId?: string;

/**
* The request GUID is a unique identifier of an HTTP request issued to Snowflake.
* Unlike the requestId, it is regenerated even when the request is resend with the retry mechanism.
* If not specified, request GUIDs are attached to all requests to Snowflake for better traceability.
* In the majority of cases it should not be set or filled with false value.
*/
excludeGuid?: string;

/**
* Use different rest endpoints based on whether the query id is available.
*/
Expand Down
8 changes: 8 additions & 0 deletions lib/constants/sf_params.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
exports.paramsNames = {
sfc-gh-fpawlowski marked this conversation as resolved.
Show resolved Hide resolved
SF_REQUEST_GUID: 'request_guid',
SF_REQUEST_ID: 'requestId',
SF_TOKEN: 'token',
SF_WAREHOUSE_NAME: 'warehouse',
SF_DB_NAME: 'databaseName',
SF_SCHEMA_NAME: 'schemaName',
};
2 changes: 1 addition & 1 deletion lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ exports.createOperationFailedError = function (
return createError(errorTypes.OperationFailedError,
{
code: errorCode,
data: { ...data, queryId: null },
data: data,
message: message,
sqlState: sqlState
});
Expand Down
9 changes: 5 additions & 4 deletions lib/http/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const HttpsProxyAgent = require('../agent/https_proxy_agent');
const HttpAgent = require('http').Agent;
const GlobalConfig = require('../../lib/global_config');
const Logger = require('../logger');
const RequestUtil = require('../http/request_util');

/**
* Returns the delay time calculated by exponential backoff with
Expand Down Expand Up @@ -107,19 +108,19 @@ function isBypassProxy(proxy, destination, agentId) {
* @inheritDoc
*/
NodeHttpClient.prototype.getAgent = function (parsedUrl, proxy, mock) {
Logger.getInstance().trace('Agent[url: %s] - getting an agent instance.', parsedUrl.href);
Logger.getInstance().trace('Agent[url: %s] - getting an agent instance.', RequestUtil.describeURL(parsedUrl.href));
if (!proxy && GlobalConfig.isEnvProxyActive()) {
const isHttps = parsedUrl.protocol === 'https:';
proxy = ProxyUtil.getProxyFromEnv(isHttps);
if (proxy) {
Logger.getInstance().debug('Agent[url: %s] - proxy info loaded from the environment variable. Proxy host: %s', parsedUrl.href, proxy.host);
Logger.getInstance().debug('Agent[url: %s] - proxy info loaded from the environment variable. Proxy host: %s', RequestUtil.describeURL(parsedUrl.href), proxy.host);
}
}
return getProxyAgent(proxy, parsedUrl, parsedUrl.href, mock);
};

function getProxyAgent(proxyOptions, parsedUrl, destination, mock) {
Logger.getInstance().trace('Agent[url: %s] - getting a proxy agent instance.', parsedUrl.href);
Logger.getInstance().trace('Agent[url: %s] - getting a proxy agent instance.', RequestUtil.describeURL(parsedUrl.href));
const agentOptions = {
protocol: parsedUrl.protocol,
hostname: parsedUrl.hostname,
Expand All @@ -129,7 +130,7 @@ function getProxyAgent(proxyOptions, parsedUrl, destination, mock) {
if (mock) {
const mockAgent = mock.agentClass(agentOptions);
if (mockAgent.protocol === parsedUrl.protocol) {
Logger.getInstance().debug('Agent[url: %s] - the mock agent will be used.', parsedUrl.href);
Logger.getInstance().debug('Agent[url: %s] - the mock agent will be used.', RequestUtil.describeURL(parsedUrl.href));
return mockAgent;
}
}
Expand Down
219 changes: 203 additions & 16 deletions lib/http/request_util.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,213 @@
const LoggingUtil = require('../logger/logging_util');
const sfParams = require('../constants/sf_params');

// Initial whitelist for attributes - they will be described with values
exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES = [
'baseUrl',
'path',
sfParams.paramsNames.SF_REQUEST_ID,
sfParams.paramsNames.SF_REQUEST_GUID,
sfParams.paramsNames.SF_WAREHOUSE_NAME,
sfParams.paramsNames.SF_DB_NAME,
sfParams.paramsNames.SF_SCHEMA_NAME,
];

// Initial blacklist for attributes - described as present/not present only
exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES = [
sfParams.paramsNames.SF_TOKEN
];

/**
* Describes a request based on its options.
* Should work with not-yet-parsed options as well (before calling prepareRequestOptions method).
*
* @param {Object} requestOptions - Object representing the request data with top-level keys.
* @param {Object} [options] - Options for describing attributes.
* @param {Array<string>} [options.additionalAttributesDescribedWithValues]
* @param {Array<string>} [options.overrideAttributesDescribedWithValues]
* @param {Array<string>} [options.additionalAttributesDescribedWithoutValues]
* @param {Array<string>} [options.overrideAttributesDescribedWithoutValues]
* @returns {string} A string representation of the request data.
*/
exports.describeRequestFromOptions = function (
sfc-gh-fpawlowski marked this conversation as resolved.
Show resolved Hide resolved
requestOptions,
{
additionalAttributesDescribedWithValues = [],
overrideAttributesDescribedWithValues,
additionalAttributesDescribedWithoutValues = [],
overrideAttributesDescribedWithoutValues
sfc-gh-astachowski marked this conversation as resolved.
Show resolved Hide resolved
} = {}
) {
const describingAttributesWithValues = mergeAttributeLists(
exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES,
sfc-gh-fpawlowski marked this conversation as resolved.
Show resolved Hide resolved
overrideAttributesDescribedWithValues,
additionalAttributesDescribedWithValues
);

const describingAttributesWithoutValues = mergeAttributeLists(
exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES,
sfc-gh-fpawlowski marked this conversation as resolved.
Show resolved Hide resolved
overrideAttributesDescribedWithoutValues,
additionalAttributesDescribedWithoutValues
);

const { method, url, params } = requestOptions || {};

return exports.describeRequestData(
{ method, url, params },
describingAttributesWithValues,
describingAttributesWithoutValues
);
};

/**
* Creates a string that represents request data from a response.
* Helps to identify the request that was the source of the response.
*
* @param {Object} response - Axios response object.
* @param {Object} [options] - Options for describing attributes.
* @param {Array<string>} [options.additionalAttributesDescribedWithValues]
* @param {Array<string>} [options.overrideAttributesDescribedWithValues]
* @param {Array<string>} [options.additionalAttributesDescribedWithoutValues]
* @param {Array<string>} [options.overrideAttributesDescribedWithoutValues]
* @returns {string} A string representation of the request data.
*/
exports.describeRequestFromResponse = function (
response,
{
additionalAttributesDescribedWithValues = [],
overrideAttributesDescribedWithValues,
additionalAttributesDescribedWithoutValues = [],
overrideAttributesDescribedWithoutValues
} = {}
) {
let methodUppercase;
let url;
let params;
const responseConfig = response.config;

const describingAttributesWithValues = mergeAttributeLists(
exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES,
overrideAttributesDescribedWithValues,
additionalAttributesDescribedWithValues
);

const describingAttributesWithoutValues = mergeAttributeLists(
exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES,
overrideAttributesDescribedWithoutValues,
additionalAttributesDescribedWithoutValues
);

if (responseConfig) {
// Ensure consistent casing for methods to match request-response pairs in logs.
methodUppercase = responseConfig.method?.toUpperCase();
url = responseConfig.url;
params = responseConfig.params;
}

return exports.describeRequestData(
{ method: methodUppercase, url, params },
describingAttributesWithValues,
describingAttributesWithoutValues
);
};

/**
* Constructs a string representation of request data.
*
* @param {Object} requestData - Object containing the method, url, and parameters.
* @param {string} requestData.method - HTTP method.
* @param {string} requestData.url - Request URL.
* @param {Object} [requestData.params] - Additional query parameters.
* @param {Array<string>} attributesWithValues - Attributes to describe with values.
* @param {Array<string>} attributesWithoutValues - Attributes to describe without values.
* @returns {string} A string describing the request data.
*/
exports.describeRequestData = function (
{ method, url, params } = {},
attributesWithValues,
attributesWithoutValues
) {
const requestObject = {
method,
...exports.constructURLData(url, params),
};

return LoggingUtil.describeAttributes(
requestObject,
attributesWithValues,
attributesWithoutValues
);
};

/**
* Should work with not yet parsed options as well (before calling prepareRequestOptions method).
* Constructs an object representing URL data including the base URL, path, and query parameters.
*
* @param requestOptions - object representing the request data with top-level keys
* @returns {string}
* @param {string} url - The full URL.
* @param {Object} [params] - Additional query parameters.
* @returns {Object} Contains baseUrl, path, and merged query parameters.
*/
exports.describeRequestFromOptions = function (requestOptions) {
return exports.describeRequestData({ method: requestOptions.method, url: requestOptions.url });
exports.constructURLData = function (url, params = {}) {
sfc-gh-fpawlowski marked this conversation as resolved.
Show resolved Hide resolved
if (!url) {
return { baseUrl: '', path: '', queryParams: {} };
}

const urlObj = new URL(url);
const queryParams = { ...params };

urlObj.searchParams.forEach((value, key) => {
queryParams[key] = value;
});

const baseUrl = `${urlObj.protocol}//${urlObj.hostname}${urlObj.port ? `:${urlObj.port}` : ''}`;

return {
baseUrl: baseUrl,
path: urlObj.pathname,
...queryParams,
};
};

/**
* Creates string that represents response data.
* Should allow to identify request, which was the source for this response.
* @param response - axios response object
* @returns {string}
* @param {string} url - The URL to describe.
* @param {Object} [options] - Options for describing attributes.
* @param {Array<string>} [options.additionalAttributesDescribedWithValues]
* @param {Array<string>} [options.overrideAttributesDescribedWithValues]
* @param {Array<string>} [options.additionalAttributesDescribedWithoutValues]
* @param {Array<string>} [options.overrideAttributesDescribedWithoutValues]
* @returns {string} A string describing the URL.
*/
exports.describeRequestFromResponse = function (response) {
// Uppercase is used to allow finding logs corresponding to the same request - response pair,
// by matching identical options' descriptions.
return exports.describeRequestData({ method: response.config?.method.toUpperCase(), url: response.config?.url });
exports.describeURL = function (
url,
{
additionalAttributesDescribedWithValues = [],
overrideAttributesDescribedWithValues,
additionalAttributesDescribedWithoutValues = [],
overrideAttributesDescribedWithoutValues
} = {}
) {
const describingAttributesWithValues = mergeAttributeLists(
exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES,
overrideAttributesDescribedWithValues,
additionalAttributesDescribedWithValues
);

const describingAttributesWithoutValues = mergeAttributeLists(
exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES,
overrideAttributesDescribedWithoutValues,
additionalAttributesDescribedWithoutValues
);

const urlData = exports.constructURLData(url);

return LoggingUtil.describeAttributes(
urlData,
describingAttributesWithValues,
describingAttributesWithoutValues
);
};

exports.describeRequestData = function (requestData) {
return `[method: ${requestData.method}, url: ${requestData.url}]`;
};
// Helper function to merge attribute arrays.
function mergeAttributeLists(defaultAttrs, overrideAttrs, additionalAttrs) {
const base = overrideAttrs || defaultAttrs;
return [...base, ...additionalAttrs];
}
46 changes: 43 additions & 3 deletions lib/logger/logging_util.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,48 @@
const Util = require('../util');

const PROVIDED_TEXT = 'provided';
const NOT_PROVIDED_TEXT = 'not provided';

exports.describePresence = function (valueToDescribe) {
const VAR_PROVIDED_TEXT = 'provided';
const VAR_NOT_PROVIDED_TEXT = 'not provided';
return Util.isNotEmptyAsString(valueToDescribe) ? PROVIDED_TEXT : NOT_PROVIDED_TEXT;
};

/**
* @param {Object} sourceObject - The object holding attribute values.
* @param {Array<string>} attributesWithValues - Attributes to show with their values.
* @param {Array<string>} attributesWithoutValues - Attributes to show as present/not present.
* @returns {string} Comma-separated string describing the attributes.
*/
exports.attributesToString = function (
sfc-gh-astachowski marked this conversation as resolved.
Show resolved Hide resolved
sourceObject = {},
attributesWithValues = [],
attributesWithoutValues = []
) {
const withValues = attributesWithValues
.filter(attr => sourceObject[attr] !== undefined)
.map(attr => `${attr}=${String(sourceObject[attr])}`);

const withoutValues = attributesWithoutValues
.map(attr => `${attr} is ${exports.describePresence(sourceObject[attr])}`);

return [...withValues, ...withoutValues].join(', ');
};

return Util.isNotEmptyAsString(valueToDescribe) ? VAR_PROVIDED_TEXT : VAR_NOT_PROVIDED_TEXT;
/**
* @param {Object} sourceObject - The object holding attribute values.
* @param {Array<string>} attributesWithValues - Attributes to show with their values.
* @param {Array<string>} attributesWithoutValues - Attributes to show as present/not present.
* @returns {string} A bracketed string of described attributes.
*/
exports.describeAttributes = function (
sourceObject,
attributesWithValues,
attributesWithoutValues
) {
const attributesDescription = exports.attributesToString(
sourceObject,
attributesWithValues,
attributesWithoutValues
);
return `[${attributesDescription}]`;
};
Loading
Loading