Merge pull request #27385 from BraulioVM/fix/27171

Do not fail if HTTP port is not set and user tries to send request to TCP port

src/Server/TCPHandler.cpp

@@ -32,6 +32,7 @@
 #include <DataTypes/DataTypeLowCardinality.h>
 #include <Compression/CompressionFactory.h>
 #include <common/logger_useful.h>
+#include <fmt/format.h>
 
 #include <Processors/Executors/PullingAsyncPipelineExecutor.h>
 
@@ -46,6 +47,27 @@
 namespace DB
 {
 
+namespace
+{
+std::string formatHTTPErrorResponse(const Poco::Util::AbstractConfiguration& config)
+{
+    std::string result = fmt::format(
+        "HTTP/1.0 400 Bad Request\r\n\r\n"
+        "Port {} is for clickhouse-client program\r\n",
+        config.getString("tcp_port"));
+
+    if (config.has("http_port"))
+    {
+        result += fmt::format(
+            "You must use port {} for HTTP.\r\n",
+            config.getString("http_port"));
+    }
+
+    return result;
+}
+}
+
+
 namespace ErrorCodes
 {
     extern const int LOGICAL_ERROR;
@@ -922,10 +944,8 @@ void TCPHandler::receiveHello()
           */
         if (packet_type == 'G' || packet_type == 'P')
         {
-            writeString("HTTP/1.0 400 Bad Request\r\n\r\n"
-                "Port " + server.config().getString("tcp_port") + " is for clickhouse-client program.\r\n"
-                "You must use port " + server.config().getString("http_port") + " for HTTP.\r\n",
-                *out);
+            writeString(formatHTTPErrorResponse(server.config()),
+                        *out);
 
             throw Exception("Client has connected to wrong port", ErrorCodes::CLIENT_HAS_CONNECTED_TO_WRONG_PORT);
         }

tests/integration/test_tcp_handler_http_responses/configs/config.d/http-port-31337.xml

@@ -0,0 +1,3 @@
+<yandex>
+  <http_port replace="replace">31337</http_port>
+</yandex>

tests/integration/test_tcp_handler_http_responses/configs/config.d/no-http-port.xml

@@ -0,0 +1,3 @@
+<yandex>
+    <http_port remove="remove"></http_port>
+</yandex>

tests/integration/test_tcp_handler_http_responses/test_case.py

@@ -0,0 +1,42 @@
+"""Test HTTP responses given by the TCP Handler."""
+from pathlib import Path
+import pytest
+from helpers.cluster import ClickHouseCluster
+import requests
+
+cluster = ClickHouseCluster(__file__)
+
+node_with_http = cluster.add_instance(
+    'node_with_http',
+    main_configs=["configs/config.d/http-port-31337.xml"]
+)
+HTTP_PORT = 31337
+
+node_without_http = cluster.add_instance(
+    'node_without_http',
+    main_configs=["configs/config.d/no-http-port.xml"]
+)
+
+@pytest.fixture(scope="module")
+def start_cluster():
+    try:
+        cluster.start()
+        yield cluster
+
+    finally:
+        cluster.shutdown()
+
+def test_request_to_http_full_instance(start_cluster):
+    response = requests.get(
+        f'http://{node_with_http.ip_address}:9000'
+    )
+    assert response.status_code == 400
+    assert str(HTTP_PORT) in response.text
+
+def test_request_to_http_less_instance(start_cluster):
+    response = requests.post(
+        f'http://{node_without_http.ip_address}:9000'
+    )
+    assert response.status_code == 400
+    assert str(HTTP_PORT) not in response.text
+    assert "8123"         not in response.text