diff --git a/src/Common/ClickHouseVersion.cpp b/src/Common/ClickHouseVersion.cpp index dd1d4d0fbb3c..e0b66ccd01f5 100644 --- a/src/Common/ClickHouseVersion.cpp +++ b/src/Common/ClickHouseVersion.cpp @@ -4,7 +4,6 @@ #include #include -#include #include @@ -30,12 +29,13 @@ ClickHouseVersion::ClickHouseVersion(std::string_view version) ReadBufferFromString buf(split[i]); if (!tryReadIntText(component, buf) || !buf.eof()) { - /// Non-numeric component (e.g. "altinityantalya"): treat this and remaining parts as suffix. - /// Valid version must have at least one numeric component (e.g. "26.1.3.20001.altinityantalya"). - if (components.empty()) + /// The first non-numeric token is treated as a build suffix (e.g. "altinityantalya" + /// in "26.1.3.20001.altinityantalya"). To prevent silently accepting malformed versions + /// like "26..1", "26.1." or "26.x.1", the suffix must be a single, non-empty, trailing + /// token, preceded by at least one numeric component. + if (components.empty() || i + 1 != split.size() || split[i].empty()) throw Exception{ErrorCodes::BAD_ARGUMENTS, "Cannot parse ClickHouse version here: {}", version}; - Strings suffix_parts(split.begin() + i, split.end()); - suffix = boost::algorithm::join(suffix_parts, "."); + suffix = split[i]; break; } components.push_back(component); diff --git a/src/Common/tests/gtest_ch_version.cpp b/src/Common/tests/gtest_ch_version.cpp new file mode 100644 index 000000000000..b11f79b25b56 --- /dev/null +++ b/src/Common/tests/gtest_ch_version.cpp @@ -0,0 +1,73 @@ +#include +#include +#include +#include + +using namespace DB; + +/// Canonical, well-formed versions must parse and round-trip through toString() unchanged. +TEST(ClickHouseVersion, ParsesValidVersions) +{ + EXPECT_EQ(ClickHouseVersion("26").toString(), "26"); /// single numeric component + EXPECT_EQ(ClickHouseVersion("26.1").toString(), "26.1"); /// short two-component form + EXPECT_EQ(ClickHouseVersion("25.3").toString(), "25.3"); /// real-world `compatibility` value + EXPECT_EQ(ClickHouseVersion("26.3.1.20001").toString(), "26.3.1.20001"); /// full upstream-shaped version + EXPECT_EQ(ClickHouseVersion("26.1.3.20001.altinityantalya").toString(), + "26.1.3.20001.altinityantalya"); /// canonical Antalya build string +} + +/// The suffix handling must stay generic: any single trailing non-numeric token is a valid +/// suffix, no matter its text or how many numeric components precede it. +TEST(ClickHouseVersion, ParsesSuffixedVariants) +{ + EXPECT_EQ(ClickHouseVersion("26.3.1.20001.altinitystable").toString(), + "26.3.1.20001.altinitystable"); /// different suffix text still accepted + EXPECT_EQ(ClickHouseVersion("26.3.1.20001.altinityfips").toString(), + "26.3.1.20001.altinityfips"); /// another suffix variant + EXPECT_EQ(ClickHouseVersion("26.1.altinityantalya").toString(), + "26.1.altinityantalya"); /// suffix after only two components (non-canonical, still valid) + EXPECT_EQ(ClickHouseVersion("26.altinityantalya").toString(), + "26.altinityantalya"); /// suffix after a single component +} + +/// Malformed inputs must fail fast instead of silently parsing to a different version +TEST(ClickHouseVersion, RejectsMalformedVersions) +{ + EXPECT_THROW(ClickHouseVersion("26..1"), Exception); /// empty middle component + EXPECT_THROW(ClickHouseVersion("26.1."), Exception); /// trailing dot -> empty trailing token + EXPECT_THROW(ClickHouseVersion("26.x.1"), Exception); /// non-numeric token that is not the last token + + EXPECT_THROW(ClickHouseVersion(""), Exception); /// empty string + EXPECT_THROW(ClickHouseVersion("."), Exception); /// lone separator, no numeric component + EXPECT_THROW(ClickHouseVersion(".26"), Exception); /// leading dot -> empty first component + EXPECT_THROW(ClickHouseVersion("26.."), Exception); /// two empty trailing tokens + EXPECT_THROW(ClickHouseVersion("26.3.1.20001."), Exception);/// trailing dot after a full version + + EXPECT_THROW(ClickHouseVersion("altinityantalya"), Exception); /// suffix with no numeric component at all + EXPECT_THROW(ClickHouseVersion("26.altinityantalya.1"), Exception); /// suffix is not the last token + EXPECT_THROW(ClickHouseVersion("26.3.1.20001.altinityantalya.1"), Exception); /// junk token after an otherwise-valid suffix + + EXPECT_THROW(ClickHouseVersion("v26.1"), Exception); /// alphabetic prefix where a number is required +} + +/// Ordering compares numeric components first (element by element, shorter list is smaller), +/// then breaks ties on the suffix string lexicographically. +TEST(ClickHouseVersion, ComparesByComponentsThenSuffix) +{ + EXPECT_TRUE(ClickHouseVersion("26.1") < ClickHouseVersion("26.2")); + EXPECT_TRUE(ClickHouseVersion("25.8") < ClickHouseVersion("26.1")); + + /// 26.1 < 26.1.0 (a trailing .0 makes it "greater" -- can be surprising, so pin it) + EXPECT_TRUE(ClickHouseVersion("26.1") < ClickHouseVersion("26.1.0")); + + /// with equal components, no suffix sorts before any suffix ("" < "altinityantalya") + EXPECT_TRUE(ClickHouseVersion("26.1.3.20001") < ClickHouseVersion("26.1.3.20001.altinityantalya")); + + /// with equal components, suffixes order lexicographically: "...antalya" < "...stable" + EXPECT_TRUE(std::is_lt(ClickHouseVersion("26.3.1.20001.altinityantalya") + <=> ClickHouseVersion("26.3.1.20001.altinitystable"))); + + /// identical strings compare equal + EXPECT_TRUE(std::is_eq(ClickHouseVersion("26.1.3.20001.altinityantalya") + <=> ClickHouseVersion("26.1.3.20001.altinityantalya"))); +}