chproxy icon indicating copy to clipboard operation
chproxy copied to clipboard

[BUG] Incompatibility with ClickHouse API

Open yudelevi opened this issue 1 year ago • 8 comments

Describe the bug There are two issues here:

  1. Introduced in #456, the /ping endpoint depends on authentication. The original endpoint in ClickHouse protocol doesn't require authentication, and the official client doesn't provide it. As a result, /ping fails. To Reproduce On the CH machine directly (user foo doesn't exist)
[dyudelevich@ch-01 ~]$ curl http://foo:[email protected]:8123/ping
Ok.

On chproxy:

root@chproxy-dev-6f4d78665b-k2vml:/# curl http://foo:[email protected]:9001/ping
"127.0.0.1:50460": invalid username or password for user "foo"; query: ""

A workaround for now is to add default/"" user to chproxy.

  1. Headers are missing when using the query by default "Transfer-Encoding: chunked" is supplied, but it's not being sent via chproxy To Reproduce Issue 2: on the CH machine directly:
*   Trying 127.0.0.1:8123...
* Connected to 127.0.0.1 (127.0.0.1) port 8123 (#0)
* Server auth using Basic with user 'foo'
> GET /ping HTTP/1.1
> Host: 127.0.0.1:8123
> Authorization: Basic Zm9vOmJhcg==
> User-Agent: curl/7.76.1
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Date: Tue, 01 Oct 2024 02:16:31 GMT
< Connection: Keep-Alive
< Content-Type: text/html; charset=UTF-8
< Transfer-Encoding: chunked
< Keep-Alive: timeout=3, max=9999
< X-ClickHouse-Summary: {"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0","result_rows":"0","result_bytes":"0","elapsed_ns":"33303"}
<
Ok.
* Connection #0 to host 127.0.0.1 left intact 

on chproxy:

root@chproxy-dev-6f4d78665b-k2vml:/# curl -v http://127.0.01:9001/ping
*   Trying 127.0.0.1:9001...
* Connected to 127.0.0.1 (127.0.0.1) port 9001 (#0)
> GET /ping HTTP/1.1
> Host: 127.0.0.1:9001
> User-Agent: curl/7.88.1
> Accept: */*
> 
< HTTP/1.1 200 OK
< Content-Type: text/html; charset=UTF-8
< Date: Tue, 01 Oct 2024 02:18:51 GMT
< X-Clickhouse-Summary: {"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0","result_rows":"0","result_bytes":"0","elapsed_ns":"56796"}
< Content-Length: 4
< 
Ok.

This causes the following error on clickhouse-connect client

  File “/usr/local/lib/python3.11/site-packages/clickhouse_connect/driver/buffer.py”, line 65, in read_leb128
    b = self.read_byte()
        ^^^^^^^^^^^^^^^^
  File “/usr/local/lib/python3.11/site-packages/clickhouse_connect/driver/buffer.py”, line 51, in read_byte
    chunk = next(self.gen, None)
            ^^^^^^^^^^^^^^^^^^^^
  File “/usr/local/lib/python3.11/site-packages/clickhouse_connect/driver/httputil.py”, line 231, in buffered
    chunk = next(read_gen, None) # Always try to read at least one chunk if there are any left
            ^^^^^^^^^^^^^^^^^^^^
  File “/usr/local/lib/python3.11/site-packages/urllib3/response.py”, line 1179, in read_chunked
    raise ResponseNotChunked(
urllib3.exceptions.ResponseNotChunked: Response is not chunked. Header ‘transfer-encoding: chunked’ is missing.

Environment information Linux, chproxy 1.26.5, clickhouse_connect==0.8.1

yudelevi avatar Oct 01 '24 02:10 yudelevi

same here

yalattas avatar Oct 15 '24 14:10 yalattas

addr = parseForwardedHeader(fwd)

here I don't see this being called at all if we set:

proxy:
    enable: true
    header: Transfer-Encoding

in proxy configuration because it will always execute line: 61-62 and then jump to: 70

ibakhsh4salla avatar Oct 16 '24 13:10 ibakhsh4salla

@ibakhsh4salla I'm looking at the code you mentioned, looks like it's a function to set up the forwarded IP header. I don't believe that transfer encoding should be set there.

yudelevi avatar Oct 17 '24 17:10 yudelevi

clickhouse-connect 0.8.6 (https://github.com/ClickHouse/clickhouse-connect/pull/421) has fixed the urllib3.exceptions.ResponseNotChunked exception (https://github.com/ClickHouse/clickhouse-connect/issues/417).

lqhl avatar Nov 04 '24 06:11 lqhl

Can confirm that second part of this is resolved by clickhouse-connect 0.8.6

yudelevi avatar Nov 14 '24 04:11 yudelevi

So if I sum up, only the /ping is an issue but there is a dirty workaround. Unfortunately, the maintainers are quite busy. By any chance, could you have a look @hulk8, since it's linked to the feature you've released a few months ago?

mga-chka avatar Feb 04 '25 14:02 mga-chka

Yes. I will fix that.

hulk8 avatar Feb 05 '25 13:02 hulk8

Hello. Any updates here ?

AKuzyashin avatar Mar 17 '25 09:03 AKuzyashin