From 4ab65aef5baddda8d80061dce966e3d65d3f09af Mon Sep 17 00:00:00 2001 From: Andrey Zvonov <32552679+zvonand@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:10:59 +0200 Subject: [PATCH 1/4] Add an Antalya-only protocol version advertised in the `ServerHello` Antalya carries wire features that do not exist upstream and rebases onto every upstream release. An Antalya-only change that takes a slot in `DBMS_TCP_PROTOCOL_VERSION` or `DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION` is renumbered by the next rebase, so the same number comes to mean two different things in two shipped builds. `DBMS_ANTALYA_PROTOCOL_VERSION` is a counter in a number space upstream cannot reach, in its own header rather than in `ProtocolDefines.h`, whose tail is where every rebase conflicts. `TCPHandler::sendHello` appends " (antalya:N)" to the `ServerHello` name on every connection; `Connection::receiveHello` strips it and keeps `min(own, N)`. Version 1 is the advertisement itself, with no payload. The advertisement is one-directional by necessity. The client writes its `Hello` before reading anything from the peer, so it cannot gate a marker on what the peer is, and every field in that packet is one an upstream server acts on: `client_name` is persisted to `system.query_log` and compared against the Query packet's `ClientInfo` under `validate_tcp_client_information`, so a marker there would fail a `remote()` query against any peer that does not strip it with `CLIENT_INFO_DOES_NOT_MATCH`. `server_name` carries no such role - it is client-side display text that reaches no system table - which is why the server is the side that speaks. A feature that needs the server to learn the client's version will need a new Antalya-only client packet type sent after the Addendum, gated on the negotiated version. The Addendum cannot carry it: `TCPHandler::receiveAddendum` reads a fixed field list and cannot tell a client that wrote an extra field from one that did not, so it would read bytes that are not there. See `docs/en/antalya/protocol.md`. The only effect on an upstream peer is cosmetic: a client that does not strip the marker displays `ClickHouse (antalya:1)` as the server name. Covered by `gtest_antalya_protocol` for the marker grammar, `05054` for the advertisement being independent of the client name, `05053` for no Antalya string reaching `system.query_log`, and `test_antalya_protocol`, which exercises both directions against a build that predates the marker. --- docs/en/antalya/protocol.md | 104 ++++++++++++ src/Client/Connection.cpp | 13 +- src/Client/Connection.h | 1 + ...gtest_sanitize_untrusted_server_string.cpp | 21 +++ src/Core/AntalyaProtocol.cpp | 82 +++++++++ src/Core/AntalyaProtocol.h | 62 +++++++ src/Core/Protocol.h | 5 + src/Core/tests/gtest_antalya_protocol.cpp | 78 +++++++++ src/Interpreters/Cluster.cpp | 7 +- src/Server/TCPHandler.cpp | 5 +- .../test_antalya_protocol/__init__.py | 0 .../configs/remote_servers.xml | 18 ++ .../configs/validate_client_info.xml | 6 + .../integration/test_antalya_protocol/test.py | 156 ++++++++++++++++++ .../05053_antalya_protocol_marker.reference | 3 + .../05053_antalya_protocol_marker.sql | 33 ++++ .../05054_antalya_protocol_server_hello.py | 127 ++++++++++++++ ...54_antalya_protocol_server_hello.reference | 2 + 18 files changed, 717 insertions(+), 6 deletions(-) create mode 100644 docs/en/antalya/protocol.md create mode 100644 src/Core/AntalyaProtocol.cpp create mode 100644 src/Core/AntalyaProtocol.h create mode 100644 src/Core/tests/gtest_antalya_protocol.cpp create mode 100644 tests/integration/test_antalya_protocol/__init__.py create mode 100644 tests/integration/test_antalya_protocol/configs/remote_servers.xml create mode 100644 tests/integration/test_antalya_protocol/configs/validate_client_info.xml create mode 100644 tests/integration/test_antalya_protocol/test.py create mode 100644 tests/queries/0_stateless/05053_antalya_protocol_marker.reference create mode 100644 tests/queries/0_stateless/05053_antalya_protocol_marker.sql create mode 100755 tests/queries/0_stateless/05054_antalya_protocol_server_hello.py create mode 100644 tests/queries/0_stateless/05054_antalya_protocol_server_hello.reference diff --git a/docs/en/antalya/protocol.md b/docs/en/antalya/protocol.md new file mode 100644 index 000000000000..00817df0f762 --- /dev/null +++ b/docs/en/antalya/protocol.md @@ -0,0 +1,104 @@ +--- +description: 'How the Antalya fork versions its own wire-protocol changes independently of upstream ClickHouse' +sidebar_label: 'Antalya Protocol Version' +sidebar_position: 40 +slug: /antalya/protocol +title: 'Antalya Protocol Version' +doc_type: 'reference' +--- + +# Antalya protocol version {#antalya-protocol-version} + +## Why a separate counter {#why-a-separate-counter} + +Antalya carries features that do not exist in upstream ClickHouse, and it rebases onto each new +upstream release. An Antalya-only wire change that takes a slot in an upstream counter - +`DBMS_TCP_PROTOCOL_VERSION` or `DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION` - has to be renumbered +whenever upstream claims that slot for something else. The same number then means two different +things in two shipped builds, and two nodes that negotiate it disagree about the bytes on the wire. + +`DBMS_ANTALYA_PROTOCOL_VERSION` (in `src/Core/AntalyaProtocol.h`) is a counter in a number space +that upstream cannot reach, so a rebase can never renumber an Antalya feature. It lives in its own +header rather than in `src/Core/ProtocolDefines.h`, whose tail is where upstream adds its own +constants and therefore where every rebase conflicts. + +## The rule {#the-rule} + +An Antalya-only wire change bumps `DBMS_ANTALYA_PROTOCOL_VERSION` and gates itself on the +negotiated value. It never bumps an upstream counter and never inserts a slot into +`DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION`. + +The client caps the server's value with `min(own, server)`, which is only sound while the counter +describes a cumulative feature set. A backport must therefore take the whole contiguous range up to +the value it needs, or not bump at all. + +## How it is advertised {#how-it-is-advertised} + +The version travels inside the `ServerHello` *name* string, in one direction only: + +```text +server -> client "ClickHouse (antalya:M)" (every connection, unconditionally) +``` + +The client caps it with its own value and strips the marker while parsing, so `server_name` reads +exactly as it would coming from upstream. A value of `0` means the server is not Antalya. + +Nothing is appended in the other direction, and the client `Hello` is never marked. The client +writes that packet before it has read a byte from the peer, so it cannot gate a marker on what the +peer is, and every field in it - `client_name`, `default_database`, `user`, `password` - is one an +upstream server acts on. `client_name` is the worst of them: a server persists it and +`validate_tcp_client_information` compares it against the Query packet's `ClientInfo`, so a marker +there fails a `remote()` query with `CLIENT_INFO_DOES_NOT_MATCH` on any peer that does not strip it. +The `ServerHello` has no such field - `server_name` is client-side display text and reaches no +system table - which is why the server is the side that speaks. + +The marker lives inside an existing string rather than in appended bytes because neither Hello nor +the Addendum has a length prefix or a terminator: a reader stops after the last field its negotiated +revision knows about, and a peer that does not expect extra bytes reads them as the next packet. + +## What version 1 supports {#what-version-1-supports} + +Version 1 is the advertisement itself; there is no wire payload beyond the marker. Because only the +server advertises, a feature gated on this version can only be one the *client* decides to use. + +A feature that needs the server to know the client's version needs a channel the client writes after +it has read the `ServerHello` - by then it knows the peer is Antalya - and that an upstream server +would never read. The Addendum is not that channel: `TCPHandler::receiveAddendum` reads a fixed +field list, and an Antalya server cannot tell a client that wrote an extra field from one that did +not, so it would read bytes that are not there and desynchronise the stream. A new Antalya-only +client packet type, numbered far above `Protocol::Client::MAX` and sent right after the Addendum, +is: an upstream client never sends it, and an Antalya client sends it only when the negotiated +version is at least the one that introduced it. That costs a version bump when it is first needed +and nothing today. + +## Scope {#scope} + +Every connection to an Antalya server is advertised to, `clickhouse-client` included. Any Antalya +client reads the value - `clickhouse-client`, Distributed, `*Cluster`, swarm, parallel replicas - +and an upstream client ignores it. + +Negotiation is per hop and not transitive: initiator to worker and worker to worker negotiate +independently. + +## Observability {#observability} + +The negotiated version, when non-zero, is appended to the client's existing connection log line +(`Connected to ... server version ...`). The server logs nothing about it, because it learns nothing +about the peer. Degradation to `0` is otherwise silent. + +No Antalya string reaches anything a server stores. `client_name` is never marked, so +`system.query_log` and `system.processes` are unaffected and filters written as +`client_name = 'ClickHouse server'` keep working. + +The marker is visible in exactly one place: an **upstream** client's `server_name`, which that +client does not strip. An upstream `clickhouse-client` prints +`Connected to ClickHouse (antalya:1) server version ...` in its interactive banner. An Antalya +client strips the marker and prints what upstream would have printed. + +## Implementation {#implementation} + +`src/Core/AntalyaProtocol.h` holds the version constant, the marker grammar and the `appendMarker` +/ `parseMarker` / `stripMarker` / `negotiate` helpers. The parse is an anchored suffix scan bounded +to the marker length, because the client runs it on a `server_name` it has not authenticated. +`TCPHandler::sendHello` appends the marker; `Connection::receiveHello` strips it and stores the +capped value. diff --git a/src/Client/Connection.cpp b/src/Client/Connection.cpp index 746c1bc0b8cc..ee0681625341 100644 --- a/src/Client/Connection.cpp +++ b/src/Client/Connection.cpp @@ -26,7 +26,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -369,8 +371,11 @@ void Connection::connect(const ConnectionTimeouts & timeouts) if (proto_recv_chunked == "chunked") in->enableChunked(); - LOG_TRACE(log_wrapper.get(), "Connected to {} server version {}.{}.{}.", - server_name, server_version_major, server_version_minor, server_version_patch); + LOG_TRACE(log_wrapper.get(), "Connected to {} server version {}.{}.{}{}.", + server_name, server_version_major, server_version_minor, server_version_patch, + (server_antalya_protocol_version > 0 + ? ", Antalya protocol: " + std::to_string(server_antalya_protocol_version) + : "")); /// Now that the handshake is complete, use the regular timeouts socket->setReceiveTimeout(timeouts.receive_timeout); @@ -622,6 +627,10 @@ void Connection::receiveHello() { readStringBinary(server_name, *in, DBMS_MAX_HELLO_STRING_SIZE); sanitizeUntrustedServerString(server_name); + + /// The marker is stripped here, so `server_name` reads exactly as it would coming from an + /// upstream server. + server_antalya_protocol_version = AntalyaProtocol::negotiate(AntalyaProtocol::stripMarker(server_name)); readVarUInt(server_version_major, *in); readVarUInt(server_version_minor, *in); readVarUInt(server_revision, *in); diff --git a/src/Client/Connection.h b/src/Client/Connection.h index c9d64c55c3be..d2a4a5504a35 100644 --- a/src/Client/Connection.h +++ b/src/Client/Connection.h @@ -238,6 +238,7 @@ class Connection : public IServerConnection UInt64 server_parallel_replicas_protocol_version = 0; UInt64 worker_cluster_function_protocol_version = 0; UInt64 server_query_plan_serialization_version = 0; + UInt64 server_antalya_protocol_version = 0; String server_timezone; String server_display_name; SettingsChanges settings_from_server; diff --git a/src/Client/tests/gtest_sanitize_untrusted_server_string.cpp b/src/Client/tests/gtest_sanitize_untrusted_server_string.cpp index 195202691eb5..bfa285da8661 100644 --- a/src/Client/tests/gtest_sanitize_untrusted_server_string.cpp +++ b/src/Client/tests/gtest_sanitize_untrusted_server_string.cpp @@ -1,6 +1,7 @@ #include #include +#include #include using namespace DB; @@ -155,3 +156,23 @@ TEST(SanitizeUntrustedServerString, MaxPasswordComplexityRulesCapIsTight) EXPECT_LE(DBMS_MAX_PASSWORD_COMPLEXITY_RULES, 4096u); EXPECT_GE(DBMS_MAX_PASSWORD_COMPLEXITY_RULES, 16u); } + +TEST(SanitizeUntrustedServerString, PreservesAnAntalyaProtocolMarker) +{ + /// The client parses `server_name` only after sanitizing it, so the marker must pass through + /// byte for byte. See `Core/AntalyaProtocol.h`. + String name = AntalyaProtocol::appendMarker("ClickHouse"); + const String before = name; + sanitizeUntrustedServerString(name); + EXPECT_EQ(name, before); + EXPECT_EQ(AntalyaProtocol::parseMarker(name), static_cast(DBMS_ANTALYA_PROTOCOL_VERSION)); +} + +TEST(SanitizeUntrustedServerString, CannotForgeAnAntalyaProtocolMarker) +{ + /// Control bytes are replaced, never deleted, so a hostile `server_name` cannot be collapsed + /// into a canonical marker. Switching the helper to deletion would break this. + String hostile = "ClickHouse\x01 (antalya:\x02" "1)"; + sanitizeUntrustedServerString(hostile); + EXPECT_EQ(AntalyaProtocol::parseMarker(hostile), 0u); +} diff --git a/src/Core/AntalyaProtocol.cpp b/src/Core/AntalyaProtocol.cpp new file mode 100644 index 000000000000..5c124299529c --- /dev/null +++ b/src/Core/AntalyaProtocol.cpp @@ -0,0 +1,82 @@ +#include + +#include +#include + +#include + + +namespace DB +{ + +namespace AntalyaProtocol +{ + +/// The largest version the grammar can spell: `MAX_MARKER_DIGITS` nines. +constexpr UInt64 MAX_VERSION = intExp10(static_cast(MAX_MARKER_DIGITS)) - 1; + +static_assert( + DBMS_ANTALYA_PROTOCOL_VERSION >= 1 && DBMS_ANTALYA_PROTOCOL_VERSION <= MAX_VERSION, + "DBMS_ANTALYA_PROTOCOL_VERSION does not fit the marker grammar"); + +String appendMarker(std::string_view name) +{ + String result; + result.reserve(name.size() + MAX_MARKER_SIZE); + result.append(name); + result.append(MARKER_PREFIX); + result.append(std::to_string(DBMS_ANTALYA_PROTOCOL_VERSION)); + result.push_back(MARKER_TERMINATOR); + return result; +} + +UInt64 parseMarker(std::string_view name) noexcept +{ + if (name.empty() || name.back() != MARKER_TERMINATOR) + return 0; + + const size_t close = name.size() - 1; + + size_t first_digit = close; + while (first_digit > 0 && isNumericASCII(name[first_digit - 1])) + { + --first_digit; + if (close - first_digit > MAX_MARKER_DIGITS) + return 0; + } + + const size_t digits = close - first_digit; + /// One version must have exactly one spelling on the wire: no empty run, no leading zero. + if (digits == 0 || name[first_digit] == '0') + return 0; + + if (first_digit < MARKER_PREFIX.size()) + return 0; + if (name.substr(first_digit - MARKER_PREFIX.size(), MARKER_PREFIX.size()) != MARKER_PREFIX) + return 0; + + UInt64 version = 0; + for (size_t i = first_digit; i < close; ++i) + version = version * 10 + static_cast(name[i] - '0'); + return version; +} + +UInt64 stripMarker(String & name) +{ + const UInt64 version = parseMarker(name); + if (version != 0) + { + /// Only the canonical spelling parses, so the digits on the wire are the ones `to_string` gives. + name.resize(name.size() - MARKER_PREFIX.size() - std::to_string(version).size() - 1); + } + return version; +} + +UInt64 negotiate(UInt64 peer_version) noexcept +{ + return std::min(peer_version, DBMS_ANTALYA_PROTOCOL_VERSION); +} + +} + +} diff --git a/src/Core/AntalyaProtocol.h b/src/Core/AntalyaProtocol.h new file mode 100644 index 000000000000..58fe69f5bff0 --- /dev/null +++ b/src/Core/AntalyaProtocol.h @@ -0,0 +1,62 @@ +#pragma once + +#include + +#include + + +namespace DB +{ + +/// ============================== ANTALYA ONLY ============================== +/// One counter for all Antalya-only wire features, independent of every upstream counter in +/// `Core/ProtocolDefines.h`. It is advertised inside the `ServerHello` name string, which a client +/// only displays, so a rebase can never renumber an Antalya feature. +/// +/// Bump it by exactly one per feature and never reuse a value; never bump an upstream counter for +/// an Antalya-only change. The client caps the server's value with `min(own, server)`, so the +/// feature set must stay cumulative: a backport takes the whole contiguous range up to the value it +/// needs, or does not bump at all. See `docs/en/antalya/protocol.md`. +/// +/// Version 1: the advertisement itself. No wire payload beyond the marker. +static constexpr auto DBMS_ANTALYA_PROTOCOL_VERSION = 1; +/// ========================================================================== + +namespace AntalyaProtocol +{ + +/// The version travels inside the `ServerHello` name string, in one direction only: +/// +/// server -> client "ClickHouse (antalya:M)" (every connection, unconditionally) +/// +/// The client strips the marker while parsing, so it never reaches anything that stores or displays +/// the name. The client `Hello` is never marked: `client_name` is the one field a peer persists to +/// `system.query_log` and compares in `validateClientInfo`, and the client writes it before it +/// knows anything about the peer. See `docs/en/antalya/protocol.md`. + +/// " (antalya:" ")", where is [1-9][0-9]{0,8}. +inline constexpr std::string_view MARKER_PREFIX = " (antalya:"; +inline constexpr char MARKER_TERMINATOR = ')'; +inline constexpr size_t MAX_MARKER_DIGITS = 9; +inline constexpr size_t MAX_MARKER_SIZE = MARKER_PREFIX.size() + MAX_MARKER_DIGITS + 1; + +/// Returns `name` with our own version's marker appended. +String appendMarker(std::string_view name); + +/// Strict, anchored suffix parse: returns 0 unless `name` ends with exactly one canonical marker. +/// Scans at most `MAX_MARKER_SIZE` bytes back from the end - the client runs this on a `server_name` +/// it has not authenticated, where an unbounded scan would be a CPU amplifier. +UInt64 parseMarker(std::string_view name) noexcept; + +/// If `name` ends with a canonical marker, removes it and returns the version it spelled; otherwise +/// leaves `name` alone and returns 0. The marker is negotiation metadata rather than part of the +/// name, so the receiver strips it before the name is stored or displayed. +UInt64 stripMarker(String & name); + +/// The version to speak with a server that advertised `peer_version`; 0 means the server is not +/// Antalya. +UInt64 negotiate(UInt64 peer_version) noexcept; + +} + +} diff --git a/src/Core/Protocol.h b/src/Core/Protocol.h index 3f58e654d9ea..1641e956a6f7 100644 --- a/src/Core/Protocol.h +++ b/src/Core/Protocol.h @@ -68,6 +68,11 @@ const char JWT_AUTHENTICAION_MARKER[] = " JWT AUTHENTICATION "; }; +/// `client_name` that `Cluster` passes to `Connection` for server-to-server connections +/// (Distributed, `*Cluster`, swarm, parallel replicas), reaching the Hello packet as +/// " server". The counterpart of `DEFAULT_CLIENT_NAME` ("client"). +static constexpr std::string_view SERVER_CLIENT_NAME = "server"; + namespace Protocol { /// Packet types that server transmits. diff --git a/src/Core/tests/gtest_antalya_protocol.cpp b/src/Core/tests/gtest_antalya_protocol.cpp new file mode 100644 index 000000000000..78938997f52d --- /dev/null +++ b/src/Core/tests/gtest_antalya_protocol.cpp @@ -0,0 +1,78 @@ +#include + +#include + +using namespace DB; +using namespace DB::AntalyaProtocol; + + +TEST(AntalyaProtocol, AppendMarkerSpellsTheWireForm) +{ + EXPECT_EQ( + appendMarker("ClickHouse server"), + "ClickHouse server (antalya:" + std::to_string(DBMS_ANTALYA_PROTOCOL_VERSION) + ")"); +} + +TEST(AntalyaProtocol, AppendThenParseRoundTrips) +{ + EXPECT_EQ(parseMarker(appendMarker("ClickHouse server")), static_cast(DBMS_ANTALYA_PROTOCOL_VERSION)); +} + +TEST(AntalyaProtocol, ParseRejectsNonCanonicalMarkers) +{ + const std::string_view rejected[] = { + "", + "ClickHouse server", + "ClickHouse client", + "(antalya:1)", /// no base name, so no leading space either + "ClickHouse server(antalya:1)", /// missing the separating space + "ClickHouse server (antalya:1", /// unterminated + "ClickHouse server (antalya:)", /// no digits + "ClickHouse server (antalya:0)", /// zero is not a valid version + "ClickHouse server (antalya:01)", /// leading zero is not canonical + "ClickHouse server (antalya:1234567890)", /// 10 digits, above the cap + "ClickHouse server (antalya:1) v2", /// marker must be the suffix + "ClickHouse server (ANTALYA:1)", /// case sensitive + "ClickHouse server (antalya:1x)", + "ClickHouse server (antalya:1 )", + "antalya:1)", + ")", + }; + + for (const auto & name : rejected) + EXPECT_EQ(parseMarker(name), 0u) << "should not have parsed: " << name; +} + +TEST(AntalyaProtocol, ParseTakesTheTrailingMarkerWhenRepeated) +{ + /// The scan is anchored at the end, so a doubled marker yields the last value. + EXPECT_EQ(parseMarker("ClickHouse server (antalya:99) (antalya:1)"), 1u); +} + +TEST(AntalyaProtocol, StripMarkerRemovesOnlyTheMarker) +{ + String marked = "ClickHouse server (antalya:42)"; + EXPECT_EQ(stripMarker(marked), 42u); + EXPECT_EQ(marked, "ClickHouse server"); + + /// The widest marker the grammar allows, to pin the digit arithmetic at both ends. + String widest = "ClickHouse (antalya:999999999)"; + EXPECT_EQ(stripMarker(widest), 999999999u); + EXPECT_EQ(widest, "ClickHouse"); + + String plain = "ClickHouse server"; + EXPECT_EQ(stripMarker(plain), 0u); + EXPECT_EQ(plain, "ClickHouse server"); + + String malformed = "ClickHouse server (antalya:01)"; + EXPECT_EQ(stripMarker(malformed), 0u); + EXPECT_EQ(malformed, "ClickHouse server (antalya:01)"); +} + +TEST(AntalyaProtocol, NegotiateCapsToOurVersion) +{ + EXPECT_EQ(negotiate(0), 0u); + EXPECT_EQ(negotiate(DBMS_ANTALYA_PROTOCOL_VERSION), static_cast(DBMS_ANTALYA_PROTOCOL_VERSION)); + EXPECT_EQ(negotiate(DBMS_ANTALYA_PROTOCOL_VERSION + 1), static_cast(DBMS_ANTALYA_PROTOCOL_VERSION)); + EXPECT_EQ(negotiate(999999999), static_cast(DBMS_ANTALYA_PROTOCOL_VERSION)); +} diff --git a/src/Interpreters/Cluster.cpp b/src/Interpreters/Cluster.cpp index f4fc30f9e8c7..613cf81f9b87 100644 --- a/src/Interpreters/Cluster.cpp +++ b/src/Interpreters/Cluster.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -516,7 +517,7 @@ Cluster::Cluster(const Poco::Util::AbstractConfiguration & config, address.quota_key, address.cluster, address.cluster_secret, - "server", + String(SERVER_CLIENT_NAME), address.compression, address.secure, address.bind_host, @@ -676,7 +677,7 @@ void Cluster::addShard( replica.quota_key, replica.cluster, replica.cluster_secret, - "server", + String(SERVER_CLIENT_NAME), replica.compression, replica.secure, replica.bind_host, @@ -845,7 +846,7 @@ Cluster::Cluster(Cluster::ReplicasAsShardsTag, const Cluster & from, const Setti address.quota_key, address.cluster, address.cluster_secret, - "server", + String(SERVER_CLIENT_NAME), address.compression, address.secure, address.bind_host, diff --git a/src/Server/TCPHandler.cpp b/src/Server/TCPHandler.cpp index 24e4eefff641..dd1673669d11 100644 --- a/src/Server/TCPHandler.cpp +++ b/src/Server/TCPHandler.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -2136,7 +2137,9 @@ void TCPHandler::processUnexpectedHello() void TCPHandler::sendHello() { writeVarUInt(Protocol::Server::Hello, *out); - writeStringBinary(VERSION_NAME, *out); + /// Unconditional: the client writes its Hello first, so the server is the only side that can + /// advertise without already knowing what the peer is. See `docs/en/antalya/protocol.md`. + writeStringBinary(AntalyaProtocol::appendMarker(VERSION_NAME), *out); writeVarUInt(VERSION_MAJOR, *out); writeVarUInt(VERSION_MINOR, *out); writeVarUInt(DBMS_TCP_PROTOCOL_VERSION, *out); diff --git a/tests/integration/test_antalya_protocol/__init__.py b/tests/integration/test_antalya_protocol/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/integration/test_antalya_protocol/configs/remote_servers.xml b/tests/integration/test_antalya_protocol/configs/remote_servers.xml new file mode 100644 index 000000000000..00754e56310a --- /dev/null +++ b/tests/integration/test_antalya_protocol/configs/remote_servers.xml @@ -0,0 +1,18 @@ + + + + + + node29000 + + + + + antalya_secret + + node29000 + + + + diff --git a/tests/integration/test_antalya_protocol/configs/validate_client_info.xml b/tests/integration/test_antalya_protocol/configs/validate_client_info.xml new file mode 100644 index 000000000000..2042b7f4a861 --- /dev/null +++ b/tests/integration/test_antalya_protocol/configs/validate_client_info.xml @@ -0,0 +1,6 @@ + + + true + diff --git a/tests/integration/test_antalya_protocol/test.py b/tests/integration/test_antalya_protocol/test.py new file mode 100644 index 000000000000..0bedc6b5d9c9 --- /dev/null +++ b/tests/integration/test_antalya_protocol/test.py @@ -0,0 +1,156 @@ +"""Antalya protocol version negotiation over the native TCP handshake. + +Covers what a stateless test cannot express: a cluster in `remote_servers` against a separate host, +one authenticated with a ``, and a peer built before the marker existed. See +`src/Core/AntalyaProtocol.h`. + +Only the side that opened the connection logs the negotiated version, because only the server +advertises and only the client reads. The log assertions count occurrences before and after the +query because the cluster fixture is module-scoped, so a plain substring check would pass on an +earlier test's output. +""" + +import pytest + +from helpers.cluster import CLICKHOUSE_CI_MIN_TESTED_VERSION, ClickHouseCluster + +cluster = ClickHouseCluster(__file__) + +MAIN_CONFIGS = ["configs/remote_servers.xml", "configs/validate_client_info.xml"] + +node1 = cluster.add_instance("node1", main_configs=MAIN_CONFIGS) +node2 = cluster.add_instance("node2", main_configs=MAIN_CONFIGS) +# A build from before the marker existed: it sends an unmarked `ServerHello` and does not know to +# strip one. Nothing in the handshake may depend on the peer being Antalya. +node_old = cluster.add_instance( + "node_old", + image="altinity/clickhouse-server", + tag=CLICKHOUSE_CI_MIN_TESTED_VERSION, + with_installed_binary=True, +) + +NEGOTIATED = "Antalya protocol: " + + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + yield cluster + finally: + cluster.shutdown() + + +def count_in_log(node, substring): + return int(node.count_in_log(substring)) + + +def test_remote_function_negotiates(started_cluster): + """The initiator reads the version off the marked `ServerHello` it gets back. The worker learns + nothing about the initiator, so only the initiator's log carries the line.""" + initiator_before = count_in_log(node1, NEGOTIATED) + worker_before = count_in_log(node2, NEGOTIATED) + + assert node1.query("SELECT count() FROM remote('node2', system.one)") == "1\n" + + assert count_in_log(node1, NEGOTIATED) > initiator_before + assert count_in_log(node2, NEGOTIATED) == worker_before + + +def test_distributed_cluster_negotiates(started_cluster): + negotiated_before = count_in_log(node1, NEGOTIATED) + + assert ( + node1.query( + "SELECT count() FROM clusterAllReplicas('plain_cluster', system.one)" + " SETTINGS prefer_localhost_replica = 0" + ) + == "1\n" + ) + + assert count_in_log(node1, NEGOTIATED) > negotiated_before + + +def test_interserver_secret_negotiates(started_cluster): + """The worker returns early from `receiveHello` for an interserver-secret connection, but + `sendHello` still runs, so the initiator still gets a marked `ServerHello`.""" + secret_before = count_in_log(node2, "INTERSERVER SECRET") + negotiated_before = count_in_log(node1, NEGOTIATED) + + assert ( + node1.query( + "SELECT count() FROM clusterAllReplicas('secret_cluster', system.one)" + " SETTINGS prefer_localhost_replica = 0" + ) + == "1\n" + ) + + assert count_in_log(node2, "INTERSERVER SECRET") > secret_before + assert count_in_log(node1, NEGOTIATED) > negotiated_before + + +def test_remote_table_function_sends_a_matching_client_info(started_cluster): + """`remote(host, )` is the one path that sends `query_kind = INITIAL_QUERY`, so + the Query packet carries its own `client_name`, which must equal the one the `Hello` sent or the + worker rejects the query with `CLIENT_INFO_DOES_NOT_MATCH`.""" + assert ( + node1.query( + "SELECT count() FROM remote('node2', numbers(10))" + " SETTINGS log_comment = 'antalya_marker_remote_function'" + ) + == "10\n" + ) + + node2.query("SYSTEM FLUSH LOGS query_log") + assert ( + node2.query( + "SELECT DISTINCT client_name FROM system.query_log" + " WHERE log_comment = 'antalya_marker_remote_function' AND type = 'QueryFinish'" + " AND is_initial_query" + ) + == "ClickHouse server\n" + ) + + +def test_client_name_in_query_log_is_untouched(started_cluster): + """Nothing marks `client_name` and the server never rewrites it, so what an end user's client + sent is what `system.query_log` stores.""" + node1.query("SELECT 1 SETTINGS log_comment = 'antalya_marker_client'") + node1.query("SYSTEM FLUSH LOGS query_log") + + assert ( + node1.query( + "SELECT DISTINCT client_name FROM system.query_log" + " WHERE log_comment = 'antalya_marker_client' AND type = 'QueryFinish'" + ) + == "ClickHouse client\n" + ) + + +def test_new_initiator_against_an_unmarked_worker(started_cluster): + """The rolling-upgrade direction. The worker predates the marker, so it neither strips nor + expects one; an initiator that marked its `Hello` would leave an Antalya string in the worker's + `client_name` and, with `validate_tcp_client_information`, fail the query outright.""" + assert ( + node1.query( + "SELECT count() FROM remote('node_old', numbers(10))" + " SETTINGS log_comment = 'antalya_marker_old_worker'" + ) + == "10\n" + ) + + node_old.query("SYSTEM FLUSH LOGS") + assert ( + node_old.query( + "SELECT DISTINCT client_name FROM system.query_log" + " WHERE log_comment = 'antalya_marker_old_worker' AND type = 'QueryFinish'" + " AND is_initial_query" + ) + == "ClickHouse server\n" + ) + + +def test_unmarked_initiator_against_a_marked_server(started_cluster): + """The other direction: an old client gets a marked `ServerHello` it does not know to strip. It + only ever displays that string, so the query must be unaffected.""" + assert node_old.query("SELECT count() FROM remote('node1', numbers(10))") == "10\n" diff --git a/tests/queries/0_stateless/05053_antalya_protocol_marker.reference b/tests/queries/0_stateless/05053_antalya_protocol_marker.reference new file mode 100644 index 000000000000..231e92f406e6 --- /dev/null +++ b/tests/queries/0_stateless/05053_antalya_protocol_marker.reference @@ -0,0 +1,3 @@ +10 +ClickHouse client +ClickHouse server diff --git a/tests/queries/0_stateless/05053_antalya_protocol_marker.sql b/tests/queries/0_stateless/05053_antalya_protocol_marker.sql new file mode 100644 index 000000000000..040e2229ca04 --- /dev/null +++ b/tests/queries/0_stateless/05053_antalya_protocol_marker.sql @@ -0,0 +1,33 @@ +-- Regression guard: no Antalya protocol string may reach `system.query_log` (see +-- `Core/AntalyaProtocol.h`). Only the `ServerHello` carries the marker, and the client strips it, so +-- `client_name` is exactly what an upstream server would store. +-- +-- `remote(host,
)` is the one path that sends `query_kind = INITIAL_QUERY`, so the +-- Query packet carries its own `client_name` and `validate_tcp_client_information` compares it +-- against the Hello's. Marking the Hello would fail this query with `CLIENT_INFO_DOES_NOT_MATCH`. + +SELECT count() FROM remote('127.0.0.2', numbers(10)) +SETTINGS log_queries = 1, log_comment = '05053_antalya_protocol_marker'; + +SYSTEM FLUSH LOGS query_log; + +-- The worker runs in `default` rather than in the test database, so its row is reached through the +-- initiating query's id. Filtering on `log_comment` alone would also pick up an earlier run of this +-- test against a different build, which is exactly what the upgrade check does. `IN` rather than a +-- scalar subquery: the worker's row is also `is_initial_query`, so a second match must narrow the +-- result rather than throw. +SELECT DISTINCT client_name +FROM system.query_log +WHERE event_date >= yesterday() + AND type = 'QueryFinish' + AND initial_query_id IN + ( + SELECT query_id + FROM system.query_log + WHERE event_date >= yesterday() + AND current_database = currentDatabase() + AND log_comment = '05053_antalya_protocol_marker' + AND type = 'QueryFinish' + AND is_initial_query + ) +ORDER BY 1; diff --git a/tests/queries/0_stateless/05054_antalya_protocol_server_hello.py b/tests/queries/0_stateless/05054_antalya_protocol_server_hello.py new file mode 100755 index 000000000000..2623ee89bfb8 --- /dev/null +++ b/tests/queries/0_stateless/05054_antalya_protocol_server_hello.py @@ -0,0 +1,127 @@ +#!/usr/bin/env python3 +# Tags: no-fasttest, no-llvm-coverage +"""The `server_name` an Antalya server puts in its Hello, read straight off the wire. + +`TCPHandler::sendHello` appends the marker to every `ServerHello`. It cannot do otherwise: the +client writes its own `Hello` first, so there is nothing about the peer for the server to condition +on, and the client `Hello` carries no marker to condition on either. Nothing the client sends may +change the reply. See `src/Core/AntalyaProtocol.h`. +""" + +import os +import re +import socket +import struct + +CLICKHOUSE_PORT = int(os.environ.get("CLICKHOUSE_PORT_TCP", 9000)) +CLICKHOUSE_HOST = os.environ.get("CLICKHOUSE_HOST", "127.0.0.1") + +CLIENT_HELLO = 0 +SERVER_HELLO = 0 +SERVER_EXCEPTION = 2 +CLIENT_REVISION = 54449 + +UPSTREAM_SERVER_NAME = "ClickHouse" +MARKED_SERVER_NAME = re.compile(r"^(.*) \(antalya:[1-9][0-9]{0,8}\)$") + +# Anything a client might put in the name field, including strings that already look marked and +# strings that nearly parse as one. None of it may change what comes back. +CLIENT_NAMES = [ + "ClickHouse client", + "ClickHouse server", + "ClickHouse test", + "", + "ClickHouse server (antalya:1)", + "ClickHouse server (antalya:999999999)", + "ClickHouse server (antalya:01)", + "ClickHouse server (ANTALYA:1)", +] + + +def write_varuint(value): + result = bytearray() + while value > 0x7F: + result.append(0x80 | (value & 0x7F)) + value >>= 7 + result.append(value & 0x7F) + return bytes(result) + + +def write_string(s): + data = s.encode("utf-8") + return write_varuint(len(data)) + data + + +def recv_exact(sock, n): + data = b"" + while len(data) < n: + chunk = sock.recv(n - len(data)) + if not chunk: + raise ConnectionError("Connection closed") + data += chunk + return data + + +def read_varuint(sock): + result = 0 + shift = 0 + while True: + byte = recv_exact(sock, 1)[0] + result |= (byte & 0x7F) << shift + if (byte & 0x80) == 0: + return result + shift += 7 + + +def read_string(sock): + length = read_varuint(sock) + return recv_exact(sock, length).decode("utf-8") if length else "" + + +def server_name_for(client_name): + """Send a Hello carrying `client_name` and return the `server_name` the server replies with.""" + with socket.create_connection((CLICKHOUSE_HOST, CLICKHOUSE_PORT), timeout=30) as sock: + pkt = bytearray() + pkt += write_varuint(CLIENT_HELLO) + pkt += write_string(client_name) + pkt += write_varuint(25) # version_major + pkt += write_varuint(1) # version_minor + pkt += write_varuint(CLIENT_REVISION) + pkt += write_string("") # default database + pkt += write_string("default") # user + pkt += write_string("") # password + sock.sendall(pkt) + + pkt_type = read_varuint(sock) + if pkt_type == SERVER_EXCEPTION: + code = struct.unpack(" Date: Wed, 16 Sep 2026 17:53:31 +0200 Subject: [PATCH 2/4] Document the Antalya protocol marker in the native protocol spec `docs/en/interfaces/specs/NativeProtocol.md` is the canonical description of the native protocol and the file a third-party client is built against, so the `ServerHello` `server_name` row now says that an Antalya build appends " (antalya:N)" and that the suffix may be ignored or stripped. That row is the whole delta against a file which tracks upstream through every rebase; the counter itself stays documented in `docs/en/antalya/protocol.md`, which also records when a future Antalya wire change must grow that delta - when it changes the layout of a packet the spec describes field by field, because a client that does not know about an added field cannot parse the stream past it. The same doc records why the advertisement has no opt-out. `server_name` is a build-time value any fork may set, so a client cannot treat it as fixed; `client_name` is the field that reaches `system.query_log` and `validate_tcp_client_information`, and it is never marked. A per-node switch would only add a state in which two Antalya nodes that both support a feature fail to negotiate it because one was configured not to advertise. --- docs/en/antalya/protocol.md | 30 ++++++++++++++++++++++ docs/en/interfaces/specs/NativeProtocol.md | 2 +- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/docs/en/antalya/protocol.md b/docs/en/antalya/protocol.md index 00817df0f762..b1f310fdc4bb 100644 --- a/docs/en/antalya/protocol.md +++ b/docs/en/antalya/protocol.md @@ -95,6 +95,36 @@ client does not strip. An upstream `clickhouse-client` prints `Connected to ClickHouse (antalya:1) server version ...` in its interactive banner. An Antalya client strips the marker and prints what upstream would have printed. +## No opt-out {#no-opt-out} + +There is no server setting that suppresses the marker. A node either speaks this protocol or is not +an Antalya build. + +`server_name` is already a build-time value - upstream sets it from the CMake project name, and any +fork may set it to anything - so a native client cannot treat it as a fixed string and must already +tolerate an arbitrary one. Nothing a server stores or compares reads it: `client_name` is the field +that reaches `system.query_log` and `validate_tcp_client_information`, and it is never marked. What +is left is the displayed name on a peer that does not strip the suffix, which is cosmetic. + +A per-node switch would buy nothing against that and would cost a state in which two Antalya nodes +that both support a feature silently fail to negotiate it, because one of them was configured not to +say so. If a peer is ever found that breaks on the suffix, the fix is to stop appending it - a bug to +fix once, not a knob for every operator to discover. + +## Relationship to the native protocol spec {#native-protocol-spec} + +`docs/en/interfaces/specs/NativeProtocol.md` is the canonical description of the *upstream* native +protocol, and it is the file a third-party client (`ch-go`, `clickhouse-go`, `clickhouse-driver`) is +built against. It tracks upstream through every rebase, so Antalya keeps its delta against that file +to the smallest thing that stops a client author from being surprised: one sentence on the +`ServerHello` `server_name` row, pointing here. Everything about the counter itself is documented in +this file. + +A future Antalya-only wire change documents itself here and leaves that delta as it is - unless it +changes the layout of a packet the spec describes field by field. A client that does not know about +an added field cannot parse the stream past it, so such a change must be described in the spec +itself, not only here. + ## Implementation {#implementation} `src/Core/AntalyaProtocol.h` holds the version constant, the marker grammar and the `appendMarker` diff --git a/docs/en/interfaces/specs/NativeProtocol.md b/docs/en/interfaces/specs/NativeProtocol.md index 34b9809b1328..02eb626f2598 100644 --- a/docs/en/interfaces/specs/NativeProtocol.md +++ b/docs/en/interfaces/specs/NativeProtocol.md @@ -440,7 +440,7 @@ Server → Client. The reply to ClientHello on successful authentication. | # | Field | Type | Role | Condition | Description | |---|------------------|---------|-----------|------------------------|-------------| -| 1 | server_name | String | universal | always | Server identifier | +| 1 | server_name | String | universal | always | Server identifier. An Altinity Antalya build appends `" (antalya:N)"`, where `N` is its Antalya protocol version; a client may ignore or strip the suffix. See [Antalya protocol version](/antalya/protocol). | | 2 | version_major | VarUInt | universal | always | Server major version | | 3 | version_minor | VarUInt | universal | always | Server minor version | | 4 | protocol_version | VarUInt | universal | always | Server's protocol version | From 81839e9d5c3e19a8050831005a826a84951496c2 Mon Sep 17 00:00:00 2001 From: Andrey Zvonov <32552679+zvonand@users.noreply.github.com> Date: Thu, 17 Sep 2026 16:00:40 +0200 Subject: [PATCH 3/4] Trim the comments and the redundant tests Keep only the comments that carry something the code does not: the bump rule for `DBMS_ANTALYA_PROTOCOL_VERSION`, the marker grammar, the bounded parse in `parseMarker`, and the sanitization ordering the client depends on. The rest repeated the code or `docs/en/antalya/protocol.md`. Drop the tests whose coverage is pinned elsewhere: - `05054_antalya_protocol_server_hello.py` hand-rolled a native client to read the `ServerHello` name, which `gtest_antalya_protocol.cpp` pins at the unit level and the integration test proves over a real connection. - `test_interserver_secret_negotiates`: `TCPHandler::sendHello` has no branches, so no kind of connection can skip the marker. - `test_remote_table_function_sends_a_matching_client_info` and `test_client_name_in_query_log_is_untouched` duplicated `05053_antalya_protocol_marker.sql`, which runs with `validate_tcp_client_information` enabled by `tests/config/install.sh`. - `test_distributed_cluster_negotiates` asserted the same log line through the same code as `test_remote_function_negotiates`. The integration module needs no server config now, so `configs/` goes with them. --- src/Client/Connection.cpp | 3 - ...gtest_sanitize_untrusted_server_string.cpp | 6 +- src/Core/AntalyaProtocol.cpp | 3 +- src/Core/AntalyaProtocol.h | 42 ++---- src/Core/Protocol.h | 4 +- src/Core/tests/gtest_antalya_protocol.cpp | 20 ++- src/Server/TCPHandler.cpp | 2 - .../configs/remote_servers.xml | 18 --- .../configs/validate_client_info.xml | 6 - .../integration/test_antalya_protocol/test.py | 98 ++------------ .../05053_antalya_protocol_marker.sql | 15 +-- .../05054_antalya_protocol_server_hello.py | 127 ------------------ ...54_antalya_protocol_server_hello.reference | 2 - 13 files changed, 38 insertions(+), 308 deletions(-) delete mode 100644 tests/integration/test_antalya_protocol/configs/remote_servers.xml delete mode 100644 tests/integration/test_antalya_protocol/configs/validate_client_info.xml delete mode 100755 tests/queries/0_stateless/05054_antalya_protocol_server_hello.py delete mode 100644 tests/queries/0_stateless/05054_antalya_protocol_server_hello.reference diff --git a/src/Client/Connection.cpp b/src/Client/Connection.cpp index ee0681625341..e732e9071ced 100644 --- a/src/Client/Connection.cpp +++ b/src/Client/Connection.cpp @@ -627,9 +627,6 @@ void Connection::receiveHello() { readStringBinary(server_name, *in, DBMS_MAX_HELLO_STRING_SIZE); sanitizeUntrustedServerString(server_name); - - /// The marker is stripped here, so `server_name` reads exactly as it would coming from an - /// upstream server. server_antalya_protocol_version = AntalyaProtocol::negotiate(AntalyaProtocol::stripMarker(server_name)); readVarUInt(server_version_major, *in); readVarUInt(server_version_minor, *in); diff --git a/src/Client/tests/gtest_sanitize_untrusted_server_string.cpp b/src/Client/tests/gtest_sanitize_untrusted_server_string.cpp index bfa285da8661..cad0530b94f2 100644 --- a/src/Client/tests/gtest_sanitize_untrusted_server_string.cpp +++ b/src/Client/tests/gtest_sanitize_untrusted_server_string.cpp @@ -159,8 +159,7 @@ TEST(SanitizeUntrustedServerString, MaxPasswordComplexityRulesCapIsTight) TEST(SanitizeUntrustedServerString, PreservesAnAntalyaProtocolMarker) { - /// The client parses `server_name` only after sanitizing it, so the marker must pass through - /// byte for byte. See `Core/AntalyaProtocol.h`. + /// The client parses the marker only after sanitizing, so it must pass through byte for byte. String name = AntalyaProtocol::appendMarker("ClickHouse"); const String before = name; sanitizeUntrustedServerString(name); @@ -170,8 +169,7 @@ TEST(SanitizeUntrustedServerString, PreservesAnAntalyaProtocolMarker) TEST(SanitizeUntrustedServerString, CannotForgeAnAntalyaProtocolMarker) { - /// Control bytes are replaced, never deleted, so a hostile `server_name` cannot be collapsed - /// into a canonical marker. Switching the helper to deletion would break this. + /// Control bytes are replaced, never deleted, so a hostile name cannot collapse into a marker. String hostile = "ClickHouse\x01 (antalya:\x02" "1)"; sanitizeUntrustedServerString(hostile); EXPECT_EQ(AntalyaProtocol::parseMarker(hostile), 0u); diff --git a/src/Core/AntalyaProtocol.cpp b/src/Core/AntalyaProtocol.cpp index 5c124299529c..f20948e07d98 100644 --- a/src/Core/AntalyaProtocol.cpp +++ b/src/Core/AntalyaProtocol.cpp @@ -12,7 +12,6 @@ namespace DB namespace AntalyaProtocol { -/// The largest version the grammar can spell: `MAX_MARKER_DIGITS` nines. constexpr UInt64 MAX_VERSION = intExp10(static_cast(MAX_MARKER_DIGITS)) - 1; static_assert( @@ -46,7 +45,7 @@ UInt64 parseMarker(std::string_view name) noexcept } const size_t digits = close - first_digit; - /// One version must have exactly one spelling on the wire: no empty run, no leading zero. + /// Exactly one spelling per version: no empty digit run, no leading zero. if (digits == 0 || name[first_digit] == '0') return 0; diff --git a/src/Core/AntalyaProtocol.h b/src/Core/AntalyaProtocol.h index 58fe69f5bff0..7f1c6a5044c8 100644 --- a/src/Core/AntalyaProtocol.h +++ b/src/Core/AntalyaProtocol.h @@ -8,31 +8,19 @@ namespace DB { -/// ============================== ANTALYA ONLY ============================== -/// One counter for all Antalya-only wire features, independent of every upstream counter in -/// `Core/ProtocolDefines.h`. It is advertised inside the `ServerHello` name string, which a client -/// only displays, so a rebase can never renumber an Antalya feature. -/// -/// Bump it by exactly one per feature and never reuse a value; never bump an upstream counter for -/// an Antalya-only change. The client caps the server's value with `min(own, server)`, so the -/// feature set must stay cumulative: a backport takes the whole contiguous range up to the value it -/// needs, or does not bump at all. See `docs/en/antalya/protocol.md`. -/// -/// Version 1: the advertisement itself. No wire payload beyond the marker. +/// Counter for Antalya-only wire features, independent of every upstream counter in +/// `Core/ProtocolDefines.h`, so that a rebase can never renumber an Antalya feature. Bump it by one +/// per feature. The client caps the server's value with `min(own, server)`, so the feature set must +/// stay cumulative: a backport takes the whole contiguous range up to the value it needs, or does +/// not bump at all. See `docs/en/antalya/protocol.md`. static constexpr auto DBMS_ANTALYA_PROTOCOL_VERSION = 1; -/// ========================================================================== namespace AntalyaProtocol { -/// The version travels inside the `ServerHello` name string, in one direction only: -/// -/// server -> client "ClickHouse (antalya:M)" (every connection, unconditionally) -/// -/// The client strips the marker while parsing, so it never reaches anything that stores or displays -/// the name. The client `Hello` is never marked: `client_name` is the one field a peer persists to -/// `system.query_log` and compares in `validateClientInfo`, and the client writes it before it -/// knows anything about the peer. See `docs/en/antalya/protocol.md`. +/// The version is advertised in the `ServerHello` name string - "ClickHouse (antalya:M)" - and +/// nowhere else: the client `Hello` is never marked, because `client_name` reaches the peer's +/// `system.query_log` and `validate_tcp_client_information`. See `docs/en/antalya/protocol.md`. /// " (antalya:" ")", where is [1-9][0-9]{0,8}. inline constexpr std::string_view MARKER_PREFIX = " (antalya:"; @@ -40,21 +28,17 @@ inline constexpr char MARKER_TERMINATOR = ')'; inline constexpr size_t MAX_MARKER_DIGITS = 9; inline constexpr size_t MAX_MARKER_SIZE = MARKER_PREFIX.size() + MAX_MARKER_DIGITS + 1; -/// Returns `name` with our own version's marker appended. String appendMarker(std::string_view name); -/// Strict, anchored suffix parse: returns 0 unless `name` ends with exactly one canonical marker. -/// Scans at most `MAX_MARKER_SIZE` bytes back from the end - the client runs this on a `server_name` -/// it has not authenticated, where an unbounded scan would be a CPU amplifier. +/// Returns 0 unless `name` ends with exactly one canonical marker. Scans at most `MAX_MARKER_SIZE` +/// bytes back from the end, because the client runs it on a `server_name` it has not authenticated. UInt64 parseMarker(std::string_view name) noexcept; -/// If `name` ends with a canonical marker, removes it and returns the version it spelled; otherwise -/// leaves `name` alone and returns 0. The marker is negotiation metadata rather than part of the -/// name, so the receiver strips it before the name is stored or displayed. +/// Removes a trailing canonical marker from `name` and returns the version it spelled; returns 0 and +/// leaves `name` alone when there is none. UInt64 stripMarker(String & name); -/// The version to speak with a server that advertised `peer_version`; 0 means the server is not -/// Antalya. +/// The version to speak with a server that advertised `peer_version`; 0 means it is not Antalya. UInt64 negotiate(UInt64 peer_version) noexcept; } diff --git a/src/Core/Protocol.h b/src/Core/Protocol.h index 1641e956a6f7..2f8dde808e6e 100644 --- a/src/Core/Protocol.h +++ b/src/Core/Protocol.h @@ -68,9 +68,7 @@ const char JWT_AUTHENTICAION_MARKER[] = " JWT AUTHENTICATION "; }; -/// `client_name` that `Cluster` passes to `Connection` for server-to-server connections -/// (Distributed, `*Cluster`, swarm, parallel replicas), reaching the Hello packet as -/// " server". The counterpart of `DEFAULT_CLIENT_NAME` ("client"). +/// `client_name` for server-to-server connections, the counterpart of `DEFAULT_CLIENT_NAME`. static constexpr std::string_view SERVER_CLIENT_NAME = "server"; namespace Protocol diff --git a/src/Core/tests/gtest_antalya_protocol.cpp b/src/Core/tests/gtest_antalya_protocol.cpp index 78938997f52d..a9bfefdc0fe4 100644 --- a/src/Core/tests/gtest_antalya_protocol.cpp +++ b/src/Core/tests/gtest_antalya_protocol.cpp @@ -24,15 +24,15 @@ TEST(AntalyaProtocol, ParseRejectsNonCanonicalMarkers) "", "ClickHouse server", "ClickHouse client", - "(antalya:1)", /// no base name, so no leading space either - "ClickHouse server(antalya:1)", /// missing the separating space - "ClickHouse server (antalya:1", /// unterminated - "ClickHouse server (antalya:)", /// no digits - "ClickHouse server (antalya:0)", /// zero is not a valid version - "ClickHouse server (antalya:01)", /// leading zero is not canonical - "ClickHouse server (antalya:1234567890)", /// 10 digits, above the cap - "ClickHouse server (antalya:1) v2", /// marker must be the suffix - "ClickHouse server (ANTALYA:1)", /// case sensitive + "(antalya:1)", + "ClickHouse server(antalya:1)", + "ClickHouse server (antalya:1", + "ClickHouse server (antalya:)", + "ClickHouse server (antalya:0)", + "ClickHouse server (antalya:01)", + "ClickHouse server (antalya:1234567890)", + "ClickHouse server (antalya:1) v2", + "ClickHouse server (ANTALYA:1)", "ClickHouse server (antalya:1x)", "ClickHouse server (antalya:1 )", "antalya:1)", @@ -45,7 +45,6 @@ TEST(AntalyaProtocol, ParseRejectsNonCanonicalMarkers) TEST(AntalyaProtocol, ParseTakesTheTrailingMarkerWhenRepeated) { - /// The scan is anchored at the end, so a doubled marker yields the last value. EXPECT_EQ(parseMarker("ClickHouse server (antalya:99) (antalya:1)"), 1u); } @@ -55,7 +54,6 @@ TEST(AntalyaProtocol, StripMarkerRemovesOnlyTheMarker) EXPECT_EQ(stripMarker(marked), 42u); EXPECT_EQ(marked, "ClickHouse server"); - /// The widest marker the grammar allows, to pin the digit arithmetic at both ends. String widest = "ClickHouse (antalya:999999999)"; EXPECT_EQ(stripMarker(widest), 999999999u); EXPECT_EQ(widest, "ClickHouse"); diff --git a/src/Server/TCPHandler.cpp b/src/Server/TCPHandler.cpp index dd1673669d11..c85dc0847448 100644 --- a/src/Server/TCPHandler.cpp +++ b/src/Server/TCPHandler.cpp @@ -2137,8 +2137,6 @@ void TCPHandler::processUnexpectedHello() void TCPHandler::sendHello() { writeVarUInt(Protocol::Server::Hello, *out); - /// Unconditional: the client writes its Hello first, so the server is the only side that can - /// advertise without already knowing what the peer is. See `docs/en/antalya/protocol.md`. writeStringBinary(AntalyaProtocol::appendMarker(VERSION_NAME), *out); writeVarUInt(VERSION_MAJOR, *out); writeVarUInt(VERSION_MINOR, *out); diff --git a/tests/integration/test_antalya_protocol/configs/remote_servers.xml b/tests/integration/test_antalya_protocol/configs/remote_servers.xml deleted file mode 100644 index 00754e56310a..000000000000 --- a/tests/integration/test_antalya_protocol/configs/remote_servers.xml +++ /dev/null @@ -1,18 +0,0 @@ - - - - - - node29000 - - - - - antalya_secret - - node29000 - - - - diff --git a/tests/integration/test_antalya_protocol/configs/validate_client_info.xml b/tests/integration/test_antalya_protocol/configs/validate_client_info.xml deleted file mode 100644 index 2042b7f4a861..000000000000 --- a/tests/integration/test_antalya_protocol/configs/validate_client_info.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - true - diff --git a/tests/integration/test_antalya_protocol/test.py b/tests/integration/test_antalya_protocol/test.py index 0bedc6b5d9c9..8f43bdb89667 100644 --- a/tests/integration/test_antalya_protocol/test.py +++ b/tests/integration/test_antalya_protocol/test.py @@ -1,13 +1,7 @@ """Antalya protocol version negotiation over the native TCP handshake. -Covers what a stateless test cannot express: a cluster in `remote_servers` against a separate host, -one authenticated with a ``, and a peer built before the marker existed. See -`src/Core/AntalyaProtocol.h`. - -Only the side that opened the connection logs the negotiated version, because only the server -advertises and only the client reads. The log assertions count occurrences before and after the -query because the cluster fixture is module-scoped, so a plain substring check would pass on an -earlier test's output. +The log assertions count occurrences before and after the query because the cluster fixture is +module-scoped, so a plain substring check would pass on an earlier test's output. """ import pytest @@ -16,12 +10,10 @@ cluster = ClickHouseCluster(__file__) -MAIN_CONFIGS = ["configs/remote_servers.xml", "configs/validate_client_info.xml"] - -node1 = cluster.add_instance("node1", main_configs=MAIN_CONFIGS) -node2 = cluster.add_instance("node2", main_configs=MAIN_CONFIGS) +node1 = cluster.add_instance("node1") +node2 = cluster.add_instance("node2") # A build from before the marker existed: it sends an unmarked `ServerHello` and does not know to -# strip one. Nothing in the handshake may depend on the peer being Antalya. +# strip one. node_old = cluster.add_instance( "node_old", image="altinity/clickhouse-server", @@ -46,8 +38,7 @@ def count_in_log(node, substring): def test_remote_function_negotiates(started_cluster): - """The initiator reads the version off the marked `ServerHello` it gets back. The worker learns - nothing about the initiator, so only the initiator's log carries the line.""" + """Only the initiator logs the version: the worker learns nothing about the peer.""" initiator_before = count_in_log(node1, NEGOTIATED) worker_before = count_in_log(node2, NEGOTIATED) @@ -57,80 +48,8 @@ def test_remote_function_negotiates(started_cluster): assert count_in_log(node2, NEGOTIATED) == worker_before -def test_distributed_cluster_negotiates(started_cluster): - negotiated_before = count_in_log(node1, NEGOTIATED) - - assert ( - node1.query( - "SELECT count() FROM clusterAllReplicas('plain_cluster', system.one)" - " SETTINGS prefer_localhost_replica = 0" - ) - == "1\n" - ) - - assert count_in_log(node1, NEGOTIATED) > negotiated_before - - -def test_interserver_secret_negotiates(started_cluster): - """The worker returns early from `receiveHello` for an interserver-secret connection, but - `sendHello` still runs, so the initiator still gets a marked `ServerHello`.""" - secret_before = count_in_log(node2, "INTERSERVER SECRET") - negotiated_before = count_in_log(node1, NEGOTIATED) - - assert ( - node1.query( - "SELECT count() FROM clusterAllReplicas('secret_cluster', system.one)" - " SETTINGS prefer_localhost_replica = 0" - ) - == "1\n" - ) - - assert count_in_log(node2, "INTERSERVER SECRET") > secret_before - assert count_in_log(node1, NEGOTIATED) > negotiated_before - - -def test_remote_table_function_sends_a_matching_client_info(started_cluster): - """`remote(host,
)` is the one path that sends `query_kind = INITIAL_QUERY`, so - the Query packet carries its own `client_name`, which must equal the one the `Hello` sent or the - worker rejects the query with `CLIENT_INFO_DOES_NOT_MATCH`.""" - assert ( - node1.query( - "SELECT count() FROM remote('node2', numbers(10))" - " SETTINGS log_comment = 'antalya_marker_remote_function'" - ) - == "10\n" - ) - - node2.query("SYSTEM FLUSH LOGS query_log") - assert ( - node2.query( - "SELECT DISTINCT client_name FROM system.query_log" - " WHERE log_comment = 'antalya_marker_remote_function' AND type = 'QueryFinish'" - " AND is_initial_query" - ) - == "ClickHouse server\n" - ) - - -def test_client_name_in_query_log_is_untouched(started_cluster): - """Nothing marks `client_name` and the server never rewrites it, so what an end user's client - sent is what `system.query_log` stores.""" - node1.query("SELECT 1 SETTINGS log_comment = 'antalya_marker_client'") - node1.query("SYSTEM FLUSH LOGS query_log") - - assert ( - node1.query( - "SELECT DISTINCT client_name FROM system.query_log" - " WHERE log_comment = 'antalya_marker_client' AND type = 'QueryFinish'" - ) - == "ClickHouse client\n" - ) - - def test_new_initiator_against_an_unmarked_worker(started_cluster): - """The rolling-upgrade direction. The worker predates the marker, so it neither strips nor - expects one; an initiator that marked its `Hello` would leave an Antalya string in the worker's - `client_name` and, with `validate_tcp_client_information`, fail the query outright.""" + """A worker that predates the marker neither strips nor expects one.""" assert ( node1.query( "SELECT count() FROM remote('node_old', numbers(10))" @@ -151,6 +70,5 @@ def test_new_initiator_against_an_unmarked_worker(started_cluster): def test_unmarked_initiator_against_a_marked_server(started_cluster): - """The other direction: an old client gets a marked `ServerHello` it does not know to strip. It - only ever displays that string, so the query must be unaffected.""" + """An old client gets a marked `ServerHello` it does not know to strip, and only displays it.""" assert node_old.query("SELECT count() FROM remote('node1', numbers(10))") == "10\n" diff --git a/tests/queries/0_stateless/05053_antalya_protocol_marker.sql b/tests/queries/0_stateless/05053_antalya_protocol_marker.sql index 040e2229ca04..df3a18612973 100644 --- a/tests/queries/0_stateless/05053_antalya_protocol_marker.sql +++ b/tests/queries/0_stateless/05053_antalya_protocol_marker.sql @@ -1,10 +1,5 @@ --- Regression guard: no Antalya protocol string may reach `system.query_log` (see --- `Core/AntalyaProtocol.h`). Only the `ServerHello` carries the marker, and the client strips it, so --- `client_name` is exactly what an upstream server would store. --- --- `remote(host,
)` is the one path that sends `query_kind = INITIAL_QUERY`, so the --- Query packet carries its own `client_name` and `validate_tcp_client_information` compares it --- against the Hello's. Marking the Hello would fail this query with `CLIENT_INFO_DOES_NOT_MATCH`. +-- No Antalya protocol string may reach `system.query_log`: only the `ServerHello` carries the +-- marker, and the client strips it. See `Core/AntalyaProtocol.h`. SELECT count() FROM remote('127.0.0.2', numbers(10)) SETTINGS log_queries = 1, log_comment = '05053_antalya_protocol_marker'; @@ -12,10 +7,8 @@ SETTINGS log_queries = 1, log_comment = '05053_antalya_protocol_marker'; SYSTEM FLUSH LOGS query_log; -- The worker runs in `default` rather than in the test database, so its row is reached through the --- initiating query's id. Filtering on `log_comment` alone would also pick up an earlier run of this --- test against a different build, which is exactly what the upgrade check does. `IN` rather than a --- scalar subquery: the worker's row is also `is_initial_query`, so a second match must narrow the --- result rather than throw. +-- initiating query's id. `IN` rather than a scalar subquery: the worker's row is also +-- `is_initial_query`, so a second match must narrow the result rather than throw. SELECT DISTINCT client_name FROM system.query_log WHERE event_date >= yesterday() diff --git a/tests/queries/0_stateless/05054_antalya_protocol_server_hello.py b/tests/queries/0_stateless/05054_antalya_protocol_server_hello.py deleted file mode 100755 index 2623ee89bfb8..000000000000 --- a/tests/queries/0_stateless/05054_antalya_protocol_server_hello.py +++ /dev/null @@ -1,127 +0,0 @@ -#!/usr/bin/env python3 -# Tags: no-fasttest, no-llvm-coverage -"""The `server_name` an Antalya server puts in its Hello, read straight off the wire. - -`TCPHandler::sendHello` appends the marker to every `ServerHello`. It cannot do otherwise: the -client writes its own `Hello` first, so there is nothing about the peer for the server to condition -on, and the client `Hello` carries no marker to condition on either. Nothing the client sends may -change the reply. See `src/Core/AntalyaProtocol.h`. -""" - -import os -import re -import socket -import struct - -CLICKHOUSE_PORT = int(os.environ.get("CLICKHOUSE_PORT_TCP", 9000)) -CLICKHOUSE_HOST = os.environ.get("CLICKHOUSE_HOST", "127.0.0.1") - -CLIENT_HELLO = 0 -SERVER_HELLO = 0 -SERVER_EXCEPTION = 2 -CLIENT_REVISION = 54449 - -UPSTREAM_SERVER_NAME = "ClickHouse" -MARKED_SERVER_NAME = re.compile(r"^(.*) \(antalya:[1-9][0-9]{0,8}\)$") - -# Anything a client might put in the name field, including strings that already look marked and -# strings that nearly parse as one. None of it may change what comes back. -CLIENT_NAMES = [ - "ClickHouse client", - "ClickHouse server", - "ClickHouse test", - "", - "ClickHouse server (antalya:1)", - "ClickHouse server (antalya:999999999)", - "ClickHouse server (antalya:01)", - "ClickHouse server (ANTALYA:1)", -] - - -def write_varuint(value): - result = bytearray() - while value > 0x7F: - result.append(0x80 | (value & 0x7F)) - value >>= 7 - result.append(value & 0x7F) - return bytes(result) - - -def write_string(s): - data = s.encode("utf-8") - return write_varuint(len(data)) + data - - -def recv_exact(sock, n): - data = b"" - while len(data) < n: - chunk = sock.recv(n - len(data)) - if not chunk: - raise ConnectionError("Connection closed") - data += chunk - return data - - -def read_varuint(sock): - result = 0 - shift = 0 - while True: - byte = recv_exact(sock, 1)[0] - result |= (byte & 0x7F) << shift - if (byte & 0x80) == 0: - return result - shift += 7 - - -def read_string(sock): - length = read_varuint(sock) - return recv_exact(sock, length).decode("utf-8") if length else "" - - -def server_name_for(client_name): - """Send a Hello carrying `client_name` and return the `server_name` the server replies with.""" - with socket.create_connection((CLICKHOUSE_HOST, CLICKHOUSE_PORT), timeout=30) as sock: - pkt = bytearray() - pkt += write_varuint(CLIENT_HELLO) - pkt += write_string(client_name) - pkt += write_varuint(25) # version_major - pkt += write_varuint(1) # version_minor - pkt += write_varuint(CLIENT_REVISION) - pkt += write_string("") # default database - pkt += write_string("default") # user - pkt += write_string("") # password - sock.sendall(pkt) - - pkt_type = read_varuint(sock) - if pkt_type == SERVER_EXCEPTION: - code = struct.unpack(" Date: Thu, 17 Sep 2026 16:30:38 +0200 Subject: [PATCH 4/4] Tighten the Antalya protocol doc The same facts were stated up to three times: that the client `Hello` is never marked, that only the server advertises, that the client caps with `min(own, server)`. State each once, lead with the wire form, and turn the rules for a future wire change into a checklist. Drop the `Scope` section, whose only unique sentence - negotiation is per hop - moved into the intro, and the paragraph describing a client-to-server channel that does not exist yet. --- docs/en/antalya/protocol.md | 155 +++++++++++++----------------------- 1 file changed, 56 insertions(+), 99 deletions(-) diff --git a/docs/en/antalya/protocol.md b/docs/en/antalya/protocol.md index b1f310fdc4bb..78065bb47afc 100644 --- a/docs/en/antalya/protocol.md +++ b/docs/en/antalya/protocol.md @@ -9,126 +9,83 @@ doc_type: 'reference' # Antalya protocol version {#antalya-protocol-version} -## Why a separate counter {#why-a-separate-counter} - -Antalya carries features that do not exist in upstream ClickHouse, and it rebases onto each new -upstream release. An Antalya-only wire change that takes a slot in an upstream counter - -`DBMS_TCP_PROTOCOL_VERSION` or `DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION` - has to be renumbered -whenever upstream claims that slot for something else. The same number then means two different -things in two shipped builds, and two nodes that negotiate it disagree about the bytes on the wire. - -`DBMS_ANTALYA_PROTOCOL_VERSION` (in `src/Core/AntalyaProtocol.h`) is a counter in a number space -that upstream cannot reach, so a rebase can never renumber an Antalya feature. It lives in its own -header rather than in `src/Core/ProtocolDefines.h`, whose tail is where upstream adds its own -constants and therefore where every rebase conflicts. - -## The rule {#the-rule} - -An Antalya-only wire change bumps `DBMS_ANTALYA_PROTOCOL_VERSION` and gates itself on the -negotiated value. It never bumps an upstream counter and never inserts a slot into -`DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION`. - -The client caps the server's value with `min(own, server)`, which is only sound while the counter -describes a cumulative feature set. A backport must therefore take the whole contiguous range up to -the value it needs, or not bump at all. - -## How it is advertised {#how-it-is-advertised} - -The version travels inside the `ServerHello` *name* string, in one direction only: +Antalya versions its own wire-protocol changes with `DBMS_ANTALYA_PROTOCOL_VERSION`, a counter that +upstream ClickHouse cannot reach, defined in `src/Core/AntalyaProtocol.h`. A server advertises it in +the `ServerHello` name string, on every connection: ```text -server -> client "ClickHouse (antalya:M)" (every connection, unconditionally) +server -> client "ClickHouse (antalya:1)" ``` -The client caps it with its own value and strips the marker while parsing, so `server_name` reads -exactly as it would coming from upstream. A value of `0` means the server is not Antalya. +The client strips the suffix, caps the value with `min(own, server)` and keeps the result. `0` means +the peer is not an Antalya build. Negotiation is per hop and not transitive: initiator to worker and +worker to worker negotiate independently. + +Version 1 is the advertisement itself. Nothing is gated on it yet. + +## Adding an Antalya-only wire change {#adding-a-wire-change} -Nothing is appended in the other direction, and the client `Hello` is never marked. The client -writes that packet before it has read a byte from the peer, so it cannot gate a marker on what the -peer is, and every field in it - `client_name`, `default_database`, `user`, `password` - is one an -upstream server acts on. `client_name` is the worst of them: a server persists it and -`validate_tcp_client_information` compares it against the Query packet's `ClientInfo`, so a marker -there fails a `remote()` query with `CLIENT_INFO_DOES_NOT_MATCH` on any peer that does not strip it. -The `ServerHello` has no such field - `server_name` is client-side display text and reaches no -system table - which is why the server is the side that speaks. +- Bump `DBMS_ANTALYA_PROTOCOL_VERSION` by one and gate the change on the negotiated value. +- Never bump `DBMS_TCP_PROTOCOL_VERSION`, and never take a slot in + `DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION` for a feature upstream does not have. +- Keep the counter cumulative. A backport takes the whole contiguous range up to the value it needs, + or does not bump at all - the `min(own, server)` cap is only sound for a cumulative feature set. +- Gate only what the *client* decides to do. The server never learns the client's version, because + only the server advertises. +- Document it here. Also document it in `docs/en/interfaces/specs/NativeProtocol.md` if it changes + the layout of a packet that file describes field by field: a third-party client (`ch-go`, + `clickhouse-go`, `clickhouse-driver`) that does not know an added field cannot parse the stream + past it. Everything else stays here, so that the spec keeps tracking upstream through rebases. -The marker lives inside an existing string rather than in appended bytes because neither Hello nor -the Addendum has a length prefix or a terminator: a reader stops after the last field its negotiated -revision knows about, and a peer that does not expect extra bytes reads them as the next packet. +## Why a counter of our own {#why-a-counter-of-our-own} -## What version 1 supports {#what-version-1-supports} +An Antalya-only change that takes a slot in an upstream counter has to be renumbered whenever +upstream claims that slot for something else. The same number then means two different things in two +shipped builds, and two nodes that negotiate it disagree about the bytes on the wire. -Version 1 is the advertisement itself; there is no wire payload beyond the marker. Because only the -server advertises, a feature gated on this version can only be one the *client* decides to use. +The constant lives in its own header rather than in `src/Core/ProtocolDefines.h`, whose tail is +where upstream adds its own constants, and therefore where every rebase conflicts. -A feature that needs the server to know the client's version needs a channel the client writes after -it has read the `ServerHello` - by then it knows the peer is Antalya - and that an upstream server -would never read. The Addendum is not that channel: `TCPHandler::receiveAddendum` reads a fixed -field list, and an Antalya server cannot tell a client that wrote an extra field from one that did -not, so it would read bytes that are not there and desynchronise the stream. A new Antalya-only -client packet type, numbered far above `Protocol::Client::MAX` and sent right after the Addendum, -is: an upstream client never sends it, and an Antalya client sends it only when the negotiated -version is at least the one that introduced it. That costs a version bump when it is first needed -and nothing today. +## Why the marker rides in `ServerHello` {#why-the-marker-rides-in-serverhello} -## Scope {#scope} +**The client `Hello` is never marked.** The client writes that packet before it has read a byte from +the peer, so it cannot gate a marker on what the peer is. Every field in it - `client_name`, +`default_database`, `user`, `password` - is one an upstream server acts on. `client_name` is the +worst: a server persists it, and `validate_tcp_client_information` compares it against the Query +packet's `ClientInfo`, so a marker there fails a `remote()` query with `CLIENT_INFO_DOES_NOT_MATCH` +on any peer that does not strip it. -Every connection to an Antalya server is advertised to, `clickhouse-client` included. Any Antalya -client reads the value - `clickhouse-client`, Distributed, `*Cluster`, swarm, parallel replicas - -and an upstream client ignores it. +`ServerHello` has no such field. `server_name` is client-side display text and reaches no system +table, which is why the server is the side that speaks. -Negotiation is per hop and not transitive: initiator to worker and worker to worker negotiate -independently. +**The marker is a suffix, not extra bytes.** Neither Hello nor the Addendum has a length prefix or a +terminator: a reader stops after the last field its negotiated revision knows about, so a peer that +does not expect extra bytes reads them as the next packet. -## Observability {#observability} +## What you see {#what-you-see} -The negotiated version, when non-zero, is appended to the client's existing connection log line -(`Connected to ... server version ...`). The server logs nothing about it, because it learns nothing -about the peer. Degradation to `0` is otherwise silent. +An Antalya client appends the negotiated version to its existing connection log line +(`Connected to ... server version ...`). The server logs nothing, and degradation to `0` is silent. -No Antalya string reaches anything a server stores. `client_name` is never marked, so -`system.query_log` and `system.processes` are unaffected and filters written as -`client_name = 'ClickHouse server'` keep working. +Nothing a server stores changes: `system.query_log` and `system.processes` keep the `client_name` +the peer sent, so filters written as `client_name = 'ClickHouse server'` keep working. -The marker is visible in exactly one place: an **upstream** client's `server_name`, which that -client does not strip. An upstream `clickhouse-client` prints -`Connected to ClickHouse (antalya:1) server version ...` in its interactive banner. An Antalya -client strips the marker and prints what upstream would have printed. +The suffix is visible in one place - an **upstream** client's `server_name`, which that client does +not strip, so its banner reads `Connected to ClickHouse (antalya:1) server version ...`. ## No opt-out {#no-opt-out} -There is no server setting that suppresses the marker. A node either speaks this protocol or is not -an Antalya build. +No setting suppresses the marker: a node either speaks this protocol or is not an Antalya build. `server_name` is already a build-time value - upstream sets it from the CMake project name, and any -fork may set it to anything - so a native client cannot treat it as a fixed string and must already -tolerate an arbitrary one. Nothing a server stores or compares reads it: `client_name` is the field -that reaches `system.query_log` and `validate_tcp_client_information`, and it is never marked. What -is left is the displayed name on a peer that does not strip the suffix, which is cosmetic. - -A per-node switch would buy nothing against that and would cost a state in which two Antalya nodes -that both support a feature silently fail to negotiate it, because one of them was configured not to -say so. If a peer is ever found that breaks on the suffix, the fix is to stop appending it - a bug to -fix once, not a knob for every operator to discover. - -## Relationship to the native protocol spec {#native-protocol-spec} - -`docs/en/interfaces/specs/NativeProtocol.md` is the canonical description of the *upstream* native -protocol, and it is the file a third-party client (`ch-go`, `clickhouse-go`, `clickhouse-driver`) is -built against. It tracks upstream through every rebase, so Antalya keeps its delta against that file -to the smallest thing that stops a client author from being surprised: one sentence on the -`ServerHello` `server_name` row, pointing here. Everything about the counter itself is documented in -this file. - -A future Antalya-only wire change documents itself here and leaves that delta as it is - unless it -changes the layout of a packet the spec describes field by field. A client that does not know about -an added field cannot parse the stream past it, so such a change must be described in the spec -itself, not only here. +fork may set it to anything - so a native client cannot treat it as a fixed string. What a switch +would protect is the displayed name on a peer that does not strip the suffix, which is cosmetic, and +it would cost a state in which two Antalya nodes that both support a feature fail to negotiate it +because one was configured not to say so. If a peer is ever found that breaks on the suffix, the fix +is to stop appending it. ## Implementation {#implementation} -`src/Core/AntalyaProtocol.h` holds the version constant, the marker grammar and the `appendMarker` -/ `parseMarker` / `stripMarker` / `negotiate` helpers. The parse is an anchored suffix scan bounded -to the marker length, because the client runs it on a `server_name` it has not authenticated. -`TCPHandler::sendHello` appends the marker; `Connection::receiveHello` strips it and stores the -capped value. +`src/Core/AntalyaProtocol.h` holds the version constant, the marker grammar and the `appendMarker` / +`parseMarker` / `stripMarker` / `negotiate` helpers. `TCPHandler::sendHello` appends the marker; +`Connection::receiveHello` strips it and stores the capped value.