From e0e4d0652b87f6b205de9510226fa1f0a26c5619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= Date: Tue, 25 Feb 2025 23:38:49 +0000 Subject: [PATCH] Fix issue #4841 Incorrectly attempt to use a collation with id larger than 255 during backend handshake Problem: When creating a backend connection, if the frontend connection has configured a collation with id larger than 255 (for example using SET NAMES utf8mb4 COLLATE utf8mb4_0900_as_ci; , ProxySQL may try to create a new connection using such collation. The problem is that at protocol level, during handshake only 1 byte can be used to configure the collation. That means that if ProxySQL sets a collation greater than 255 , only the less significant byte is used. Therefore even if ProxySQL believes that the backend connection will use the specified collation, mysql_real_connect() is using a different collation. For example, using collation 305 will lead to use collation 49 (305-256). Solution: When a collation id greater than 255 is detected, the default collation for that character set is used. ProxySQL will later issue a SET NAMES ... COLLATE to address the mismatch. --- lib/mysql_connection.cpp | 21 ++++++-- test/tap/tap/utils.cpp | 12 +++-- test/tap/tap/utils.h | 2 +- test/tap/tests/test_utf8mb4_as_ci-4841-t.cpp | 57 ++++++++++++++++++++ 4 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 test/tap/tests/test_utf8mb4_as_ci-4841-t.cpp diff --git a/lib/mysql_connection.cpp b/lib/mysql_connection.cpp index c889c380d..cf6921f28 100644 --- a/lib/mysql_connection.cpp +++ b/lib/mysql_connection.cpp @@ -779,11 +779,11 @@ void MySQL_Connection::connect_start_SetCharset() { csname = mysql_variables.client_get_value(myds->sess, SQL_CHARACTER_SET); } - const MARIADB_CHARSET_INFO * c = NULL; + MARIADB_CHARSET_INFO * c = NULL; if (csname) - c = proxysql_find_charset_nr(atoi(csname)); + c = (MARIADB_CHARSET_INFO *)proxysql_find_charset_nr(atoi(csname)); else - c = proxysql_find_charset_name(mysql_thread___default_variables[SQL_CHARACTER_SET]); + c = (MARIADB_CHARSET_INFO *)proxysql_find_charset_name(mysql_thread___default_variables[SQL_CHARACTER_SET]); if (!c) { // LCOV_EXCL_START @@ -791,6 +791,21 @@ void MySQL_Connection::connect_start_SetCharset() { assert(0); // LCOV_EXCL_STOP } + + if (c->nr > 255) { + const char *csname_default = c->csname; + c = NULL; + c = (MARIADB_CHARSET_INFO *)proxysql_find_charset_name(csname_default); + if (!c) { + // LCOV_EXCL_START + proxy_error("Not existing charset number %s\n", mysql_thread___default_variables[SQL_CHARACTER_SET]); + assert(0); + // LCOV_EXCL_STOP + } + } + + + { /* We are connecting to backend setting charset in mysql_options. * Client already has sent us a character set and client connection variables have been already set. diff --git a/test/tap/tap/utils.cpp b/test/tap/tap/utils.cpp index a15411033..a63d5c331 100644 --- a/test/tap/tap/utils.cpp +++ b/test/tap/tap/utils.cpp @@ -243,13 +243,15 @@ int mysql_query_t__(MYSQL* mysql, const char* query, const char* f, int ln, cons return mysql_query(mysql, query); } -int show_variable(MYSQL *mysql, const string& var_name, string& var_value) { - char query[128]; +int show_variable(MYSQL *mysql, const string& var_name, string& var_value, bool new_connection) { - snprintf(query, sizeof(query),"show variables like '%s'", var_name.c_str()); - if (mysql_query(mysql, query)) { + std::string query = "show variables "; + query += (new_connection == true ? "/* create_new_connection=1 */ " : ""); + query += " like '" + var_name + "'"; + + if (mysql_query(mysql, query.c_str())) { fprintf(stderr, "Failed to execute query [%s] : no %d, %s\n", - query, mysql_errno(mysql), mysql_error(mysql)); + query.c_str(), mysql_errno(mysql), mysql_error(mysql)); return exit_status(); } diff --git a/test/tap/tap/utils.h b/test/tap/tap/utils.h index 29eeb4c73..1c458b41b 100644 --- a/test/tap/tap/utils.h +++ b/test/tap/tap/utils.h @@ -119,7 +119,7 @@ int mysql_query_t__(MYSQL* mysql, const char* query, const char* f, int ln, cons } \ } while(0) -int show_variable(MYSQL *mysql, const std::string& var_name, std::string& var_value); +int show_variable(MYSQL *mysql, const std::string& var_name, std::string& var_value, bool new_connection=false); int show_admin_global_variable(MYSQL *mysql, const std::string& var_name, std::string& var_value); int set_admin_global_variable(MYSQL *mysql, const std::string& var_name, const std::string& var_value); int get_server_version(MYSQL *mysql, std::string& version); diff --git a/test/tap/tests/test_utf8mb4_as_ci-4841-t.cpp b/test/tap/tests/test_utf8mb4_as_ci-4841-t.cpp new file mode 100644 index 000000000..63f80e16f --- /dev/null +++ b/test/tap/tests/test_utf8mb4_as_ci-4841-t.cpp @@ -0,0 +1,57 @@ +/** + * @file test_utf8mb4_as_ci-4841-t.cpp + * @brief This test checks the use of collation 305 (utf8mb4_as_ci) . + * @details The test performs a 'SET NAMES' query to set utf8mb4_as_ci collation, then run a query + * on backend to verify the collation. + */ + +#include +#include +#include +#include + +#include +#include "mysql.h" + +#include "tap.h" +#include "command_line.h" +#include "utils.h" + +int main(int argc, char** argv) { + CommandLine cl; + + if(cl.getEnv()) + return exit_status(); + + plan(1); + diag("Testing SET NAMES utf8mb4 COLLATE utf8mb4_0900_as_ci"); + + MYSQL* mysql = mysql_init(NULL); + if (!mysql) + return exit_status(); + + if (!mysql_real_connect(mysql, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) { + fprintf(stderr, "Failed to connect to database: Error: %s\n", + mysql_error(mysql)); + return exit_status(); + } + + char * query = (char *)"SET NAMES utf8mb4 COLLATE utf8mb4_0900_as_ci"; + if (mysql_query(mysql, query)) { + fprintf(stderr, "%s: Error: %s\n", + query, + mysql_error(mysql)); + return exit_status(); + } + + std::string var_collation_connection = "collation_connection"; + std::string var_value; + + show_variable(mysql, var_collation_connection, var_value, true); + ok(var_value.compare("utf8mb4_0900_as_ci") == 0, "collation_connection , Expected utf8mb4_0900_as_ci . Actual %s", var_value.c_str()); // ok_1 + + mysql_close(mysql); + + return exit_status(); +} +