[client-v2] Error body without X-ClickHouse-Exception-Code is cut short: only the first chunk is read, and short bodies are lost with response compression

未关闭
#3,338 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

还没有人认领这个 Issue。

评估

难度
3/5
预计耗时
1-2 天
新手友好度
72/100
Issue 类型
缺陷
描述清晰度
描述清楚
活跃度
活跃
技术栈
java

调研方向

Start with HttpAPIClientHelper.readNotClickHouseError and the related CompressedBlockInputStream.readFully behavior described in the issue. Reproduce the short-body and multi-chunk cases with compress=true and compress=false, then inspect tests around error response handling and the existing PR #3322 test. Done when error messages include the full plain-text body up to 1024 bytes while successful responses and errors with an exception-code header retain their existing behavior.

由索引模型根据 Issue 内容生成。

描述

area:error-handling bug client-api-v2

Description

HttpAPIClientHelper.readNotClickHouseError makes the ServerException message for an error response (400, 401, 403, 404, 500) that has no X-ClickHouse-Exception-Code header. Examples: a proxy or load-balancer error page, or the ClickHouse server's own There is no handle /... 404. The method does not read the full body. On main (f9586aba0) there are three defects:

  1. Only one read() on the raw stream. InputStream.read() can return fewer bytes than are available. Apache HttpCore 5 ChunkedInputStream returns at most the rest of the current chunk. So a body sent with Transfer-Encoding: chunked in more than one chunk is cut at the end of the first chunk, also when all bytes arrive in one TCP write. This occurs with compress=true and with compress=false.
  2. With compress=true (the default), the last 25 bytes of the read are lost. After the "Invalid LZ4 magic byte" fallback, the 25 header bytes are in buffer[0..25) and the next read writes to buffer[25..]. But the message is new String(buffer, 0, msgLen), which ignores offset. A body of exactly 25 bytes gives unknown server error.
  3. With compress=true, a body shorter than 25 bytes is lost. CompressedBlockInputStream.readFully throws IOException("Incomplete read: 18 of 25"), not a ClientException. The generic catch logs a WARN with a stack trace, and the message is unknown server error. Example: the 18-byte body 404 page not found.

Open PR #3322 fixes defect 2: I ran the same reproduction on its head (a7f13225e). Defects 1 and 3 still occur on that head. Note: #3322 adds testTruncatedCompressedErrorBodyStaysUnreadable, which intentionally keeps a body shorter than 25 bytes "unreadable" for a response with an exception code. A response without the header comes from a proxy or a non-query handler, so its body is plain text, not a truncated compressed block.

Steps to reproduce
  1. Point client-v2 at an HTTP endpoint that returns 404 for POST / with a plain-text body and no X-ClickHouse-Exception-Code header. A small socket server or WireMock is sufficient (see the code example). For defect 2, a real ClickHouse server with a wrong endpoint path (http://host:8123/nope) also works.
  2. Call client.query("SELECT 1").get().
  3. Compare ServerException.getMessage() with the body that was sent.
Error Log or Exception StackTrace

Results on main f9586aba0:

Body sent compress=true (default) compress=false
404 page not found (18 B) unknown server error (transport error: 404) + WARN below full body
25 B unknown server error full body
26 B a (1 char) full body
60 B first 35 chars full body
nginx 404 HTML page (144 B) ends at <hr><center>nginx</ full body
129 B in 3 chunks, one TCP write upstream connect e (18 chars) first chunk only (43 chars)
ClickHouse 26.9.1.1629 404 There is no handle /nope last 25 bytes lost: the last line is clickhouse instead of clickhouse-client < query > result full body

On PR #3322 head a7f13225e: rows 2-5 and 7 are correct. Row 1 is still unknown server error. Row 6 is still the first chunk only, in both modes.

WARN for row 1:

WARN HttpAPIClientHelper - Failed to read error message (queryId = )
java.io.IOException: Incomplete read: 18 of 25
	at com.clickhouse.client.api.internal.CompressedBlockInputStream.readFully(CompressedBlockInputStream.java:89)
	at com.clickhouse.client.api.internal.CompressedBlockInputStream.refill(CompressedBlockInputStream.java:108)
	at com.clickhouse.client.api.internal.CompressedBlockInputStream.read(CompressedBlockInputStream.java:63)
	at com.clickhouse.client.api.internal.HttpAPIClientHelper.readNotClickHouseError(HttpAPIClientHelper.java:459)
	at com.clickhouse.client.api.internal.HttpAPIClientHelper.readError(HttpAPIClientHelper.java:439)
	at com.clickhouse.client.api.internal.HttpAPIClientHelper.doExecuteRequest(HttpAPIClientHelper.java:868)
Expected Behaviour

The ServerException message contains the full body text, up to ERROR_BODY_BUFFER_SIZE (1024 bytes). This is what compress=false gives today for a body that arrives in one chunk.

Code Example

Chunked case (row 6). The server writes the whole response in one write(), so the result does not depend on timing:

String[] chunks = {
        "upstream connect error or disconnect/reset ",
        "before headers. reset reason: connection ",
        "failure, transport failure reason: marker-END"};
StringBuilder rsp = new StringBuilder("HTTP/1.1 404 Not Found\r\nContent-Type: text/plain\r\n"
        + "Transfer-Encoding: chunked\r\nConnection: close\r\n\r\n");
for (String c : chunks) {
    rsp.append(Integer.toHexString(c.length())).append("\r\n").append(c).append("\r\n");
}
rsp.append("0\r\n\r\n");

ServerSocket ss = new ServerSocket(0);
new Thread(() -> {
    try (Socket s = ss.accept()) {
        // read the request headers up to "\r\n\r\n" (omitted), then:
        s.getOutputStream().write(rsp.toString().getBytes(StandardCharsets.UTF_8));
        s.shutdownOutput();
        // drain the request body (omitted)
    } catch (Exception ignore) { }
}).start();

try (Client client = new Client.Builder()
        .addEndpoint(Protocol.HTTP, "localhost", ss.getLocalPort(), false)
        .setUsername("default").setPassword("x")
        .build()) {                         // add .compressServerResponse(false) for the contrast column
    client.query("SELECT 1").get(10, TimeUnit.SECONDS);
} catch (ServerException e) {
    System.out.println(e.getMessage());     // "upstream connect e (transport error: 404)"
}

Rows 1-5 use the same client with a WireMock stub: aResponse().withStatus(404).withBody(<body>), no X-ClickHouse-Exception-Code header.

Root cause
  • HttpAPIClientHelper.java:459: one body.read(buffer, offset, buffer.length - offset). There is no loop until EOF or until the buffer is full (defect 1).
  • HttpAPIClientHelper.java:461: new String(buffer, 0, msgLen, ...) ignores offset, which is set to 25 at line 473 (defect 2).
  • CompressedBlockInputStream.java:89: readFully throws IOException when the body ends before 25 bytes. The fallback at HttpAPIClientHelper.java:467 catches only ClientException, so lines 478-480 discard the bytes that were already read (defect 3).
Suggested fix
  • Read in a loop until EOF or until the buffer is full, and build the message from offset + total bytes read.
  • For defect 3, two possible approaches:
    • wrapResponseEntity does not put CompressedBlockEntity around an error response without X-ClickHouse-Exception-Code. It already skips the wrapper for 401/403. Then the header fallback is not necessary on this path.
    • Or CompressedBlockInputStream records how many header bytes it read before the stream ended, and readNotClickHouseError uses those bytes as plain text.
  • Keep the current behavior for successful responses and for error responses that have the exception-code header: they still go through CompressedBlockInputStream.
Configuration
Client Configuration
new Client.Builder()
        .addEndpoint(Protocol.HTTP, "localhost", port, false)
        .setUsername("default").setPassword("x")
        .build();                           // defaults: compress=true, use_http_compression=false
Environment
  • Cloud
  • Client version: main f9586aba0 (0.12.0-rc1-SNAPSHOT); also checked on PR #3322 head a7f13225e
  • Language version: Java 17 (Eclipse Temurin), httpcore5 5.4.3
  • OS: Linux (Docker)
ClickHouse Server
  • ClickHouse Server version: 26.9.1.1629 (only the real-server row; the other rows use a mock HTTP server)
  • ClickHouse Server non-default settings, if any: none
  • CREATE TABLE statements for tables involved: none
  • Sample data for all these tables: none

Found by automated analysis (Polyglot AI) while working on #2287 (PR #3335, which changes only readClickHouseError). Verified by running the client against a mock HTTP server, a raw socket server, and a live ClickHouse 26.9.1.1629 server, not only by reading the code.

主要语言
Java
星标
1.6k
派生
649
平均合并
1 天 20 小时
30 天内合并 PR
39

环境准备

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

ClickHouse/clickhouse-java 的其他 Issue

查看 ClickHouse/clickhouse-java 的全部 Issue

相似的 Issue

更多 Java Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。