fix(http): keep response header count and line length unlimited after httpclient5 5.6.4

CHANGELOG.md

@@ -306,6 +306,9 @@
 
 - **[repo]** Upgraded `org.apache.httpcomponents.client5:httpclient5` from `5.4.4` to `5.6.4` in `client-v2` and
   `clickhouse-http-client` to pick up the fixes of the newer 5.x releases, including known vulnerabilities.
+  The new HTTP core version limits an incoming message to 100 headers and to a line length of 8192 bytes by default,
+  which a response of a long query with `send_progress_in_http_headers` exceeds, so both HTTP transports now disable
+  these two limits explicitly and keep the behavior of the previous version.
   (https://github.com/ClickHouse/clickhouse-java/issues/3078)
 
 ### Docs & Examples

clickhouse-http-client/src/main/java/com/clickhouse/client/http/ApacheHttpConnectionImpl.java

@@ -465,14 +465,25 @@ static class HttpConnectionManager extends PoolingHttpClientConnectionManager {
             }
         }
 
+        private static ManagedHttpClientConnectionFactory newConnectionFactory(int networkBufferSize) {
+            // Server may send an unbounded number of X-ClickHouse-Progress headers (one per
+            // progress interval) and headers with long values, so response header count and
+            // line length must stay unlimited (negative value disables the limit).
+            Http1Config http1Config = Http1Config.custom()
+                    .setBufferSize(Math.min(1_000_000, networkBufferSize))
+                    .setMaxHeaderCount(-1)
+                    .setMaxLineLength(-1)
+                    .build();
+            // Parser factory keeps its own config, so the same config is passed to it explicitly.
+            return new ManagedHttpClientConnectionFactory(http1Config, CharCodingConfig.DEFAULT,
+                    new DefaultHttpResponseParserFactory(http1Config));
+        }
+
         public HttpConnectionManager(Registry<ConnectionSocketFactory> socketFactory, ClickHouseConfig config,
                                      PoolConcurrencyPolicy poolConcurrentcyPolicy, PoolReusePolicy poolReusePolicy,
                                      TimeValue ttl, int networkBufferSize) {
             super(socketFactory, poolConcurrentcyPolicy, poolReusePolicy, ttl,
-                    new ManagedHttpClientConnectionFactory(Http1Config.custom()
-                            .setBufferSize(Math.min(1_000_000, networkBufferSize)).build(),
-                            CharCodingConfig.DEFAULT,
-                            DefaultHttpResponseParserFactory.INSTANCE));
+                    newConnectionFactory(networkBufferSize));
             ConnectionConfig connConfig = ConnectionConfig.custom()
                     .setConnectTimeout(Timeout.of(config.getConnectionTimeout(), TimeUnit.MILLISECONDS))
                     .setValidateAfterInactivity(config.getLongOption(ClickHouseHttpOption.AHC_VALIDATE_AFTER_INACTIVITY), TimeUnit.MILLISECONDS)

clickhouse-http-client/src/test/java/com/clickhouse/client/http/ApacheHttpConnectionImplTest.java

@@ -148,6 +148,29 @@ public void testConnection() throws Exception {
         }
     }
 
+    @Test(groups = { "integration" })
+    public void testResponseWithManyProgressHeaders() throws Exception {
+        ClickHouseNode server = getServer(ClickHouseProtocol.HTTP);
+
+        try (ClickHouseClient client = ClickHouseClient.newInstance()) {
+            // The server sends one X-ClickHouse-Progress header per progress interval, so a
+            // slow query produces many more response headers than the HTTP/1 parser default.
+            ClickHouseRequest<?> req = newRequest(client, server)
+                    .set("send_progress_in_http_headers", 1)
+                    .set("http_headers_progress_interval_ms", 10)
+                    .set("max_block_size", 1)
+                    .set("wait_end_of_query", 1);
+            try (ClickHouseResponse resp = req.query("select number, sleep(0.05) from numbers(150)")
+                    .executeAndWait()) {
+                int count = 0;
+                for (Object ignored : resp.records()) {
+                    count++;
+                }
+                Assert.assertEquals(count, 150);
+            }
+        }
+    }
+
     @Test(groups = { "integration" })
     @Ignore("Need to disable the option to provide custom socket factory. note: need to remove it")
     public void testCustomOptions() throws Exception {

client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java

@@ -233,14 +233,29 @@ private ConnectionConfig createConnectionConfig(Map<String, Object> configuratio
         return connConfig.build();
     }
 
+    private static MeteredManagedHttpClientConnectionFactory createConnectionFactory(int networkBufferSize) {
+        // Server may send an unbounded number of X-ClickHouse-Progress headers (one per
+        // progress interval) and headers with long values, so response header count and
+        // line length must stay unlimited (negative value disables the limit).
+        Http1Config http1Config = Http1Config.custom()
+                .setBufferSize(networkBufferSize)
+                .setMaxHeaderCount(-1)
+                .setMaxLineLength(-1)
+                .build();
+        // Parser factory keeps its own config, so the same config is passed to it explicitly.
+        return new MeteredManagedHttpClientConnectionFactory(http1Config, CharCodingConfig.DEFAULT,
+                new DefaultHttpResponseParserFactory(http1Config));
+    }
+
     private HttpClientConnectionManager basicConnectionManager(LayeredConnectionSocketFactory sslConnectionSocketFactory, SocketConfig socketConfig, Map<String, Object> configuration) {
         Lookup<TlsSocketStrategy> tlsSocketStrategyLookup = RegistryBuilder.<TlsSocketStrategy>create()
                 .register(URIScheme.HTTPS.id, (socket, target, port, attachment, context) ->
                         (SSLSocket) sslConnectionSocketFactory.createLayeredSocket(socket, target, port, context))
                 .build();
 
+        int networkBufferSize = ClientConfigProperties.CLIENT_NETWORK_BUFFER_SIZE.getOrDefault(configuration);
         BasicHttpClientConnectionManager connManager = BasicHttpClientConnectionManager.create(
-                null, null, tlsSocketStrategyLookup, null);
+                null, null, tlsSocketStrategyLookup, createConnectionFactory(networkBufferSize));
         connManager.setConnectionConfig(createConnectionConfig(configuration));
         connManager.setSocketConfig(socketConfig);
 
@@ -269,12 +284,7 @@ private HttpClientConnectionManager poolConnectionManager(LayeredConnectionSocke
         ClientConfigProperties.HTTP_MAX_OPEN_CONNECTIONS.applyIfSet(configuration, connMgrBuilder::setMaxConnPerRoute);
 
         int networkBufferSize = ClientConfigProperties.CLIENT_NETWORK_BUFFER_SIZE.getOrDefault(configuration);
-        MeteredManagedHttpClientConnectionFactory connectionFactory = new MeteredManagedHttpClientConnectionFactory(
-                Http1Config.custom()
-                        .setBufferSize(networkBufferSize)
-                        .build(),
-                CharCodingConfig.DEFAULT,
-                DefaultHttpResponseParserFactory.INSTANCE);
+        MeteredManagedHttpClientConnectionFactory connectionFactory = createConnectionFactory(networkBufferSize);
 
         connMgrBuilder.setConnectionFactory(connectionFactory);
         connMgrBuilder.setSSLSocketFactory(sslConnectionSocketFactory);

client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java

@@ -1160,6 +1160,37 @@
     }
 
 
+    @DataProvider(name = "connectionPoolEnabled")
+    public static Object[][] connectionPoolEnabled() {
+        return new Object[][] { { true }, { false } };
+    }
+
+    @Test(groups = { "integration" }, dataProvider = "connectionPoolEnabled")
+    public void testResponseWithManyProgressHeaders(boolean connectionPoolEnabled) throws Exception {
+        if (isCloud()) {
+            return; // mocked server
+        }
+
+        ClickHouseNode server = getServer(ClickHouseProtocol.HTTP);
+        try (Client client = new Client.Builder().addEndpoint(server.getBaseUri())
+                .setUsername("default")
+                .setPassword(ClickHouseServerForTest.getPassword())
+                .enableConnectionPool(connectionPoolEnabled)
+                .build()) {
+
+            // The server sends one X-ClickHouse-Progress header per progress interval, so a
+            // slow query produces many more response headers than the HTTP/1 parser default.
+            QuerySettings settings = new QuerySettings()
+                    .serverSetting("send_progress_in_http_headers", "1")
+                    .serverSetting("http_headers_progress_interval_ms", "10")
+                    .serverSetting("max_block_size", "1");
+
+            List<GenericRecord> records = client.queryAll(
+                    "SELECT number, sleep(0.05) FROM numbers(150)", settings);
+            Assert.assertEquals(records.size(), 150);
+        }
+    }
+
     @Test(groups = { "integration" }, dataProvider = "testUserAgentHasCompleteProductName_dataProvider", dataProviderClass = HttpTransportTests.class)
     public void testUserAgentHasCompleteProductName(String clientName, Pattern userAgentPattern) throws Exception {
         if (isCloud()) {