From 7fe7bdca7ea9a59939eff63fdbeb50685ba82ebf Mon Sep 17 00:00:00 2001
From: Teemu Ollakka <teemu.ollakka@galeracluster.com>
Date: Tue, 14 Jan 2025 16:36:21 +0200
Subject: [PATCH] Fix condition to treat an id as alphanumeric string

An id is considered an alphanumeric string if it contains a
non-empty sequence of alphanumeric characters followed by
one or more null characters.

Fixed unit tests accordingly.
---
 src/id.cpp       | 28 ++++++++++++++++++++++++----
 test/id_test.cpp | 47 ++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/src/id.cpp b/src/id.cpp
index 2999dc33..cef0a2ec 100644
--- a/src/id.cpp
+++ b/src/id.cpp
@@ -57,14 +57,34 @@ std::string wsrep::id::to_string() const
   return os.str();
 }
 
+
+/*
+ * If the buffer pointed by ptr contains only alphanumeric chars followed by
+ * one or more null terminators, consider it a valid alphanumeric string.
+ *
+ * A string starting with null is not an alphanumeric string.
+ */
+static bool is_alphanumeric_string(const char* ptr, size_t size)
+{
+    if (size == 0)
+        return false;
+    if (ptr[0] == '\0')
+        return false;
+    const char* first_not_alphanumeric = std::find_if_not(
+        ptr, ptr + size, [](char c) { return ::isalnum(c); });
+    if (static_cast<size_t>(std::distance(ptr, first_not_alphanumeric)) == size)
+        return false;
+    return std::all_of(first_not_alphanumeric, ptr + size,
+                       [](char c) { return (c == '\0'); });
+}
+
 std::ostream& wsrep::operator<<(std::ostream& os, const wsrep::id& id)
 {
     const char* ptr(static_cast<const char*>(id.data()));
     size_t size(id.size());
-    if (static_cast<size_t>(
-            std::count_if(ptr, ptr + size,
-                          [](char c) { return (::isalnum(c) || c == '\0'); }))
-        == size)
+    /* If the buffer pointed by ptr contains only alphanumeric chars followed by
+     * one or more null terminators, return the string. */
+    if (is_alphanumeric_string(ptr, size))
     {
         return (os << std::string(ptr, ::strnlen(ptr, size)));
     }
diff --git a/test/id_test.cpp b/test/id_test.cpp
index 63dea58e..262641be 100644
--- a/test/id_test.cpp
+++ b/test/id_test.cpp
@@ -33,7 +33,7 @@ BOOST_AUTO_TEST_CASE(id_test_uuid)
     wsrep::id id(uuid_str);
     std::ostringstream os;
     os << id;
-    BOOST_REQUIRE(uuid_str == os.str());
+    BOOST_REQUIRE_EQUAL(uuid_str, os.str());
 }
 
 BOOST_AUTO_TEST_CASE(id_test_string)
@@ -42,16 +42,19 @@ BOOST_AUTO_TEST_CASE(id_test_string)
     wsrep::id id(id_str);
     std::ostringstream os;
     os << id;
-    BOOST_REQUIRE(id_str == os.str());
+    BOOST_REQUIRE_EQUAL(id_str, os.str());
 }
 
 BOOST_AUTO_TEST_CASE(id_test_string_max)
 {
-    std::string id_str("1234567890123456");
+    std::string id_str("1234"
+                       "5678"
+                       "9012"
+                       "3456");
     wsrep::id id(id_str);
     std::ostringstream os;
     os << id;
-    BOOST_REQUIRE(id_str == os.str());
+    BOOST_REQUIRE_EQUAL(os.str(), "31323334-3536-3738-3930-313233343536");
 }
 
 BOOST_AUTO_TEST_CASE(id_test_string_too_long)
@@ -67,7 +70,7 @@ BOOST_AUTO_TEST_CASE(id_test_binary)
     wsrep::id id(data, sizeof(data));
     std::ostringstream os;
     os << id;
-    BOOST_REQUIRE(os.str() == "01020304-0506-0708-0900-010203040506");
+    BOOST_REQUIRE_EQUAL(os.str(), "01020304-0506-0708-0900-010203040506");
 }
 
 BOOST_AUTO_TEST_CASE(id_test_binary_too_long)
@@ -76,3 +79,37 @@ BOOST_AUTO_TEST_CASE(id_test_binary_too_long)
     BOOST_REQUIRE_EXCEPTION(wsrep::id id(data, sizeof(data)),
                             wsrep::runtime_error, exception_check);;
 }
+
+BOOST_AUTO_TEST_CASE(id_test_binary_all_alphanumeric)
+{
+    char data[16] = {'a', 'a', 'a', 'a',
+                     'a', 'a', 'a', 'a',
+                     'a', 'a', 'a', 'a',
+                     'a', 'a', 'a', 'a'};
+    wsrep::id id(data, sizeof(data));
+    std::ostringstream os;
+    os << id;
+    /* Value of 'a' is 97 in ASCII, which is 0x61 in hex. */
+    BOOST_REQUIRE_EQUAL(os.str(), "61616161-6161-6161-6161-616161616161");
+}
+
+BOOST_AUTO_TEST_CASE(id_test_binary_all_except_middle_alphanumeric)
+{
+    char data[16] = {'a', 'a', 'a', 'a',
+                     'a', 'a', 'a', 'a',
+                     'a', 'a', '\0', 'a',
+                     'a', 'a', 'a', 'a'};
+    wsrep::id id(data, sizeof(data));
+    std::ostringstream os;
+    os << id;
+    BOOST_REQUIRE_EQUAL(os.str(), "61616161-6161-6161-6161-006161616161");
+}
+
+BOOST_AUTO_TEST_CASE(id_test_null)
+{
+    char data[16] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
+    wsrep::id id(data, sizeof(data));
+    std::ostringstream os;
+    os << id;
+    BOOST_REQUIRE_EQUAL(os.str(), "00000000-0000-0000-0000-000000000000");
+}