[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 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
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時間
マージ済み PR(30日)
39

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

ClickHouse/clickhouse-java のほかの issue

ClickHouse/clickhouse-java の issue をすべて見る

似ている issue

Java の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。