Not working if the http interface is reverse-proxied by nginx with a suffix

[ngugcx]

Describe the bug

The http interface of my clickhouse server is reverse-proxied using nignx with suffix.
http://localhost:8123 is mapped to https://my.domain/clickhouse

This error is prompted when client.ping():
node:internal/process/promises:288
            triggerUncaughtException(err, true /* fromPromise */);
            ^

Error: Client network socket disconnected before secure TLS connection was established
    at connResetException (node:internal/errors:717:14)
    at TLSSocket.onConnectEnd (node:_tls_wrap:1595:19)
    at TLSSocket.emit (node:events:525:35)
    at endReadableNT (node:internal/streams/readable:1359:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'ECONNRESET',
  path: null,
  host: 'my.domain',
  port: 443,
  localAddress: undefined
}

It's working when http://localhost:8123 is mapped to https://my.domain/ without a suffix.
And when accessing https://my.domain/clickhouse using curl, all works well too.

Steps to reproduce

1. Setup reverse proxy using nignx, map http://localhost:8123 to https://my.domain/clickhouse
2. execute ping()

Expected behaviour

ping() should return true.

Code example

const client = createClient({
  host: process.env.CLICKHOUSE_HOST ?? 'https://my.domain/clickhouse',
  username: process.env.CLICKHOUSE_USER ?? 'username',
  password: process.env.CLICKHOUSE_PASSWORD ?? 'password'
})

client.ping()

Environment

• Client version: 0.0.16
• Language version: node v18.16.0
• OS: Windows 10

ClickHouse server

• ClickHouse Server version: 23

[slvrtrn]

I think it is because the HTTP connection is chosen instead of HTTPS: https://github.com/ClickHouse/clickhouse-js/blob/main/src/connection/connection.ts#L74-L81

if it was https://localhost:8123 then it should've been fine.

[martoche]

Hi, same problem here.

Unfortunately, when you call ClickHouseClient.query, you can't pass a pathname params down to the request function which is ultimately called.

See:
https://github.com/ClickHouse/clickhouse-js/blob/main/packages/client-common/src/client.ts#L188
https://github.com/ClickHouse/clickhouse-js/blob/main/packages/client-web/src/connection/web_connection.ts#L131

The only solution I've found (besides forking the repo) is to monkey patch the request function:

const clickhouseClient = createClient({})

// monkey-patch clickhouseClient.connection.request to force the "pathname" value
let request = clickhouseClient.connection.request
clickhouseClient.connection.request = function (arg0) {
  arg0.pathname = "/clickhouse"
  return request.apply(this, [arg0])
}

// call clickhouseClient.query

A solution would be to allow passing the pathname param in the ClickHouseClient.query function.

[slvrtrn]

@martoche, you are referring to an issue with the web version; is it correct?
I think we could add an optional pathname (or similar) to the common client config itself.

For example:

const client = createClient({
  pathname: '/clickhouse' // defaults to '/'
})

// ### Node.js
// ping -> /clickhouse/ping
// other -> /clickhouse

// ### Web (no specific ping call, it just uses SELECT 1)
// all requests -> /clickhouse

[martoche]

@slvrtrn Correct, I'm using the web version.

The solution you suggest would be perfect! 😍

[slvrtrn]

@martoche, as it turned out, there is an even easier way to do it, which will allow to just use the existing host config like this:

const client = createClient({
  host: 'http://proxy:8123/clickhouse' 
})

See #207

[slvrtrn]

@martoche, 0.2.5 is out with the mentioned changes ^

[martoche]

Works great, thank you very much!
(issue can be closed)

[slvrtrn]

@ngugcx, please feel free to re-open if the issue in your scenario was not in HTTP vs HTTPS as described here.

[slvrtrn]

@martoche, if you will upgrade to 1.0.0, please use separate pathname config option.