From 04a250da44d800bea400b79717bb49f116f65c19 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sun, 18 Oct 2026 11:33:03 -0400 Subject: [PATCH] Zero ServerInfo fields missing from a server Hello ServerInfo had no default member initializers, and Client::Impl::ReceiveHello reads timezone, display_name and version_patch only when the server revision carries them. A server below revision 54401 left version_patch indeterminate, and reconnecting to an older server kept the previous server's values. Default-initialize the numeric members and reset server_info_ before parsing each Hello. --- clickhouse/client.cpp | 2 ++ clickhouse/client.h | 8 +++--- ut/client_protocol_ut.cpp | 59 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index 531498c0..1013fb3e 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -1204,6 +1204,8 @@ bool Client::Impl::ReceiveHello() { } if (packet_type == ServerCodes::Hello) { + server_info_ = ServerInfo{}; + if (!WireFormat::ReadString(*input_, &server_info_.name)) { return false; } diff --git a/clickhouse/client.h b/clickhouse/client.h index dfe31536..3f3f1228 100644 --- a/clickhouse/client.h +++ b/clickhouse/client.h @@ -37,10 +37,10 @@ struct ServerInfo { std::string name; std::string timezone; std::string display_name; - uint64_t version_major; - uint64_t version_minor; - uint64_t version_patch; - uint64_t revision; + uint64_t version_major = 0; + uint64_t version_minor = 0; + uint64_t version_patch = 0; + uint64_t revision = 0; }; /// Methods of block compression. diff --git a/ut/client_protocol_ut.cpp b/ut/client_protocol_ut.cpp index 72cfdd5d..b433cce3 100644 --- a/ut/client_protocol_ut.cpp +++ b/ut/client_protocol_ut.cpp @@ -50,6 +50,21 @@ class ScriptedSocketFactory : public SocketFactory { std::vector script_; }; +/// Serves the next script on each connect, repeating the last one. +class SequencedSocketFactory : public SocketFactory { +public: + explicit SequencedSocketFactory(std::vector> scripts) : scripts_(std::move(scripts)) {} + + std::unique_ptr connect(const ClientOptions&, const Endpoint&) override { + const size_t index = next_ < scripts_.size() - 1 ? next_++ : next_; + return std::make_unique(scripts_[index]); + } + +private: + const std::vector> scripts_; + size_t next_ = 0; +}; + ClientOptions ScriptedClientOptions() { return ClientOptions() .SetHost("scripted.test") @@ -68,6 +83,19 @@ const std::vector kServerHello = { 0xD0, 0x86, 0x03, // revision = 50000 (varint) }; +/// A ServerHello at revision 54401 (DBMS_MIN_REVISION_WITH_VERSION_PATCH), +/// the first revision that carries every ServerInfo field. +const std::vector kServerHelloWithPatch = { + 0x00, // packet type: ServerCodes::Hello (varint) + 0x02, 0x43, 0x48, // server name: "CH" (string, len 2) + 0x02, // version_major = 2 (varint) + 0x03, // version_minor = 3 (varint) + 0x81, 0xA9, 0x03, // revision = 54401 (varint) + 0x03, 0x55, 0x54, 0x43, // timezone: "UTC" (string, len 3) + 0x01, 0x64, // display_name: "d" (string, len 1) + 0x07, // version_patch = 7 (varint) +}; + /// An exception packet with a valid code but a corrupt `name` string. const std::vector kCorruptExceptionPacket = { 0x02, // packet type: ServerCodes::Exception (varint) @@ -238,3 +266,34 @@ TEST(ClientProtocol, WellFormedSelectResponseSucceeds) { })); EXPECT_EQ(blocks, 1u); } + +TEST(ClientProtocol, OldRevisionServerHelloLeavesMissingFieldsZero) { + Client client(ScriptedClientOptions(), std::make_unique(kServerHello)); + + const ServerInfo& info = client.GetServerInfo(); + EXPECT_EQ(info.name, "CH"); + EXPECT_EQ(info.version_major, 1u); + EXPECT_EQ(info.version_minor, 1u); + EXPECT_EQ(info.revision, 50000u); + EXPECT_EQ(info.timezone, ""); + EXPECT_EQ(info.display_name, ""); + EXPECT_EQ(info.version_patch, 0u); +} + +TEST(ClientProtocol, ReconnectToOldRevisionServerClearsPreviousServerInfo) { + Client client(ScriptedClientOptions(), + std::make_unique(std::vector>{kServerHelloWithPatch, kServerHello})); + + EXPECT_EQ(client.GetServerInfo().timezone, "UTC"); + EXPECT_EQ(client.GetServerInfo().display_name, "d"); + EXPECT_EQ(client.GetServerInfo().version_patch, 7u); + + client.ResetConnection(); + + const ServerInfo& info = client.GetServerInfo(); + EXPECT_EQ(info.name, "CH"); + EXPECT_EQ(info.revision, 50000u); + EXPECT_EQ(info.timezone, ""); + EXPECT_EQ(info.display_name, ""); + EXPECT_EQ(info.version_patch, 0u); +}