diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a906f7..029ccb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,34 @@ +# 1.2.0 (Node.js) + +## New features + +- (Experimental) Added an option to provide a custom HTTP Agent in the client configuration via the `http_agent` option ([#283](https://github.com/ClickHouse/clickhouse-js/issues/283), related: [#278](https://github.com/ClickHouse/clickhouse-js/issues/278)). The following conditions apply if a custom HTTP Agent is provided: + - The `max_open_connections` and `tls` options will have _no effect_ and will be ignored by the client, as it is a part of the underlying HTTP Agent configuration. + - `keep_alive.enabled` will only regulate the default value of the `Connection` header (`true` -> `Connection: keep-alive`, `false` -> `Connection: close`). + - While the idle socket management will still work, it is now possible to disable it completely by setting the `keep_alive.idle_socket_ttl` value to `0`. +- (Experimental) Added a new client configuration option: `set_basic_auth_header`, which disables the `Authorization` header that is set by the client by default for every outgoing HTTP request. One of the possible scenarios when it is necessary to disable this header is when a custom HTTPS agent is used, and the server requires TLS authorization. For example: + + ```ts + const agent = new https.Agent({ + ca: fs.readFileSync('./ca.crt'), + }) + const client = createClient({ + url: 'https://server.clickhouseconnect.test:8443', + http_agent: agent, + // With a custom HTTPS agent, the client won't use the default HTTPS connection implementation; the headers should be provided manually + http_headers: { + 'X-ClickHouse-User': 'default', + 'X-ClickHouse-Key': '', + }, + // Authorization header conflicts with the TLS headers; disable it. + set_basic_auth_header: false, + }) + ``` + +NB: It is currently not possible to set the `set_basic_auth_header` option via the URL params. + +If you have feedback on these experimental features, please let us know by creating [an issue](https://github.com/ClickHouse/clickhouse-js/issues) in the repository. + # 1.1.0 (Common, Node.js, Web) ## New features diff --git a/packages/client-common/src/version.ts b/packages/client-common/src/version.ts index 9e64831..366ff6b 100644 --- a/packages/client-common/src/version.ts +++ b/packages/client-common/src/version.ts @@ -1 +1 @@ -export default '1.1.0' +export default '1.2.0' diff --git a/packages/client-node/__tests__/integration/node_custom_http_agent.test.ts b/packages/client-node/__tests__/integration/node_custom_http_agent.test.ts new file mode 100644 index 0000000..9fdd480 --- /dev/null +++ b/packages/client-node/__tests__/integration/node_custom_http_agent.test.ts @@ -0,0 +1,33 @@ +import { TestEnv, whenOnEnv } from '@test/utils' +import http from 'http' +import Http from 'http' +import { createClient } from '../../src' + +/** HTTPS agent tests are in tls.test.ts as it requires a secure connection. */ +describe('[Node.js] custom HTTP agent', () => { + let httpRequestStub: jasmine.Spy + beforeEach(() => { + httpRequestStub = spyOn(Http, 'request').and.callThrough() + }) + + // disabled with Cloud as it uses a simple HTTP agent + whenOnEnv(TestEnv.LocalSingleNode, TestEnv.LocalCluster).it( + 'should use provided http agent instead of the default one', + async () => { + const agent = new http.Agent({ + maxFreeSockets: 5, + }) + const client = createClient({ + http_agent: agent, + }) + const rs = await client.query({ + query: 'SELECT 42 AS result', + format: 'JSONEachRow', + }) + expect(await rs.json()).toEqual([{ result: 42 }]) + expect(httpRequestStub).toHaveBeenCalledTimes(1) + const callArgs = httpRequestStub.calls.mostRecent().args + expect(callArgs[1].agent).toBe(agent) + }, + ) +}) diff --git a/packages/client-node/__tests__/tls/tls.test.ts b/packages/client-node/__tests__/tls/tls.test.ts index 2b26b3d..2bd0be7 100644 --- a/packages/client-node/__tests__/tls/tls.test.ts +++ b/packages/client-node/__tests__/tls/tls.test.ts @@ -1,6 +1,8 @@ import type { ClickHouseClient } from '@clickhouse/client-common' import { createTestClient } from '@test/utils' import * as fs from 'fs' +import Http from 'http' +import https from 'node:https' import type Stream from 'stream' import { createClient } from '../../src' @@ -135,5 +137,36 @@ describe('[Node.js] TLS connection', () => { }) expect(await resultSet.text()).toEqual('0\n1\n2\n') }) + + describe('Custom HTTPS agent', () => { + let httpRequestStub: jasmine.Spy + beforeEach(() => { + httpRequestStub = spyOn(Http, 'request').and.callThrough() + }) + + it('should work with a custom HTTPS agent', async () => { + const agent = new https.Agent({ + maxFreeSockets: 5, + ca: ca_cert, + }) + const client = createClient({ + url: 'https://server.clickhouseconnect.test:8443', + http_agent: agent, + http_headers: { + 'X-ClickHouse-User': 'default', + 'X-ClickHouse-Key': '', + }, + set_basic_auth_header: false, + }) + const rs = await client.query({ + query: 'SELECT 144 AS result', + format: 'JSONEachRow', + }) + expect(await rs.json()).toEqual([{ result: 144 }]) + expect(httpRequestStub).toHaveBeenCalledTimes(1) + const callArgs = httpRequestStub.calls.mostRecent().args + expect(callArgs[1].agent).toBe(agent) + }) + }) }) }) diff --git a/packages/client-node/__tests__/unit/node_client.test.ts b/packages/client-node/__tests__/unit/node_client.test.ts index f12b0ef..de42c3f 100644 --- a/packages/client-node/__tests__/unit/node_client.test.ts +++ b/packages/client-node/__tests__/unit/node_client.test.ts @@ -4,6 +4,7 @@ import type { } from '@clickhouse/client-common' import { DefaultLogger, LogWriter } from '@clickhouse/client-common' import { createClient } from '../../src' +import type { CreateConnectionParams } from '../../src/connection' import * as c from '../../src/connection/create_connection' describe('[Node.js] createClient', () => { @@ -66,14 +67,16 @@ describe('[Node.js] createClient', () => { 'keep_alive_idle_socket_ttl=1500', ].join('&'), }) - expect(createConnectionStub).toHaveBeenCalledWith( - params, - undefined, // TLS - { + expect(createConnectionStub).toHaveBeenCalledWith({ + connection_params: params, + tls: undefined, + keep_alive: { enabled: true, idle_socket_ttl: 1500, }, - ) + set_basic_auth_header: true, + http_agent: undefined, + } satisfies CreateConnectionParams) expect(createConnectionStub).toHaveBeenCalledTimes(1) }) @@ -91,14 +94,19 @@ describe('[Node.js] createClient', () => { 'keep_alive_idle_socket_ttl=1500', ].join('&'), }) - expect(createConnectionStub).toHaveBeenCalledWith( - { ...params, url: new URL('https://my.host:8443/my_proxy') }, - undefined, // TLS - { + expect(createConnectionStub).toHaveBeenCalledWith({ + connection_params: { + ...params, + url: new URL('https://my.host:8443/my_proxy'), + }, + tls: undefined, + keep_alive: { enabled: true, idle_socket_ttl: 1500, }, - ) + set_basic_auth_header: true, + http_agent: undefined, + } satisfies CreateConnectionParams) expect(createConnectionStub).toHaveBeenCalledTimes(1) }) }) diff --git a/packages/client-node/__tests__/unit/node_config.test.ts b/packages/client-node/__tests__/unit/node_config.test.ts index 3c56fb0..d81a43a 100644 --- a/packages/client-node/__tests__/unit/node_config.test.ts +++ b/packages/client-node/__tests__/unit/node_config.test.ts @@ -5,9 +5,13 @@ import type { import { LogWriter } from '@clickhouse/client-common' import { TestLogger } from '@test/utils' import { Buffer } from 'buffer' +import http from 'http' import type { NodeClickHouseClientConfigOptions } from '../../src/config' import { NodeConfigImpl } from '../../src/config' -import type { NodeBaseConnection } from '../../src/connection' +import type { + CreateConnectionParams, + NodeBaseConnection, +} from '../../src/connection' import * as c from '../../src/connection/create_connection' describe('[Node.js] Config implementation details', () => { @@ -88,14 +92,16 @@ describe('[Node.js] Config implementation details', () => { url: new URL('http://localhost:8123'), } const res = NodeConfigImpl.make_connection(nodeConfig as any, params) - expect(createConnectionStub).toHaveBeenCalledWith( - params, - undefined, // TLS - { + expect(createConnectionStub).toHaveBeenCalledWith({ + connection_params: params, + tls: undefined, + keep_alive: { enabled: true, idle_socket_ttl: 2500, }, - ) + http_agent: undefined, + set_basic_auth_header: true, + } satisfies CreateConnectionParams) expect(createConnectionStub).toHaveBeenCalledTimes(1) expect(res).toEqual(fakeConnection) }) @@ -108,17 +114,19 @@ describe('[Node.js] Config implementation details', () => { }, } const res = NodeConfigImpl.make_connection(nodeConfig as any, params) - expect(createConnectionStub).toHaveBeenCalledWith( - params, - { + expect(createConnectionStub).toHaveBeenCalledWith({ + connection_params: params, + tls: { type: 'Basic', ca_cert: Buffer.from('my_ca_cert'), }, - { + keep_alive: { enabled: true, idle_socket_ttl: 2500, }, - ) + http_agent: undefined, + set_basic_auth_header: true, + } satisfies CreateConnectionParams) expect(createConnectionStub).toHaveBeenCalledTimes(1) expect(res).toEqual(fakeConnection) }) @@ -133,19 +141,21 @@ describe('[Node.js] Config implementation details', () => { }, } const res = NodeConfigImpl.make_connection(nodeConfig as any, params) - expect(createConnectionStub).toHaveBeenCalledWith( - params, - { + expect(createConnectionStub).toHaveBeenCalledWith({ + connection_params: params, + tls: { type: 'Mutual', ca_cert: Buffer.from('my_ca_cert'), cert: Buffer.from('my_cert'), key: Buffer.from('my_key'), }, - { + keep_alive: { enabled: true, idle_socket_ttl: 2500, }, - ) + http_agent: undefined, + set_basic_auth_header: true, + } satisfies CreateConnectionParams) expect(createConnectionStub).toHaveBeenCalledTimes(1) expect(res).toEqual(fakeConnection) }) @@ -162,17 +172,47 @@ describe('[Node.js] Config implementation details', () => { }, } const res = NodeConfigImpl.make_connection(nodeConfig as any, params) - expect(createConnectionStub).toHaveBeenCalledWith( - params, - { + expect(createConnectionStub).toHaveBeenCalledWith({ + connection_params: params, + tls: { type: 'Basic', ca_cert: Buffer.from('my_ca_cert'), }, - { + keep_alive: { enabled: false, idle_socket_ttl: 42_000, }, - ) + http_agent: undefined, + set_basic_auth_header: true, + } satisfies CreateConnectionParams) + expect(createConnectionStub).toHaveBeenCalledTimes(1) + expect(res).toEqual(fakeConnection) + }) + + it('should create a connection with a custom agent and disabled auth header', async () => { + const agent = new http.Agent({ + keepAlive: true, + maxSockets: 2, + }) + const nodeConfig: NodeClickHouseClientConfigOptions = { + url: new URL('https://localhost:8123'), + keep_alive: { + enabled: true, + }, + set_basic_auth_header: false, + http_agent: agent, + } + const res = NodeConfigImpl.make_connection(nodeConfig as any, params) + expect(createConnectionStub).toHaveBeenCalledWith({ + connection_params: params, + tls: undefined, + keep_alive: { + enabled: true, + idle_socket_ttl: 2500, + }, + http_agent: agent, + set_basic_auth_header: false, + } satisfies CreateConnectionParams) expect(createConnectionStub).toHaveBeenCalledTimes(1) expect(res).toEqual(fakeConnection) }) diff --git a/packages/client-node/__tests__/unit/node_create_connection.test.ts b/packages/client-node/__tests__/unit/node_create_connection.test.ts index 960d02c..f5cec16 100644 --- a/packages/client-node/__tests__/unit/node_create_connection.test.ts +++ b/packages/client-node/__tests__/unit/node_create_connection.test.ts @@ -1,10 +1,13 @@ import type { ConnectionParams } from '@clickhouse/client-common' +import http from 'http' +import https from 'node:https' import { createConnection, type NodeConnectionParams, NodeHttpConnection, NodeHttpsConnection, } from '../../src/connection' +import { NodeCustomAgentConnection } from '../../src/connection/node_custom_agent_connection' describe('[Node.js] createConnection', () => { const keepAliveParams: NodeConnectionParams['keep_alive'] = { @@ -16,34 +19,74 @@ describe('[Node.js] createConnection', () => { it('should create an instance of HTTP adapter', async () => { expect(adapter).toBeInstanceOf(NodeHttpConnection) }) - const adapter = createConnection( - { + const adapter = createConnection({ + connection_params: { url: new URL('http://localhost'), } as ConnectionParams, - tlsParams, - keepAliveParams, - ) + tls: tlsParams, + keep_alive: keepAliveParams, + http_agent: undefined, + set_basic_auth_header: true, + }) it('should create an instance of HTTPS adapter', async () => { - const adapter = createConnection( - { + const adapter = createConnection({ + connection_params: { url: new URL('https://localhost'), } as ConnectionParams, - tlsParams, - keepAliveParams, - ) + tls: tlsParams, + keep_alive: keepAliveParams, + http_agent: undefined, + set_basic_auth_header: true, + }) expect(adapter).toBeInstanceOf(NodeHttpsConnection) }) it('should throw if the supplied protocol is unknown', async () => { expect(() => - createConnection( - { + createConnection({ + connection_params: { url: new URL('tcp://localhost'), } as ConnectionParams, - tlsParams, - keepAliveParams, - ), + tls: tlsParams, + keep_alive: keepAliveParams, + http_agent: undefined, + set_basic_auth_header: true, + }), ).toThrowError('Only HTTP and HTTPS protocols are supported') }) + + describe('Custom HTTP agent', () => { + it('should create an instance with a custom HTTP agent', async () => { + const adapter = createConnection({ + connection_params: { + url: new URL('https://localhost'), + } as ConnectionParams, + tls: tlsParams, + keep_alive: keepAliveParams, + http_agent: new http.Agent({ + keepAlive: true, + maxSockets: 2, + }), + set_basic_auth_header: false, + }) + expect(adapter).toBeInstanceOf(NodeCustomAgentConnection) + }) + + it('should create an instance with a custom HTTPS agent', async () => { + const adapter = createConnection({ + connection_params: { + url: new URL('https://localhost'), + } as ConnectionParams, + tls: tlsParams, + keep_alive: keepAliveParams, + http_agent: new https.Agent({ + keepAlive: true, + maxSockets: 2, + }), + set_basic_auth_header: true, + }) + expect(adapter).toBeInstanceOf(NodeCustomAgentConnection) + }) + }) }) diff --git a/packages/client-node/__tests__/utils/http_stubs.ts b/packages/client-node/__tests__/utils/http_stubs.ts index 1ca9dce..e5c79a9 100644 --- a/packages/client-node/__tests__/utils/http_stubs.ts +++ b/packages/client-node/__tests__/utils/http_stubs.ts @@ -113,6 +113,7 @@ export function buildHttpConnection(config: Partial) { enabled: false, idle_socket_ttl: 2500, }, + set_basic_auth_header: true, ...config, }) } @@ -126,6 +127,7 @@ export class MyTestHttpConnection extends NodeBaseConnection { keep_alive: { enabled: false, }, + set_basic_auth_header: true, } as NodeConnectionParams, {} as Http.Agent, ) diff --git a/packages/client-node/src/config.ts b/packages/client-node/src/config.ts index bf714d7..1e4ca2a 100644 --- a/packages/client-node/src/config.ts +++ b/packages/client-node/src/config.ts @@ -7,6 +7,8 @@ import { type ConnectionParams, numberConfigURLValue, } from '@clickhouse/client-common' +import type http from 'http' +import type https from 'node:https' import type Stream from 'stream' import { createConnection, type TLSParams } from './connection' import { ResultSet } from './result_set' @@ -22,10 +24,21 @@ export type NodeClickHouseClientConfigOptions = enabled?: boolean /** For how long keep a particular idle socket alive on the client side (in milliseconds). * It is supposed to be a fair bit less that the ClickHouse server KeepAlive timeout, - * which is by default 3000 ms for pre-23.11 versions. + * which is by default 3000 ms for pre-23.11 versions.
+ * When set to `0`, the idle socket management feature is disabled. * @default 2500 */ idle_socket_ttl?: number } + /** Custom HTTP agent to use for the outgoing HTTP(s) requests. + * If set, {@link BaseClickHouseClientConfigOptions.max_open_connections}, {@link tls} and {@link keep_alive} + * options have no effect, as it is part of the default underlying agent configuration. + * @experimental - unstable API, might be a subject to change in the future; please provide your feedback in the repository. + * @default undefined */ + http_agent?: http.Agent | https.Agent + /** Enable or disable the `Authorization` header with basic auth for the outgoing HTTP(s) requests. + * @experimental - unstable API, might be a subject to change in the future; please provide your feedback in the repository. + * @default true (enabled) */ + set_basic_auth_header?: boolean } interface BasicTLSOptions { @@ -95,7 +108,13 @@ export const NodeConfigImpl: Required< enabled: nodeConfig?.keep_alive?.enabled ?? true, idle_socket_ttl: nodeConfig?.keep_alive?.idle_socket_ttl ?? 2500, } - return createConnection(params, tls, keep_alive) + return createConnection({ + connection_params: params, + set_basic_auth_header: nodeConfig.set_basic_auth_header ?? true, + http_agent: nodeConfig.http_agent, + keep_alive, + tls, + }) }, values_encoder: new NodeValuesEncoder(), make_result_set: (( diff --git a/packages/client-node/src/connection/create_connection.ts b/packages/client-node/src/connection/create_connection.ts index f10f77a..cf0629e 100644 --- a/packages/client-node/src/connection/create_connection.ts +++ b/packages/client-node/src/connection/create_connection.ts @@ -1,21 +1,51 @@ import type { ConnectionParams } from '@clickhouse/client-common' +import type http from 'http' +import type https from 'node:https' import type { NodeBaseConnection, NodeConnectionParams, } from './node_base_connection' +import { NodeCustomAgentConnection } from './node_custom_agent_connection' import { NodeHttpConnection } from './node_http_connection' import { NodeHttpsConnection } from './node_https_connection' -export function createConnection( - params: ConnectionParams, - tls: NodeConnectionParams['tls'], - keep_alive: NodeConnectionParams['keep_alive'], -): NodeBaseConnection { - switch (params.url.protocol) { +export interface CreateConnectionParams { + connection_params: ConnectionParams + tls: NodeConnectionParams['tls'] + keep_alive: NodeConnectionParams['keep_alive'] + http_agent: http.Agent | https.Agent | undefined + set_basic_auth_header: boolean +} + +export function createConnection({ + connection_params, + tls, + keep_alive, + http_agent, + set_basic_auth_header, +}: CreateConnectionParams): NodeBaseConnection { + if (http_agent !== undefined) { + return new NodeCustomAgentConnection({ + ...connection_params, + set_basic_auth_header, + keep_alive, // only used to enforce proper KeepAlive headers + http_agent, + }) + } + switch (connection_params.url.protocol) { case 'http:': - return new NodeHttpConnection({ ...params, keep_alive }) + return new NodeHttpConnection({ + ...connection_params, + set_basic_auth_header, + keep_alive, + }) case 'https:': - return new NodeHttpsConnection({ ...params, tls, keep_alive }) + return new NodeHttpsConnection({ + ...connection_params, + set_basic_auth_header, + keep_alive, + tls, + }) default: throw new Error('Only HTTP and HTTPS protocols are supported') } diff --git a/packages/client-node/src/connection/node_base_connection.ts b/packages/client-node/src/connection/node_base_connection.ts index 5df78df..9a9c9f1 100644 --- a/packages/client-node/src/connection/node_base_connection.ts +++ b/packages/client-node/src/connection/node_base_connection.ts @@ -23,6 +23,7 @@ import { import crypto from 'crypto' import type Http from 'http' import type * as net from 'net' +import type Https from 'node:https' import Stream from 'stream' import type { URLSearchParams } from 'url' import Zlib from 'zlib' @@ -32,6 +33,8 @@ import { drainStream } from './stream' export type NodeConnectionParams = ConnectionParams & { tls?: TLSParams + http_agent?: Http.Agent | Https.Agent + set_basic_auth_header: boolean keep_alive: { enabled: boolean idle_socket_ttl: number @@ -247,12 +250,18 @@ export abstract class NodeBaseConnection protected buildRequestHeaders( params?: BaseQueryParams, ): Http.OutgoingHttpHeaders { - return { - ...this.defaultHeaders, - Authorization: - params?.auth !== undefined - ? `Basic ${Buffer.from(`${params.auth.username}:${params.auth.password}`).toString('base64')}` - : this.defaultAuthHeader, + if (this.params.set_basic_auth_header) { + return { + ...this.defaultHeaders, + Authorization: + params?.auth !== undefined + ? `Basic ${Buffer.from(`${params.auth.username}:${params.auth.password}`).toString('base64')}` + : this.defaultAuthHeader, + } + } else { + return { + ...this.defaultHeaders, + } } } @@ -479,7 +488,10 @@ export abstract class NodeBaseConnection } const onSocket = (socket: net.Socket) => { - if (this.params.keep_alive.enabled) { + if ( + this.params.keep_alive.enabled && + this.params.keep_alive.idle_socket_ttl > 0 + ) { const socketInfo = this.knownSockets.get(socket) // It is the first time we encounter this socket, // so it doesn't have the idle timeout handler attached to it diff --git a/packages/client-node/src/connection/node_custom_agent_connection.ts b/packages/client-node/src/connection/node_custom_agent_connection.ts new file mode 100644 index 0000000..0684c32 --- /dev/null +++ b/packages/client-node/src/connection/node_custom_agent_connection.ts @@ -0,0 +1,33 @@ +import { withCompressionHeaders } from '@clickhouse/client-common' +import Http from 'http' +import type { + NodeConnectionParams, + RequestParams, +} from './node_base_connection' +import { NodeBaseConnection } from './node_base_connection' + +export class NodeCustomAgentConnection extends NodeBaseConnection { + constructor(params: NodeConnectionParams) { + if (!params.http_agent) { + throw new Error( + 'http_agent is required to create NodeCustomAgentConnection', + ) + } + super(params, params.http_agent) + } + + protected createClientRequest(params: RequestParams): Http.ClientRequest { + const headers = withCompressionHeaders({ + headers: params.headers, + compress_request: params.compress_request, + decompress_response: params.decompress_response, + }) + return Http.request(params.url, { + method: params.method, + agent: this.agent, + timeout: this.params.request_timeout, + signal: params.abort_signal, + headers, + }) + } +} diff --git a/packages/client-node/src/version.ts b/packages/client-node/src/version.ts index 9e64831..366ff6b 100644 --- a/packages/client-node/src/version.ts +++ b/packages/client-node/src/version.ts @@ -1 +1 @@ -export default '1.1.0' +export default '1.2.0' diff --git a/packages/client-web/src/version.ts b/packages/client-web/src/version.ts index 9e64831..366ff6b 100644 --- a/packages/client-web/src/version.ts +++ b/packages/client-web/src/version.ts @@ -1 +1 @@ -export default '1.1.0' +export default '1.2.0'