Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Incompatibility with ClickHouse API #476

Open
yudelevi opened this issue Oct 1, 2024 · 4 comments
Open

[BUG] Incompatibility with ClickHouse API #476

yudelevi opened this issue Oct 1, 2024 · 4 comments

Comments

@yudelevi
Copy link

yudelevi commented Oct 1, 2024

Describe the bug
There are two issues here:

  1. Introduced in Support ping endpoint #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:bar@127.0.0.1:8123/ping
Ok.

On chproxy:

root@chproxy-dev-6f4d78665b-k2vml:/# curl http://foo:bar@127.0.01: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

@yalattas
Copy link

same here

@ibakhsh4salla
Copy link

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

@yudelevi
Copy link
Author

@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.

@lqhl
Copy link

lqhl commented Nov 4, 2024

clickhouse-connect 0.8.6 (ClickHouse/clickhouse-connect#421) has fixed the urllib3.exceptions.ResponseNotChunked exception (ClickHouse/clickhouse-connect#417).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants