Merge pull request ClickHouse#84025 from ClickHouse/ncb/fix-client-replxx-crash

fix crash with clickhouse-client with syntax highlighting

Author: bharatnc

src/Client/ClientBaseHelpers.cpp

@@ -413,7 +413,42 @@ void highlight(const String & query, std::vector<replxx::Replxx::Color> & colors
 
 String highlighted(const String & query, const Context & context)
 {
-    size_t num_code_points = UTF8::countCodePoints(reinterpret_cast<const UInt8 *>(query.data()), query.size());
+    // Issue: https://github.com/ClickHouse/ClickHouse/issues/83987
+    /// Previously utf-8 code points were calculated in the following way:
+    /// size_t num_code_points = UTF8::countCodePoints(reinterpret_cast<const UInt8 *>(query.data()), query.size());
+    /// But, `UTF8::countCodePoints` and `UTF8::seqLength` seem to handle invalid UTF-8 sequences inconsistently
+    /// (e.g., hex literals like x'A0'), causing count mismatches and crashes.
+    /// For a quick fix, since the highlight function uses `UTF8::seqLength` for iteration, use the same logic for
+    /// counting to ensure consistency when invalid UTF-8 bytes are detected (use UTF8::countCodePoints for all other
+    /// cases to mainly for performance concerns).
+    /// TODO: @bharatnc Fix UTF8::countCodePoints to handle invalid UTF-8 sequences consistently with seqLength so that
+    /// this logic can be removed.
+    bool has_invalid_utf8 = false;
+    for (const char c : query)
+    {
+        /// Standalone UTF-8 continuation bytes (0x80-0xBF, e.g. 0xA0 from hex literals)
+        /// cause countCodePoints/seqLength inconsistency.
+        if (static_cast<unsigned char>(c) >= 0x80 && static_cast<unsigned char>(c) <= 0xBF)
+        {
+            has_invalid_utf8 = true;
+            break;
+        }
+    }
+    size_t num_code_points;
+    if (has_invalid_utf8)
+    {
+        num_code_points = 0;
+        const char * pos = query.data();
+        const char * end = pos + query.size();
+        while (pos < end)
+        {
+            pos += UTF8::seqLength(*pos);
+            ++num_code_points;
+        }
+    }
+    else
+        num_code_points = UTF8::countCodePoints(reinterpret_cast<const UInt8 *>(query.data()), query.size());
+
     std::vector<replxx::Replxx::Color> colors(num_code_points, replxx::Replxx::Color::DEFAULT);
     highlight(query, colors, context, 0);
 

tests/queries/0_stateless/03568_client_hex_literal_crash.expect

@@ -0,0 +1,37 @@
+#!/usr/bin/expect -f
+
+set basedir [file dirname $argv0]
+set basename [file tail $argv0]
+if {[info exists env(CLICKHOUSE_TMP)]} {
+    set CLICKHOUSE_TMP $env(CLICKHOUSE_TMP)
+} else {
+    set CLICKHOUSE_TMP "."
+}
+exp_internal -f $CLICKHOUSE_TMP/$basename.debuglog 0
+set history_file $CLICKHOUSE_TMP/$basename.history
+
+log_user 0
+set timeout 60
+match_max 100000
+
+expect_after {
+    # Do not ignore eof from expect
+    -i $any_spawn_id eof { exp_continue }
+    # A default timeout action is to do nothing, change it to fail
+    -i $any_spawn_id timeout { exit 1 }
+}
+
+spawn bash -c "source $basedir/../shell_config.sh ; \$CLICKHOUSE_CLIENT_BINARY \${CLICKHOUSE_CLIENT_EXPECT_OPT/--highlight 0/--highlight 1} --history_file=$history_file"
+expect "\n:) "
+
+# Validate that hex character sequence doesn't cause the client to crash.
+send  "SELECT x'A0' FORMAT TSV;\r"
+
+# expect at least one byte and a newline to pass
+expect -re {.+\r\n};
+
+expect ":) "
+
+# Ctrl+D
+send -- "\4"
+expect eof