From 5cec293cc85d78195a98928a581681d488afa7cb Mon Sep 17 00:00:00 2001 From: Vikrant Puppala Date: Thu, 27 Mar 2025 09:52:59 +0530 Subject: [PATCH 1/6] improve url handling Signed-off-by: Vikrant Puppala --- lib/connection/connections/HttpConnection.ts | 2 +- .../connections/HttpConnection.test.ts | 44 ++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/lib/connection/connections/HttpConnection.ts b/lib/connection/connections/HttpConnection.ts index dc9c3902..2cafb925 100644 --- a/lib/connection/connections/HttpConnection.ts +++ b/lib/connection/connections/HttpConnection.ts @@ -98,7 +98,7 @@ export default class HttpConnection implements IConnectionProvider { this.connection = new ThriftHttpConnection( { - url: `${options.https ? 'https' : 'http'}://${options.host}:${options.port}${options.path ?? '/'}`, + url: `${options.https ? 'https' : 'http'}://${options.host.replace(/\/$/, '')}:${options.port}${options.path ? ('/' + options.path.replace(/\/$/, '')) : '/'}`, transport: thrift.TBufferedTransport, protocol: thrift.TBinaryProtocol, getRetryPolicy: () => this.getRetryPolicy(), diff --git a/tests/unit/connection/connections/HttpConnection.test.ts b/tests/unit/connection/connections/HttpConnection.test.ts index c7b39972..baccc298 100644 --- a/tests/unit/connection/connections/HttpConnection.test.ts +++ b/tests/unit/connection/connections/HttpConnection.test.ts @@ -2,7 +2,7 @@ import http from 'http'; import { expect } from 'chai'; import HttpConnection from '../../../../lib/connection/connections/HttpConnection'; import ThriftHttpConnection from '../../../../lib/connection/connections/ThriftHttpConnection'; - +import IConnectionOptions from '../../../../lib/connection/contracts/IConnectionOptions'; import ClientContextStub from '../../.stubs/ClientContextStub'; describe('HttpConnection.connect', () => { @@ -127,4 +127,46 @@ describe('HttpConnection.connect', () => { ...extraHeaders, }); }); + + it('should handle trailing slashes in host and path correctly', async () => { + interface TestCase { + input: { + host: string; + path?: string; + }; + expected: string; + } + + const testCases: TestCase[] = [ + { + input: { host: 'xyz.com/', path: '/sql/v1/' }, + expected: 'https://xyz.com:443/sql/v1' + }, + { + input: { host: 'xyz.com', path: 'sql/v1' }, + expected: 'https://xyz.com:443/sql/v1' + }, + { + input: { host: 'xyz.com/', path: 'sql/v1' }, + expected: 'https://xyz.com:443/sql/v1' + }, + { + input: { host: 'xyz.com', path: undefined }, + expected: 'https://xyz.com:443/' + } + ]; + + for (const testCase of testCases) { + const options: IConnectionOptions = { + host: testCase.input.host, + port: 443, + path: testCase.input.path, + https: true, + }; + + const connection = new HttpConnection(options, new ClientContextStub()); + const thriftConnection = await connection.getThriftConnection(); + expect(thriftConnection.url).to.be.equal(testCase.expected); + } + }); }); From 85249caeb330569042ee688e14a91d640bf5bafe Mon Sep 17 00:00:00 2001 From: Vikrant Puppala Date: Thu, 27 Mar 2025 10:06:36 +0530 Subject: [PATCH 2/6] improve url handling Signed-off-by: Vikrant Puppala --- lib/connection/connections/HttpConnection.ts | 2 +- .../connection/connections/HttpConnection.test.ts | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/connection/connections/HttpConnection.ts b/lib/connection/connections/HttpConnection.ts index 2cafb925..f163a540 100644 --- a/lib/connection/connections/HttpConnection.ts +++ b/lib/connection/connections/HttpConnection.ts @@ -98,7 +98,7 @@ export default class HttpConnection implements IConnectionProvider { this.connection = new ThriftHttpConnection( { - url: `${options.https ? 'https' : 'http'}://${options.host.replace(/\/$/, '')}:${options.port}${options.path ? ('/' + options.path.replace(/\/$/, '')) : '/'}`, + url: `${options.https ? 'https' : 'http'}://${options.host.replace(/\/$/, '')}:${options.port}${options.path ?? '/'}`, transport: thrift.TBufferedTransport, protocol: thrift.TBinaryProtocol, getRetryPolicy: () => this.getRetryPolicy(), diff --git a/tests/unit/connection/connections/HttpConnection.test.ts b/tests/unit/connection/connections/HttpConnection.test.ts index baccc298..f08c3fd5 100644 --- a/tests/unit/connection/connections/HttpConnection.test.ts +++ b/tests/unit/connection/connections/HttpConnection.test.ts @@ -128,7 +128,7 @@ describe('HttpConnection.connect', () => { }); }); - it('should handle trailing slashes in host and path correctly', async () => { + it('should handle trailing slashes in host correctly', async () => { interface TestCase { input: { host: string; @@ -139,19 +139,15 @@ describe('HttpConnection.connect', () => { const testCases: TestCase[] = [ { - input: { host: 'xyz.com/', path: '/sql/v1/' }, + input: { host: 'xyz.com/', path: '/sql/v1' }, expected: 'https://xyz.com:443/sql/v1' }, { - input: { host: 'xyz.com', path: 'sql/v1' }, + input: { host: 'xyz.com', path: '/sql/v1' }, expected: 'https://xyz.com:443/sql/v1' }, { - input: { host: 'xyz.com/', path: 'sql/v1' }, - expected: 'https://xyz.com:443/sql/v1' - }, - { - input: { host: 'xyz.com', path: undefined }, + input: { host: 'xyz.com/', path: undefined }, expected: 'https://xyz.com:443/' } ]; From 08988eb34b4eb5c46a0ae8698a234e2538b1908f Mon Sep 17 00:00:00 2001 From: Vikrant Puppala Date: Thu, 27 Mar 2025 10:09:52 +0530 Subject: [PATCH 3/6] lint Signed-off-by: Vikrant Puppala --- lib/connection/connections/HttpConnection.ts | 4 +++- tests/unit/connection/connections/HttpConnection.test.ts | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/connection/connections/HttpConnection.ts b/lib/connection/connections/HttpConnection.ts index f163a540..a3f93f41 100644 --- a/lib/connection/connections/HttpConnection.ts +++ b/lib/connection/connections/HttpConnection.ts @@ -98,7 +98,9 @@ export default class HttpConnection implements IConnectionProvider { this.connection = new ThriftHttpConnection( { - url: `${options.https ? 'https' : 'http'}://${options.host.replace(/\/$/, '')}:${options.port}${options.path ?? '/'}`, + url: `${options.https ? 'https' : 'http'}://${options.host.replace(/\/$/, '')}:${options.port}${ + options.path ?? '/' + }`, transport: thrift.TBufferedTransport, protocol: thrift.TBinaryProtocol, getRetryPolicy: () => this.getRetryPolicy(), diff --git a/tests/unit/connection/connections/HttpConnection.test.ts b/tests/unit/connection/connections/HttpConnection.test.ts index f08c3fd5..3731417a 100644 --- a/tests/unit/connection/connections/HttpConnection.test.ts +++ b/tests/unit/connection/connections/HttpConnection.test.ts @@ -140,16 +140,16 @@ describe('HttpConnection.connect', () => { const testCases: TestCase[] = [ { input: { host: 'xyz.com/', path: '/sql/v1' }, - expected: 'https://xyz.com:443/sql/v1' + expected: 'https://xyz.com:443/sql/v1', }, { input: { host: 'xyz.com', path: '/sql/v1' }, - expected: 'https://xyz.com:443/sql/v1' + expected: 'https://xyz.com:443/sql/v1', }, { input: { host: 'xyz.com/', path: undefined }, - expected: 'https://xyz.com:443/' - } + expected: 'https://xyz.com:443/', + }, ]; for (const testCase of testCases) { From e9549dd0f24af895a11d4c7c1e2bca78f5cb861f Mon Sep 17 00:00:00 2001 From: Vikrant Puppala Date: Thu, 27 Mar 2025 12:31:34 +0530 Subject: [PATCH 4/6] add more tests Signed-off-by: Vikrant Puppala --- .../connections/HttpConnection.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/unit/connection/connections/HttpConnection.test.ts b/tests/unit/connection/connections/HttpConnection.test.ts index 3731417a..d1b39745 100644 --- a/tests/unit/connection/connections/HttpConnection.test.ts +++ b/tests/unit/connection/connections/HttpConnection.test.ts @@ -150,6 +150,22 @@ describe('HttpConnection.connect', () => { input: { host: 'xyz.com/', path: undefined }, expected: 'https://xyz.com:443/', }, + { + input: { host: 'xyz.com', path: 'sql/v1' }, + expected: 'https://xyz.com:443/sql/v1', + }, + { + input: { host: 'xyz.com/', path: 'sql/v1' }, + expected: 'https://xyz.com:443/sql/v1', + }, + { + input: { host: 'xyz.com', path: 'sql/v1/' }, + expected: 'https://xyz.com:443/sql/v1', + }, + { + input: { host: 'https://xyz.com', path: 'sql/v1' }, + expected: 'https://xyz.com:443/sql/v1', + }, ]; for (const testCase of testCases) { From ea982027b1cdbbed697a9e2e4c7ece557023a600 Mon Sep 17 00:00:00 2001 From: Vikrant Puppala Date: Thu, 27 Mar 2025 12:35:17 +0530 Subject: [PATCH 5/6] fix Signed-off-by: Vikrant Puppala --- lib/connection/connections/HttpConnection.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/connection/connections/HttpConnection.ts b/lib/connection/connections/HttpConnection.ts index a3f93f41..6a6cce2c 100644 --- a/lib/connection/connections/HttpConnection.ts +++ b/lib/connection/connections/HttpConnection.ts @@ -98,9 +98,7 @@ export default class HttpConnection implements IConnectionProvider { this.connection = new ThriftHttpConnection( { - url: `${options.https ? 'https' : 'http'}://${options.host.replace(/\/$/, '')}:${options.port}${ - options.path ?? '/' - }`, + url: `${options.https ? 'https' : 'http'}://${options.host.replace(/\/$/, '')}:${options.port}${options.path ? (options.path.startsWith('/') ? '' : '/') + options.path.replace(/\/$/, '') : '/'}`, transport: thrift.TBufferedTransport, protocol: thrift.TBinaryProtocol, getRetryPolicy: () => this.getRetryPolicy(), From c69fa04f2f3b521f27d71f8b2efebda995378874 Mon Sep 17 00:00:00 2001 From: Vikrant Puppala Date: Thu, 27 Mar 2025 13:09:07 +0530 Subject: [PATCH 6/6] fix Signed-off-by: Vikrant Puppala --- lib/connection/connections/HttpConnection.ts | 4 +++- tests/unit/connection/connections/HttpConnection.test.ts | 4 ---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/connection/connections/HttpConnection.ts b/lib/connection/connections/HttpConnection.ts index 6a6cce2c..8c56019c 100644 --- a/lib/connection/connections/HttpConnection.ts +++ b/lib/connection/connections/HttpConnection.ts @@ -98,7 +98,9 @@ export default class HttpConnection implements IConnectionProvider { this.connection = new ThriftHttpConnection( { - url: `${options.https ? 'https' : 'http'}://${options.host.replace(/\/$/, '')}:${options.port}${options.path ? (options.path.startsWith('/') ? '' : '/') + options.path.replace(/\/$/, '') : '/'}`, + url: `${options.https ? 'https' : 'http'}://${options.host.replace(/\/$/, '')}:${options.port}${ + options.path ? (options.path.startsWith('/') ? '' : '/') + options.path.replace(/\/$/, '') : '/' + }`, transport: thrift.TBufferedTransport, protocol: thrift.TBinaryProtocol, getRetryPolicy: () => this.getRetryPolicy(), diff --git a/tests/unit/connection/connections/HttpConnection.test.ts b/tests/unit/connection/connections/HttpConnection.test.ts index d1b39745..44d38a69 100644 --- a/tests/unit/connection/connections/HttpConnection.test.ts +++ b/tests/unit/connection/connections/HttpConnection.test.ts @@ -162,10 +162,6 @@ describe('HttpConnection.connect', () => { input: { host: 'xyz.com', path: 'sql/v1/' }, expected: 'https://xyz.com:443/sql/v1', }, - { - input: { host: 'https://xyz.com', path: 'sql/v1' }, - expected: 'https://xyz.com:443/sql/v1', - }, ]; for (const testCase of testCases) {