From a35f4686fc1e719b7cfda0262dd32b86e5eebdb7 Mon Sep 17 00:00:00 2001 From: Shivam Raj Date: Wed, 19 Feb 2025 01:26:13 +0530 Subject: [PATCH 1/4] renamed clientId to userAgentHeader in connect args --- examples/repl | 2 +- lib/DBSQLClient.ts | 2 +- lib/contracts/IDBSQLClient.ts | 2 +- lib/utils/buildUserAgentString.ts | 4 ++-- tests/unit/utils/utils.test.ts | 22 +++++++++++----------- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/examples/repl b/examples/repl index d214549c..effdccf1 100755 --- a/examples/repl +++ b/examples/repl @@ -15,7 +15,7 @@ async function initClient({ host, endpointId, token }) { host, path: `/sql/2.0/warehouses/${endpointId}`, token, - clientId: 'REPL', + userAgentHeader: 'REPL', }); } diff --git a/lib/DBSQLClient.ts b/lib/DBSQLClient.ts index d3d9427c..1e5a6d2a 100644 --- a/lib/DBSQLClient.ts +++ b/lib/DBSQLClient.ts @@ -111,7 +111,7 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I socketTimeout: options.socketTimeout, proxy: options.proxy, headers: { - 'User-Agent': buildUserAgentString(options.clientId), + 'User-Agent': buildUserAgentString(options.userAgentHeader), }, }; } diff --git a/lib/contracts/IDBSQLClient.ts b/lib/contracts/IDBSQLClient.ts index 0c0cee89..cd5116ff 100644 --- a/lib/contracts/IDBSQLClient.ts +++ b/lib/contracts/IDBSQLClient.ts @@ -30,7 +30,7 @@ export type ConnectionOptions = { host: string; port?: number; path: string; - clientId?: string; + userAgentHeader?: string; socketTimeout?: number; proxy?: ProxyOptions; } & AuthOptions; diff --git a/lib/utils/buildUserAgentString.ts b/lib/utils/buildUserAgentString.ts index ce26fcfd..bc78735a 100644 --- a/lib/utils/buildUserAgentString.ts +++ b/lib/utils/buildUserAgentString.ts @@ -11,7 +11,7 @@ function getOperatingSystemVersion(): string { return `${os.type()} ${os.release()}`; } -export default function buildUserAgentString(clientId?: string): string { - const extra = [clientId, getNodeVersion(), getOperatingSystemVersion()].filter(Boolean); +export default function buildUserAgentString(userAgentHeader?: string): string { + const extra = [userAgentHeader, getNodeVersion(), getOperatingSystemVersion()].filter(Boolean); return `${productName}/${packageVersion} (${extra.join('; ')})`; } diff --git a/tests/unit/utils/utils.test.ts b/tests/unit/utils/utils.test.ts index f94e88c4..4868b180 100644 --- a/tests/unit/utils/utils.test.ts +++ b/tests/unit/utils/utils.test.ts @@ -20,13 +20,13 @@ describe('buildUserAgentString', () => { // // UserAgent ::= '/' '(' ')' // ProductName ::= 'NodejsDatabricksSqlConnector' - // ::= [ ';' ] 'Node.js' ';' + // ::= [ ';' ] 'Node.js' ';' // // Examples: - // - with provided: NodejsDatabricksSqlConnector/0.1.8-beta.1 (Client ID; Node.js 16.13.1; Darwin 21.5.0) - // - without provided: NodejsDatabricksSqlConnector/0.1.8-beta.1 (Node.js 16.13.1; Darwin 21.5.0) + // - with provided: NodejsDatabricksSqlConnector/0.1.8-beta.1 (Client ID; Node.js 16.13.1; Darwin 21.5.0) + // - without provided: NodejsDatabricksSqlConnector/0.1.8-beta.1 (Node.js 16.13.1; Darwin 21.5.0) - function checkUserAgentString(ua: string, clientId?: string) { + function checkUserAgentString(ua: string, userAgentHeader?: string) { // Prefix: 'NodejsDatabricksSqlConnector/' // Version: three period-separated digits and optional suffix const re = @@ -38,18 +38,18 @@ describe('buildUserAgentString', () => { expect(comment.split(';').length).to.be.gte(2); // at least Node and OS version should be there - if (clientId) { - expect(comment.trim()).to.satisfy((s: string) => s.startsWith(`${clientId};`)); + if (userAgentHeader) { + expect(comment.trim()).to.satisfy((s: string) => s.startsWith(`${userAgentHeader};`)); } } - it('matches pattern with clientId', () => { - const clientId = 'Some Client ID'; - const ua = buildUserAgentString(clientId); - checkUserAgentString(ua, clientId); + it('matches pattern with userAgentHeader', () => { + const userAgentHeader = 'Some Client ID'; + const ua = buildUserAgentString(userAgentHeader); + checkUserAgentString(ua, userAgentHeader); }); - it('matches pattern without clientId', () => { + it('matches pattern without userAgentHeader', () => { const ua = buildUserAgentString(); checkUserAgentString(ua); }); From 85a0e6d096fdd35f4750eb3e140fb8063fe7ab08 Mon Sep 17 00:00:00 2001 From: Shivam Raj Date: Wed, 19 Feb 2025 22:35:37 +0530 Subject: [PATCH 2/4] addressed comments --- examples/repl | 2 +- lib/DBSQLClient.ts | 2 +- lib/contracts/IDBSQLClient.ts | 2 +- lib/utils/buildUserAgentString.ts | 4 ++-- tests/unit/utils/utils.test.ts | 24 +++++++++++++----------- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/examples/repl b/examples/repl index effdccf1..c546511b 100755 --- a/examples/repl +++ b/examples/repl @@ -15,7 +15,7 @@ async function initClient({ host, endpointId, token }) { host, path: `/sql/2.0/warehouses/${endpointId}`, token, - userAgentHeader: 'REPL', + userAgentEntry: 'REPL', }); } diff --git a/lib/DBSQLClient.ts b/lib/DBSQLClient.ts index 1e5a6d2a..ead39d9e 100644 --- a/lib/DBSQLClient.ts +++ b/lib/DBSQLClient.ts @@ -111,7 +111,7 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I socketTimeout: options.socketTimeout, proxy: options.proxy, headers: { - 'User-Agent': buildUserAgentString(options.userAgentHeader), + 'User-Agent': buildUserAgentString(options.userAgentEntry), }, }; } diff --git a/lib/contracts/IDBSQLClient.ts b/lib/contracts/IDBSQLClient.ts index cd5116ff..73fedde1 100644 --- a/lib/contracts/IDBSQLClient.ts +++ b/lib/contracts/IDBSQLClient.ts @@ -30,7 +30,7 @@ export type ConnectionOptions = { host: string; port?: number; path: string; - userAgentHeader?: string; + userAgentEntry?: string; socketTimeout?: number; proxy?: ProxyOptions; } & AuthOptions; diff --git a/lib/utils/buildUserAgentString.ts b/lib/utils/buildUserAgentString.ts index bc78735a..1cbbf177 100644 --- a/lib/utils/buildUserAgentString.ts +++ b/lib/utils/buildUserAgentString.ts @@ -11,7 +11,7 @@ function getOperatingSystemVersion(): string { return `${os.type()} ${os.release()}`; } -export default function buildUserAgentString(userAgentHeader?: string): string { - const extra = [userAgentHeader, getNodeVersion(), getOperatingSystemVersion()].filter(Boolean); +export default function buildUserAgentString(userAgentEntry?: string): string { + const extra = [userAgentEntry, getNodeVersion(), getOperatingSystemVersion()].filter(Boolean); return `${productName}/${packageVersion} (${extra.join('; ')})`; } diff --git a/tests/unit/utils/utils.test.ts b/tests/unit/utils/utils.test.ts index fe42170d..6f0369b2 100644 --- a/tests/unit/utils/utils.test.ts +++ b/tests/unit/utils/utils.test.ts @@ -19,14 +19,16 @@ describe('buildUserAgentString', () => { // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent // // UserAgent ::= '/' '(' ')' + // where: // ProductName ::= 'NodejsDatabricksSqlConnector' - // ::= [ ';' ] 'Node.js' ';' + // ProductVersion ::= three period-separated digits + // ::= [ ';' ] 'Node.js' ';' // // Examples: - // - with provided: NodejsDatabricksSqlConnector/0.1.8-beta.1 (Client ID; Node.js 16.13.1; Darwin 21.5.0) - // - without provided: NodejsDatabricksSqlConnector/0.1.8-beta.1 (Node.js 16.13.1; Darwin 21.5.0) + // - with provided: NodejsDatabricksSqlConnector/0.1.8-beta.1 (; Node.js 16.13.1; Darwin 21.5.0) + // - without provided: NodejsDatabricksSqlConnector/0.1.8-beta.1 (Node.js 16.13.1; Darwin 21.5.0) - function checkUserAgentString(ua: string, userAgentHeader?: string) { + function checkUserAgentString(ua: string, userAgentEntry?: string) { // Prefix: 'NodejsDatabricksSqlConnector/' // Version: three period-separated digits and optional suffix const re = @@ -39,18 +41,18 @@ describe('buildUserAgentString', () => { const parts = comment.split(';').map((s) => s.trim()); expect(parts.length).to.be.gte(2); // at least Node and OS version should be there - if (userAgentHeader) { - expect(comment.trim()).to.satisfy((s: string) => s.startsWith(`${userAgentHeader};`)); + if (userAgentEntry) { + expect(comment.trim()).to.satisfy((s: string) => s.startsWith(`${userAgentEntry};`)); } } - it('matches pattern with userAgentHeader', () => { - const userAgentHeader = 'Some Client ID'; - const ua = buildUserAgentString(userAgentHeader); - checkUserAgentString(ua, userAgentHeader); + it('matches pattern with userAgentEntry', () => { + const userAgentEntry = 'Some user agent'; + const ua = buildUserAgentString(userAgentEntry); + checkUserAgentString(ua, userAgentEntry); }); - it('matches pattern without userAgentHeader', () => { + it('matches pattern without userAgentEntry', () => { const ua = buildUserAgentString(); checkUserAgentString(ua); }); From cfbbd6f5aa2bf023000876614a7ece5207bebcc8 Mon Sep 17 00:00:00 2001 From: Shivam Raj Date: Sat, 22 Feb 2025 00:23:49 +0530 Subject: [PATCH 3/4] add warning for deprecated clientId param --- lib/DBSQLClient.ts | 8 ++++++++ tests/unit/DBSQLClient.test.ts | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/lib/DBSQLClient.ts b/lib/DBSQLClient.ts index ead39d9e..cc2f82e9 100644 --- a/lib/DBSQLClient.ts +++ b/lib/DBSQLClient.ts @@ -160,6 +160,14 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I * const session = client.connect({host, path, token}); */ public async connect(options: ConnectionOptions, authProvider?: IAuthentication): Promise { + const deprecatedClientId = (options as any).clientId; + if (deprecatedClientId !== undefined) { + this.logger.log( + LogLevel.warn, + 'Warning: The "clientId" option is deprecated. Please use "userAgentEntry" instead.', + ); + } + this.authProvider = this.createAuthProvider(options, authProvider); this.connectionProvider = this.createConnectionProvider(options); diff --git a/tests/unit/DBSQLClient.test.ts b/tests/unit/DBSQLClient.test.ts index 5caf8420..f4ac593f 100644 --- a/tests/unit/DBSQLClient.test.ts +++ b/tests/unit/DBSQLClient.test.ts @@ -79,6 +79,25 @@ describe('DBSQLClient.connect', () => { expect(thriftConnectionStub.on.called).to.be.true; }); + + it('should log a warning when deprecated clientId is passed', async () => { + const client = new DBSQLClient(); + const logSpy = sinon.spy((client as any).logger, 'log'); + + const optionsWithDeprecated = { + ...connectOptions, + clientId: 'clientId', + }; + + await client.connect(optionsWithDeprecated as any); + + const warningRegex = /Warning: The "clientId" option is deprecated\. Please use "userAgentEntry" instead\./; + const callFound = logSpy.getCalls().some((call) => warningRegex.test(call.args[1])); + + expect(callFound).to.be.true; + + logSpy.restore(); + }); }); describe('DBSQLClient.openSession', () => { From 2c908286c242cd1ee74573fb18f3487ccc9b40dd Mon Sep 17 00:00:00 2001 From: Shivam Raj Date: Mon, 10 Mar 2025 13:12:23 +0530 Subject: [PATCH 4/4] add backward compatibility --- lib/DBSQLClient.ts | 3 +++ lib/utils/buildUserAgentString.ts | 14 ++++++++++++++ tests/unit/utils/utils.test.ts | 6 ++++++ 3 files changed, 23 insertions(+) diff --git a/lib/DBSQLClient.ts b/lib/DBSQLClient.ts index cc2f82e9..85888d16 100644 --- a/lib/DBSQLClient.ts +++ b/lib/DBSQLClient.ts @@ -166,6 +166,9 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I LogLevel.warn, 'Warning: The "clientId" option is deprecated. Please use "userAgentEntry" instead.', ); + if (!options.userAgentEntry) { + options.userAgentEntry = deprecatedClientId; + } } this.authProvider = this.createAuthProvider(options, authProvider); diff --git a/lib/utils/buildUserAgentString.ts b/lib/utils/buildUserAgentString.ts index 1cbbf177..cd021e93 100644 --- a/lib/utils/buildUserAgentString.ts +++ b/lib/utils/buildUserAgentString.ts @@ -11,7 +11,21 @@ function getOperatingSystemVersion(): string { return `${os.type()} ${os.release()}`; } +function redactInternalToken(userAgentEntry: string): string { + const internalTokenPrefixes = ['dkea', 'dskea', 'dapi', 'dsapi', 'dose']; + for (const prefix of internalTokenPrefixes) { + if (userAgentEntry.startsWith(prefix)) { + return ''; + } + } + return userAgentEntry; +} + export default function buildUserAgentString(userAgentEntry?: string): string { + if (userAgentEntry) { + userAgentEntry = redactInternalToken(userAgentEntry); + } + const extra = [userAgentEntry, getNodeVersion(), getOperatingSystemVersion()].filter(Boolean); return `${productName}/${packageVersion} (${extra.join('; ')})`; } diff --git a/tests/unit/utils/utils.test.ts b/tests/unit/utils/utils.test.ts index 6f0369b2..ed96326e 100644 --- a/tests/unit/utils/utils.test.ts +++ b/tests/unit/utils/utils.test.ts @@ -56,6 +56,12 @@ describe('buildUserAgentString', () => { const ua = buildUserAgentString(); checkUserAgentString(ua); }); + + it('should redact internal token in userAgentEntry', () => { + const userAgentEntry = 'dkea-internal-token'; + const userAgentString = buildUserAgentString(userAgentEntry); + expect(userAgentString).to.include(''); + }); }); describe('formatProgress', () => {