SNOW-1846395 improve url data sanitization (#980)

lib/constants/sf_params.js

@@ -1,2 +1,8 @@
-exports.paramsNames = {};
-exports.paramsNames.SF_REQUEST_GUID = 'request_guid';
+exports.paramsNames = Object.freeze({
+  SF_REQUEST_GUID: 'request_guid',
+  SF_REQUEST_ID: 'requestId',
+  SF_TOKEN: 'token',
+  SF_WAREHOUSE_NAME: 'warehouse',
+  SF_DB_NAME: 'databaseName',
+  SF_SCHEMA_NAME: 'schemaName',
+});

lib/errors.js

@@ -342,7 +342,7 @@ exports.createOperationFailedError = function (
   return createError(errorTypes.OperationFailedError,
     {
       code: errorCode,
-      data: { ...data, queryId: null },
+      data: data,
       message: message,
       sqlState: sqlState
     });

lib/http/node.js

@@ -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
@@ -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,
@@ -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;
     }
   }

lib/http/request_util.js

@@ -1,37 +1,202 @@
+const LoggingUtil = require('../logger/logging_util');
 const sfParams = require('../constants/sf_params');
 
+// Initial whitelist for attributes - they will be described with values
+const DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES = [
+  'baseUrl',
+  'path',
+  'method',
+  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
+const DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES = [
+  sfParams.paramsNames.SF_TOKEN
+];
+
+// Helper function to resolve attributes arrays given defaults and overrides.
+function resolveAttributeList(defaultAttrs, overrideAttrs) {
+  return overrideAttrs || defaultAttrs;
+}
+
 /**
- * Should work with not yet parsed options as well (before calling prepareRequestOptions method).
+ * Describes a request based on its options.
+ * Should work with not-yet-parsed options as well (before calling prepareRequestOptions method).
  *
- * @param requestOptions - object representing the request data with top-level keys
- * @returns {string}
+ * @param {Object} requestOptions - Object representing the request data with top-level keys.
+ * @param {Object} [options] - Options for describing attributes.
+ * @param {Array<string>} [options.overrideAttributesDescribedWithValues]
+ * @param {Array<string>} [options.overrideAttributesDescribedWithoutValues]
+ * @returns {string} A string representation of the request data.
  */
-exports.describeRequestFromOptions = function (requestOptions) {
-  return exports.describeRequestData({ method: requestOptions.method, url: requestOptions.url, guid: requestOptions.params?.[sfParams.paramsNames.SF_REQUEST_GUID] });
-};
+function describeRequestFromOptions(
+  requestOptions,
+  {
+    overrideAttributesDescribedWithValues,
+    overrideAttributesDescribedWithoutValues
+  } = {}
+) {
+  const describingAttributesWithValues = resolveAttributeList(
+    DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES,
+    overrideAttributesDescribedWithValues
+  );
+
+  const describingAttributesWithoutValues = resolveAttributeList(
+    DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES,
+    overrideAttributesDescribedWithoutValues
+  );
+
+  const { method, url, params } = requestOptions || {};
+
+  return describeRequestData(
+    { method, url, params },
+    describingAttributesWithValues,
+    describingAttributesWithoutValues
+  );
+}
 
 /**
- * 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}
+ * 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.overrideAttributesDescribedWithValues]
+ * @param {Array<string>} [options.overrideAttributesDescribedWithoutValues]
+ * @returns {string} A string representation of the request data.
  */
-exports.describeRequestFromResponse = function (response) {
-  let methodUppercase = undefined;
-  let url = undefined;
-  let guid = undefined;
-  const responseConfig = response.config;
+function describeRequestFromResponse(
+  response,
+  {
+    overrideAttributesDescribedWithValues,
+    overrideAttributesDescribedWithoutValues
+  } = {}
+) {
+  let method;
+  let url;
+  let params;
+  const responseConfig = response?.config;
+
+  const describingAttributesWithValues = resolveAttributeList(
+    DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES,
+    overrideAttributesDescribedWithValues
+  );
+
+  const describingAttributesWithoutValues = resolveAttributeList(
+    DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES,
+    overrideAttributesDescribedWithoutValues
+  );
+
   if (responseConfig) {
-    // Uppercase is used to allow finding logs corresponding to the same request - response pair,
-    // by matching identical options' descriptions.
-    methodUppercase = responseConfig.method.toUpperCase();
+    method = responseConfig.method;
     url = responseConfig.url;
-    guid = responseConfig.params?.[sfParams.paramsNames.SF_REQUEST_GUID];
+    params = responseConfig.params;
+  }
+
+  return describeRequestData(
+    { method, 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.
+ */
+function describeRequestData(
+  { method, url, params } = {},
+  attributesWithValues,
+  attributesWithoutValues
+) {
+  const requestObject = {
+    // Ensure consistent casing for methods to match request-response pairs in logs.
+    method: method?.toUpperCase(),
+    ...constructURLData(url, params),
+  };
+
+  return LoggingUtil.describeAttributes(
+    requestObject,
+    attributesWithValues,
+    attributesWithoutValues
+  );
+}
+
+/**
+ * Constructs an object representing URL data including the base URL, path, and query parameters.
+ *
+ * @param {string} url - The full URL.
+ * @param {Object} [params] - Additional query parameters.
+ * @returns {Object} Contains baseUrl, path, and merged query parameters.
+ */
+function constructURLData(url, params = {}) {
+  if (!url) {
+    return { baseUrl: undefined, path: undefined, queryParams: {} };
   }
-  return exports.describeRequestData({ method: methodUppercase, url: url, guid: guid });
-};
 
-exports.describeRequestData = function ({ method, url, guid } = {}) {
-  const guidDescription = guid ? `, guid: ${guid}` : '';
-  return `[method: ${method}, url: ${url}` + guidDescription + ']';
-};
+  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,
+  };
+}
+
+/**
+ * @param {string} url - The URL to describe.
+ * @param {Object} [options] - Options for describing attributes.
+ * @param {Array<string>} [options.overrideAttributesDescribedWithValues]
+ * @param {Array<string>} [options.overrideAttributesDescribedWithoutValues]
+ * @returns {string} A string describing the URL.
+ */
+function describeURL(
+  url,
+  {
+    overrideAttributesDescribedWithValues,
+    overrideAttributesDescribedWithoutValues
+  } = {}
+) {
+  const describingAttributesWithValues = resolveAttributeList(
+    DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES,
+    overrideAttributesDescribedWithValues
+  );
+
+  const describingAttributesWithoutValues = resolveAttributeList(
+    DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES,
+    overrideAttributesDescribedWithoutValues
+  );
+
+  const urlData = constructURLData(url);
+
+  return LoggingUtil.describeAttributes(
+    urlData,
+    describingAttributesWithValues,
+    describingAttributesWithoutValues
+  );
+}
+
+exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES = DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES;
+exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES = DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES;
+
+exports.describeRequestFromOptions = describeRequestFromOptions;
+exports.describeRequestFromResponse = describeRequestFromResponse;
+exports.describeURL = describeURL;

lib/logger/logging_util.js

@@ -1,8 +1,61 @@
 const Util = require('../util');
 
-exports.describePresence = function (valueToDescribe) {
-  const VAR_PROVIDED_TEXT = 'provided';
-  const VAR_NOT_PROVIDED_TEXT = 'not provided';
+const PROVIDED_TEXT = 'provided';
+const NOT_PROVIDED_TEXT = 'not provided';
 
-  return Util.isNotEmptyAsString(valueToDescribe) ? VAR_PROVIDED_TEXT : VAR_NOT_PROVIDED_TEXT;
+/**
+ * Describes the presence of a given value. If the value is not empty (as a string),
+ * returns the corresponding text (by default: 'provided' or 'not provided').
+ *
+ * @param {*} valueToDescribe - The value to check for presence.
+ * @param {Object} [options] - Optional overrides for the "provided" and "not provided" text.
+ * @param {string} [options.overrideProvidedText]
+ * @param {string} [options.overrideNotProvidedText]
+ * @returns {string} A string indicating the presence of `valueToDescribe`.
+ */
+exports.describePresence = function (valueToDescribe, { overrideProvidedText, overrideNotProvidedText } = {}) {
+  const providedText = overrideProvidedText || PROVIDED_TEXT;
+  const notProvidedText = overrideNotProvidedText || NOT_PROVIDED_TEXT;
+  return Util.isNotEmptyAsString(valueToDescribe) ? providedText : notProvidedText;
+};
+
+/**
+ * @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 (
+  sourceObject = {},
+  attributesWithValues = [],
+  attributesWithoutValues = []
+) {
+  const withValues = attributesWithValues
+    .filter(attr => sourceObject[attr] !== undefined)
+    .map(attr => `${attr}=${String(sourceObject[attr])}`);
+
+  const withoutValues = attributesWithoutValues
+    .filter(attr => sourceObject[attr] !== undefined)
+    .map(attr => `${attr} is ${exports.describePresence(sourceObject[attr])}`);
+
+  return [...withValues, ...withoutValues].join(', ');
+};
+
+/**
+ * @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}]`;
 };