diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ad5a57f6..6a2a26af2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -153,6 +153,13 @@ ### Bug Fixes +- **[client-v2]** Fixed a query with statement parameters sent in the request body + (`client.http.use_form_request_for_query=true`) failing with `LZ4 decompression failed ... (LZ4_DECODER_FAILED)` + when client request compression and HTTP compression were both enabled. The multipart body is always sent + uncompressed, but the request still declared `Content-Encoding: lz4`; ClickHouse `26.8+` honours that header for + multipart requests and tried to decompress a plain body. The header is now omitted for multipart requests, like + the `decompress` query parameter already was. Response compression (`Accept-Encoding`, + `enable_http_compression`) is unchanged. (https://github.com/ClickHouse/clickhouse-java/issues/3075) - **[client-v1]** Fixed the `DateTime64` case of `testReadWriteSimpleTypes` failing against ClickHouse 26.8. From 26.8 an unquoted number written to a `DateTime64` column in the `Values`/`Quoted` and `JSON` paths is a Unix timestamp in seconds instead of the raw scaled value - the server setting `input_format_read_datetime_number_as_raw_value` diff --git a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java index 036ed48d2..9dab47a39 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java @@ -624,6 +624,11 @@ public TransportRequest createRequest(Endpoint server, Map reque final HttpEntity httpEntity; if (useMultipart) { + // a multipart body is always sent as-is, so the request must not declare a content encoding - the + // server would fail to decompress the plain body. Removed after addHeaders() to also drop an + // encoding set by the application with `http_header_*`. + req.removeHeaders(HttpHeaders.CONTENT_ENCODING); + MultipartEntityBuilder multipartEntityBuilder = MultipartEntityBuilder.create(); addStatementParams(requestConfig, multipartEntityBuilder::addTextBody); multipartEntityBuilder.addTextBody(ClickHouseHttpProto.QPARAM_QUERY_STMT, body); diff --git a/client-v2/src/test/java/com/clickhouse/client/api/internal/HttpAPIClientHelperTest.java b/client-v2/src/test/java/com/clickhouse/client/api/internal/HttpAPIClientHelperTest.java index 16841dc6e..0c7fa375a 100644 --- a/client-v2/src/test/java/com/clickhouse/client/api/internal/HttpAPIClientHelperTest.java +++ b/client-v2/src/test/java/com/clickhouse/client/api/internal/HttpAPIClientHelperTest.java @@ -13,7 +13,9 @@ import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpEntity; +import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.message.BasicHeader; import org.mockito.ArgumentCaptor; import org.mockito.MockedConstruction; @@ -327,6 +329,125 @@ public void testShouldRetryUsesServerExceptionFromCause(Throwable ex, boolean ex assertEquals(helper.shouldRetry(ex, new HashMap<>()), expectedRetry); } + /** + * A multipart body (statement parameters sent as form data) is never compressed, so the request must not + * declare a content encoding - the server would try to decompress the plain body and fail with + * LZ4_DECODER_FAILED. A request that is not multipart, and response compression, keep their signalling. + */ + @DataProvider(name = "requestCompressionSignalling") + public static Object[][] requestCompressionSignalling() { + return new Object[][] { + // clientCompression, useHttpCompression, sendParamsInBody, withParams, + // contentEncoding, acceptEncoding, decompressParam + {true, true, true, true, null, "lz4", false}, + {true, true, true, false, "lz4", "lz4", false}, // no parameters -> not a multipart request + {true, true, false, true, "lz4", "lz4", false}, + {false, true, true, true, null, "lz4", false}, + {true, false, true, true, null, null, false}, + {true, false, false, true, null, null, true}, + }; + } + + @Test(dataProvider = "requestCompressionSignalling") + public void testRequestCompressionSignalling(boolean clientCompression, boolean useHttpCompression, + boolean sendParamsInBody, boolean withParams, + String expectedContentEncoding, String expectedAcceptEncoding, + boolean expectDecompressParam) { + Map reqConfig = compressionConfig(clientCompression, useHttpCompression, sendParamsInBody); + if (withParams) { + reqConfig.put(HttpAPIClientHelper.KEY_STATEMENT_PARAMS, Collections.singletonMap("p1", "1")); + } + + HttpPost req = newHelper().createRequest(new HttpEndpoint("localhost", 8123, false, "/"), reqConfig, + "SELECT {p1:Int32}").getDelegate(); + + String setup = "clientCompression=" + clientCompression + ", useHttpCompression=" + useHttpCompression + + ", sendParamsInBody=" + sendParamsInBody + ", withParams=" + withParams; + assertEquals(headerValue(req, HttpHeaders.CONTENT_ENCODING), expectedContentEncoding, + "unexpected " + HttpHeaders.CONTENT_ENCODING + " for " + setup); + assertEquals(req.getEntity().getContentEncoding(), expectedContentEncoding, + "the request body entity must declare the same encoding as the request for " + setup); + assertEquals(headerValue(req, HttpHeaders.ACCEPT_ENCODING), expectedAcceptEncoding, + "response compression signalling must not depend on the request body form"); + + String query = req.getRequestUri(); + assertEquals(query.contains(ClickHouseHttpProto.QPARAM_DECOMPRESS + "=1"), expectDecompressParam, + "unexpected " + ClickHouseHttpProto.QPARAM_DECOMPRESS + " parameter in " + query); + assertEquals(query.contains(ClickHouseHttpProto.QPARAM_ENABLE_HTTP_COMPRESSION + "=1"), useHttpCompression, + "unexpected " + ClickHouseHttpProto.QPARAM_ENABLE_HTTP_COMPRESSION + " parameter in " + query); + } + + @DataProvider(name = "contentEncodingHeaderNames") + public static Object[][] contentEncodingHeaderNames() { + return new Object[][] {{HttpHeaders.CONTENT_ENCODING}, {"content-encoding"}}; + } + + /** + * A content encoding set by the application through {@code http_header_*} cannot make the plain multipart + * body compressed either, so it must not reach the server, whatever the header is spelled like. + */ + @Test(dataProvider = "contentEncodingHeaderNames") + public void testCustomContentEncodingHeaderRemovedForMultipartRequest(String headerName) { + Map reqConfig = compressionConfig(false, false, true); + reqConfig.put(HttpAPIClientHelper.KEY_STATEMENT_PARAMS, Collections.singletonMap("p1", "1")); + reqConfig.put(ClientConfigProperties.HTTP_HEADER_PREFIX + headerName, "lz4"); + + HttpPost req = newHelper().createRequest(new HttpEndpoint("localhost", 8123, false, "/"), reqConfig, + "SELECT {p1:Int32}").getDelegate(); + + assertNull(headerValue(req, HttpHeaders.CONTENT_ENCODING), + "a custom " + headerName + " must be removed from a multipart request"); + } + + /** + * A request that is not multipart is unaffected: a content encoding set by the application through + * {@code http_header_*} still reaches the server. + */ + @Test(dataProvider = "contentEncodingHeaderNames") + public void testCustomContentEncodingHeaderKeptForRequestWithoutParams(String headerName) { + Map reqConfig = compressionConfig(false, false, true); + reqConfig.put(ClientConfigProperties.HTTP_HEADER_PREFIX + headerName, "lz4"); + + HttpPost req = newHelper().createRequest(new HttpEndpoint("localhost", 8123, false, "/"), reqConfig, + "SELECT 1").getDelegate(); + + assertEquals(headerValue(req, HttpHeaders.CONTENT_ENCODING), "lz4", + "a custom " + headerName + " must be kept on a request that is not multipart"); + } + + /** + * Data is streamed into the request body, so an insert is never a multipart request and keeps compressing + * its body even when the client is configured to send statement parameters in the body. + */ + @Test + public void testDataRequestKeepsContentEncodingWhenParamsInBodyEnabled() { + Map reqConfig = compressionConfig(true, true, true); + + HttpPost req = newHelper().createRequest(new HttpEndpoint("localhost", 8123, false, "/"), reqConfig, + out -> out.write(1)).getDelegate(); + + assertEquals(headerValue(req, HttpHeaders.CONTENT_ENCODING), "lz4", + "an insert body is compressed, so the request must declare the content encoding"); + } + + private static HttpAPIClientHelper newHelper() { + return HttpAPIClientHelperFactory.newHelper(new HashMap<>(), LZ4Factory.fastestInstance()); + } + + private static Map compressionConfig(boolean clientCompression, boolean useHttpCompression, + boolean sendParamsInBody) { + Map reqConfig = new HashMap<>(); + reqConfig.put(ClientConfigProperties.COMPRESS_CLIENT_REQUEST.getKey(), clientCompression); + reqConfig.put(ClientConfigProperties.USE_HTTP_COMPRESSION.getKey(), useHttpCompression); + reqConfig.put(ClientConfigProperties.HTTP_SEND_PARAMS_IN_BODY.getKey(), sendParamsInBody); + return reqConfig; + } + + private static String headerValue(HttpPost req, String name) { + Header header = req.getFirstHeader(name); + return header == null ? null : header.getValue(); + } + /** * A server error is logged at WARN only for an unknown status code (the switch's default branch). Known * error paths emit no server-error WARN: readError surfaces an exception-code error, a mapped code (502)