From c3532088d4668b26cb6e88c623f887c3949356ec Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 13 Apr 2026 13:13:29 +0000 Subject: [PATCH 1/5] Add mysql-enable_client_session_tracking variable for client connections Introduce a new mysql global variable 'enable_client_session_tracking' (default: true) that controls whether CLIENT_SESSION_TRACKING is advertised to clients during handshake. This follows the same pattern as the existing 'enable_client_deprecate_eof' variable, allowing operators to disable session tracking capability on the client side independently of the server side. Closes #5621 --- include/MySQL_Thread.h | 1 + include/proxysql_structs.h | 2 ++ lib/MySQL_Protocol.cpp | 11 +++++++++++ lib/MySQL_Thread.cpp | 4 ++++ test/tap/tests/test_greeting_capabilities-t.cpp | 17 ++++++++++------- 5 files changed, 28 insertions(+), 7 deletions(-) diff --git a/include/MySQL_Thread.h b/include/MySQL_Thread.h index 6725ff730..bbae7ea09 100644 --- a/include/MySQL_Thread.h +++ b/include/MySQL_Thread.h @@ -584,6 +584,7 @@ class MySQL_Threads_Handler bool kill_backend_connection_when_disconnect; bool client_session_track_gtid; bool enable_client_deprecate_eof; + bool enable_client_session_tracking; bool enable_server_deprecate_eof; bool enable_load_data_local_infile; bool log_mysql_warnings_enabled; diff --git a/include/proxysql_structs.h b/include/proxysql_structs.h index 6f466ec14..4036c6b1d 100644 --- a/include/proxysql_structs.h +++ b/include/proxysql_structs.h @@ -1325,6 +1325,7 @@ __thread char * mysql_thread___default_variables[SQL_NAME_LAST_LOW_WM]; __thread int mysql_thread___query_digests_grouping_limit; __thread int mysql_thread___query_digests_groups_grouping_limit; __thread bool mysql_thread___enable_client_deprecate_eof; +__thread bool mysql_thread___enable_client_session_tracking; __thread bool mysql_thread___enable_server_deprecate_eof; __thread bool mysql_thread___log_mysql_warnings_enabled; __thread bool mysql_thread___enable_load_data_local_infile; @@ -1657,6 +1658,7 @@ extern __thread char * mysql_thread___default_variables[SQL_NAME_LAST_LOW_WM]; extern __thread int mysql_thread___query_digests_grouping_limit; extern __thread int mysql_thread___query_digests_groups_grouping_limit; extern __thread bool mysql_thread___enable_client_deprecate_eof; +extern __thread bool mysql_thread___enable_client_session_tracking; extern __thread bool mysql_thread___enable_server_deprecate_eof; extern __thread bool mysql_thread___log_mysql_warnings_enabled; extern __thread bool mysql_thread___enable_load_data_local_infile; diff --git a/lib/MySQL_Protocol.cpp b/lib/MySQL_Protocol.cpp index a6a4eb9b4..6e98c7960 100644 --- a/lib/MySQL_Protocol.cpp +++ b/lib/MySQL_Protocol.cpp @@ -1093,6 +1093,11 @@ bool MySQL_Protocol::generate_pkt_initial_handshake(bool send, void **ptr, unsig } else { mysql_thread___server_capabilities &= ~CLIENT_DEPRECATE_EOF; } + if (mysql_thread___enable_client_session_tracking) { + mysql_thread___server_capabilities |= CLIENT_SESSION_TRACKING; + } else { + mysql_thread___server_capabilities &= ~CLIENT_SESSION_TRACKING; + } uint32_t server_capabilities = mysql_thread___server_capabilities; if (deprecate_eof_active && mysql_thread___enable_client_deprecate_eof) { server_capabilities |= CLIENT_DEPRECATE_EOF; @@ -1601,6 +1606,12 @@ bool MySQL_Protocol::PPHR_2(unsigned char *pkt, unsigned int len, bool& ret, MyP if (!mysql_thread___enable_client_deprecate_eof) { vars1.capabilities &= ~CLIENT_DEPRECATE_EOF; } + // similarly, enforce disabling 'CLIENT_SESSION_TRACKING' from the supported + // capabilities when explicitly disabled by global variable + // 'mysql_thread___enable_client_session_tracking'. + if (!mysql_thread___enable_client_session_tracking) { + vars1.capabilities &= ~CLIENT_SESSION_TRACKING; + } (*myds)->myconn->options.client_flag = vars1.capabilities; pkt += sizeof(uint32_t); vars1.max_pkt = CPY4(pkt); diff --git a/lib/MySQL_Thread.cpp b/lib/MySQL_Thread.cpp index 079f108dc..151806f46 100644 --- a/lib/MySQL_Thread.cpp +++ b/lib/MySQL_Thread.cpp @@ -358,6 +358,7 @@ static char * mysql_thread_variables_names[]= { (char *)"connect_timeout_server", (char *)"connect_timeout_server_max", (char *)"enable_client_deprecate_eof", + (char *)"enable_client_session_tracking", (char *)"enable_server_deprecate_eof", (char *)"enable_load_data_local_infile", (char *)"eventslog_filename", @@ -1441,6 +1442,7 @@ MySQL_Threads_Handler::MySQL_Threads_Handler() { variables.query_digests_grouping_limit = 3; variables.query_digests_groups_grouping_limit= 10; // changed in 2.6.0 , was 0 variables.enable_client_deprecate_eof=true; + variables.enable_client_session_tracking=true; variables.enable_server_deprecate_eof=true; variables.enable_load_data_local_infile=false; variables.log_mysql_warnings_enabled=false; @@ -2505,6 +2507,7 @@ char ** MySQL_Threads_Handler::get_variables_list() { VariablesPointers_bool["connection_warming"] = make_tuple(&variables.connection_warming, false); VariablesPointers_bool["default_reconnect"] = make_tuple(&variables.default_reconnect, false); VariablesPointers_bool["enable_client_deprecate_eof"] = make_tuple(&variables.enable_client_deprecate_eof, false); + VariablesPointers_bool["enable_client_session_tracking"] = make_tuple(&variables.enable_client_session_tracking, false); VariablesPointers_bool["enable_server_deprecate_eof"] = make_tuple(&variables.enable_server_deprecate_eof, false); #ifdef PROXYSQLFFTO VariablesPointers_bool["ffto_enabled"] = make_tuple(&variables.ffto_enabled, false); @@ -4743,6 +4746,7 @@ void MySQL_Thread::refresh_variables() { REFRESH_VARIABLE_BOOL(servers_stats); REFRESH_VARIABLE_BOOL(default_reconnect); REFRESH_VARIABLE_BOOL(enable_client_deprecate_eof); + REFRESH_VARIABLE_BOOL(enable_client_session_tracking); REFRESH_VARIABLE_BOOL(enable_server_deprecate_eof); REFRESH_VARIABLE_BOOL(enable_load_data_local_infile); REFRESH_VARIABLE_BOOL(log_mysql_warnings_enabled); diff --git a/test/tap/tests/test_greeting_capabilities-t.cpp b/test/tap/tests/test_greeting_capabilities-t.cpp index 51fe72714..74ef0024b 100644 --- a/test/tap/tests/test_greeting_capabilities-t.cpp +++ b/test/tap/tests/test_greeting_capabilities-t.cpp @@ -1,8 +1,9 @@ /** * @file test_greeting_capabilities-t.cpp * @brief Checks that ProxySQL sends the correct capabilities during handshake. - * @details Thist test should also check conditional capabilities enabled by config variables. E.g: + * @details This test also checks conditional capabilities enabled by config variables. E.g: * 'CLIENT_DEPRECATE_EOF' when enabled through 'mysql-enable_client_deprecate_eof'. + * 'CLIENT_SESSION_TRACKING' when enabled through 'mysql-enable_client_session_tracking'. */ #include @@ -27,7 +28,6 @@ std::vector def_capabilities { CLIENT_MULTI_STATEMENTS, CLIENT_PS_MULTI_RESULTS, CLIENT_PLUGIN_AUTH, - CLIENT_SESSION_TRACKING, CLIENT_REMEMBER_OPTIONS }; @@ -64,7 +64,9 @@ pair check_server_capabilities( } int test_proxy_capabilites(const CommandLine& cl, MYSQL* admin) { + // Test 1: disable CLIENT_DEPRECATE_EOF and CLIENT_SESSION_TRACKING MYSQL_QUERY(admin, "SET mysql-enable_client_deprecate_eof=0"); + MYSQL_QUERY(admin, "SET mysql-enable_client_session_tracking=0"); MYSQL_QUERY(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); MYSQL* proxy = mysql_init(NULL); @@ -74,17 +76,19 @@ int test_proxy_capabilites(const CommandLine& cl, MYSQL* admin) { return EXIT_FAILURE; } - pair caps_res { check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF }, false) }; + pair caps_res { check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF, CLIENT_SESSION_TRACKING }, false) }; uint64_t ext_caps = (proxy->server_capabilities >> 16) << 16; mysql_close(proxy); ok( - caps_res.first, "ProxySQL greeting should return the expected capabilities - Exp: '%ld', Act: '%ld'", + caps_res.first, "ProxySQL greeting should return the expected capabilities with deprecate_eof and session_tracking disabled - Exp: '%ld', Act: '%ld'", caps_res.second, ext_caps ); + // Test 2: enable CLIENT_DEPRECATE_EOF and CLIENT_SESSION_TRACKING MYSQL_QUERY(admin, "SET mysql-enable_client_deprecate_eof=1"); + MYSQL_QUERY(admin, "SET mysql-enable_client_session_tracking=1"); MYSQL_QUERY(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); proxy = mysql_init(NULL); @@ -95,11 +99,11 @@ int test_proxy_capabilites(const CommandLine& cl, MYSQL* admin) { return EXIT_FAILURE; } - caps_res = check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF }, true); + caps_res = check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF, CLIENT_SESSION_TRACKING }, true); ext_caps = (proxy->server_capabilities >> 16) << 16; ok( - caps_res.first, "ProxySQL greeting should return the expected capabilities - Exp: '%ld', Act: '%ld'", + caps_res.first, "ProxySQL greeting should return the expected capabilities with deprecate_eof and session_tracking enabled - Exp: '%ld', Act: '%ld'", caps_res.second, ext_caps ); @@ -111,7 +115,6 @@ int test_proxy_capabilites(const CommandLine& cl, MYSQL* admin) { int main(int argc, char** argv) { CommandLine cl; - // TODO: Harcoded for now, this is an initial version of the test. plan(2); if (cl.getEnv()) { From 6e1483f1f3ba2d60b90b3fa3b91b5fe7be4e6ccc Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 13 Apr 2026 17:32:27 +0000 Subject: [PATCH 2/5] Address PR review feedback for test_greeting_capabilities - Fix check_server_capabilities() to accumulate results with &= instead of overwriting caps_match per flag, preventing false positives with multiple conditional capabilities - Add mixed toggle tests (deprecate_eof=1/session_tracking=0 and vice versa) for better isolation coverage --- .../tests/test_greeting_capabilities-t.cpp | 62 +++++++++++++++++-- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/test/tap/tests/test_greeting_capabilities-t.cpp b/test/tap/tests/test_greeting_capabilities-t.cpp index 74ef0024b..43ed47f2b 100644 --- a/test/tap/tests/test_greeting_capabilities-t.cpp +++ b/test/tap/tests/test_greeting_capabilities-t.cpp @@ -38,24 +38,27 @@ pair check_server_capabilities( uint64_t exp_caps = 0; for (const uint64_t cap : def_capabilities) { - caps_match = proxy->server_capabilities & cap; + const bool has_cap = (proxy->server_capabilities & cap) != 0; + caps_match &= has_cap; exp_caps |= cap; - if (caps_match == false) { + if (!has_cap) { diag("Missing expected DEFAULT capability: %ld", cap); } } for (const uint64_t exp_cap : exp_conn_caps) { + bool this_check_ok = false; if (present) { - caps_match = proxy->server_capabilities & exp_cap; + this_check_ok = (proxy->server_capabilities & exp_cap) != 0; exp_caps |= exp_cap; } else { - caps_match = !(proxy->server_capabilities & exp_cap); + this_check_ok = (proxy->server_capabilities & exp_cap) == 0; exp_caps &= ~exp_cap; } + caps_match &= this_check_ok; - if (caps_match == false) { + if (!this_check_ok) { diag("Missing expected CONDITIONAL capability: %ld", exp_cap); } } @@ -109,13 +112,60 @@ int test_proxy_capabilites(const CommandLine& cl, MYSQL* admin) { mysql_close(proxy); + // Test 3: enable CLIENT_DEPRECATE_EOF only (session_tracking disabled) + MYSQL_QUERY(admin, "SET mysql-enable_client_deprecate_eof=1"); + MYSQL_QUERY(admin, "SET mysql-enable_client_session_tracking=0"); + MYSQL_QUERY(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); + + proxy = mysql_init(NULL); + proxy->options.client_flag |= CLIENT_DEPRECATE_EOF; + + if (!mysql_real_connect(proxy, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxy)); + return EXIT_FAILURE; + } + + caps_res = check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF }, true); + caps_res.first &= (check_server_capabilities(proxy, { CLIENT_SESSION_TRACKING }, false).first); + ext_caps = (proxy->server_capabilities >> 16) << 16; + + ok( + caps_res.first, "ProxySQL greeting with deprecate_eof enabled and session_tracking disabled - Exp: '%ld', Act: '%ld'", + caps_res.second, ext_caps + ); + + mysql_close(proxy); + + // Test 4: enable CLIENT_SESSION_TRACKING only (deprecate_eof disabled) + MYSQL_QUERY(admin, "SET mysql-enable_client_deprecate_eof=0"); + MYSQL_QUERY(admin, "SET mysql-enable_client_session_tracking=1"); + MYSQL_QUERY(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); + + proxy = mysql_init(NULL); + + if (!mysql_real_connect(proxy, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxy)); + return EXIT_FAILURE; + } + + caps_res = check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF }, false); + caps_res.first &= (check_server_capabilities(proxy, { CLIENT_SESSION_TRACKING }, true).first); + ext_caps = (proxy->server_capabilities >> 16) << 16; + + ok( + caps_res.first, "ProxySQL greeting with deprecate_eof disabled and session_tracking enabled - Exp: '%ld', Act: '%ld'", + caps_res.second, ext_caps + ); + + mysql_close(proxy); + return EXIT_SUCCESS; } int main(int argc, char** argv) { CommandLine cl; - plan(2); + plan(4); if (cl.getEnv()) { diag("Failed to get the required environmental variables."); From 536c9d9169cfdb9724cc8c46b7e3181af7671ee6 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 17 Apr 2026 15:44:34 +0000 Subject: [PATCH 3/5] Restore greeting upper-word capabilities dropped in 8c6a6444d The zstd refactor in 8c6a6444d replaced the per-greeting 'extended_capabilities' local with 'server_capabilities' derived from 'mysql_thread___server_capabilities'. As a side effect, the four upper-word bits the greeting used to hardcode were no longer advertised: CLIENT_MULTI_STATEMENTS CLIENT_MULTI_RESULTS CLIENT_PS_MULTI_RESULTS CLIENT_REMEMBER_OPTIONS OR these bits back into 'mysql_thread___server_capabilities' before the upper-word memcpy, matching what real MySQL 8.x advertises (see #4023). Also rewrite test_greeting_capabilities-t with doxygen documentation and step-by-step diag() tracing that decodes capability bitmasks to names, and reinstate the baseline-capability assertion that silently degraded when commit 6e1483f1f tightened the caps_match overwrite into '&='. In 'PPHR_2', fold the CLIENT_SESSION_TRACKING enforcement into the shared two-step comment block for clarity. --- lib/MySQL_Protocol.cpp | 26 +- .../tests/test_greeting_capabilities-t.cpp | 505 ++++++++++++++---- 2 files changed, 408 insertions(+), 123 deletions(-) diff --git a/lib/MySQL_Protocol.cpp b/lib/MySQL_Protocol.cpp index 6e98c7960..0d9f41915 100644 --- a/lib/MySQL_Protocol.cpp +++ b/lib/MySQL_Protocol.cpp @@ -1088,6 +1088,13 @@ bool MySQL_Protocol::generate_pkt_initial_handshake(bool send, void **ptr, unsig } mysql_thread___server_capabilities |= CLIENT_LONG_FLAG; mysql_thread___server_capabilities |= CLIENT_MYSQL | CLIENT_PLUGIN_AUTH | CLIENT_RESERVED; + // Upper-word capabilities that ProxySQL always advertises, matching real + // MySQL server greetings (see issue #4023). These were previously supplied + // via a separate 'extended_capabilities' local and were accidentally + // dropped during the zstd refactor in 8c6a6444d. + mysql_thread___server_capabilities |= + CLIENT_MULTI_STATEMENTS | CLIENT_MULTI_RESULTS | + CLIENT_PS_MULTI_RESULTS | CLIENT_REMEMBER_OPTIONS; if (mysql_thread___enable_client_deprecate_eof) { mysql_thread___server_capabilities |= CLIENT_DEPRECATE_EOF; } else { @@ -1594,21 +1601,16 @@ bool MySQL_Protocol::PPHR_2(unsigned char *pkt, unsigned int len, bool& ret, MyP if (vars1.capabilities & CLIENT_MULTI_STATEMENTS) { vars1.capabilities |= CLIENT_MULTI_RESULTS; } - // we enforce disabling 'CLIENT_DEPRECATE_EOF' from the supported capabilities - // in case it's explicitly disabled by global variable 'mysql_thread___enable_client_deprecate_eof'. - // This is because further checks to actually threat the connection as a connection - // supporting 'CLIENT_DEPRECATE_EOF' rely in 'client_flag' field from - // 'MySQL_Connection::options'. - // This is the second step for ensuring that the connection is being handling - // in both ProxySQL and client side as a connection without 'CLIENT_DEPRECATE_EOF' support. - // First step is replying to client during initial handshake (in 'generate_pkt_initial_handshake') - // specifying no 'CLIENT_DEPRECATE_EOF' support in 'server_capabilities'. + // Enforce disabling 'CLIENT_DEPRECATE_EOF' and 'CLIENT_SESSION_TRACKING' from the + // supported capabilities when explicitly disabled via + // 'mysql_thread___enable_client_deprecate_eof' / 'mysql_thread___enable_client_session_tracking'. + // Downstream logic that treats the connection as supporting either capability relies on + // the 'client_flag' field of 'MySQL_Connection::options'. This is the second step of a + // two-step enforcement: the first step omits these bits from 'server_capabilities' + // during the initial handshake (see 'generate_pkt_initial_handshake'). if (!mysql_thread___enable_client_deprecate_eof) { vars1.capabilities &= ~CLIENT_DEPRECATE_EOF; } - // similarly, enforce disabling 'CLIENT_SESSION_TRACKING' from the supported - // capabilities when explicitly disabled by global variable - // 'mysql_thread___enable_client_session_tracking'. if (!mysql_thread___enable_client_session_tracking) { vars1.capabilities &= ~CLIENT_SESSION_TRACKING; } diff --git a/test/tap/tests/test_greeting_capabilities-t.cpp b/test/tap/tests/test_greeting_capabilities-t.cpp index 43ed47f2b..d0bf4b408 100644 --- a/test/tap/tests/test_greeting_capabilities-t.cpp +++ b/test/tap/tests/test_greeting_capabilities-t.cpp @@ -1,14 +1,50 @@ /** * @file test_greeting_capabilities-t.cpp - * @brief Checks that ProxySQL sends the correct capabilities during handshake. - * @details This test also checks conditional capabilities enabled by config variables. E.g: - * 'CLIENT_DEPRECATE_EOF' when enabled through 'mysql-enable_client_deprecate_eof'. - * 'CLIENT_SESSION_TRACKING' when enabled through 'mysql-enable_client_session_tracking'. + * @brief Verifies the capability flags that ProxySQL advertises in the server + * 'Greeting' packet during the initial MySQL handshake. + * + * @details + * Background + * ---------- + * When a MySQL client connects to ProxySQL, ProxySQL answers with the initial + * handshake packet (a.k.a. "Server Greeting"). Embedded in that packet is a + * 32-bit capability bitmask that tells the client which protocol features the + * server supports (see MySQL Client/Server Protocol documentation). + * + * A subset of those capability bits are controlled by runtime ProxySQL + * variables and can therefore be toggled at will: + * + * - CLIENT_DEPRECATE_EOF -> `mysql-enable_client_deprecate_eof` + * - CLIENT_SESSION_TRACKING -> `mysql-enable_client_session_tracking` + * + * This test exercises all four combinations of those two toggles and checks + * the resulting capability bitmask returned by ProxySQL. + * + * Test strategy + * ------------- + * For each combination `{deprecate_eof, session_tracking}` in + * `{off,off} / {on,on} / {on,off} / {off,on}`: + * + * 1. Set the two variables on the ProxySQL admin interface. + * 2. Apply the variables to runtime (`LOAD MYSQL VARIABLES TO RUNTIME`). + * 3. Open a fresh client connection against ProxySQL (mysql_real_connect). + * 4. Read `mysql->server_capabilities` (populated by libmariadb from the + * greeting packet). + * 5. Check that every capability in the expectation's `present_caps` is set + * and every capability in `absent_caps` is cleared. + * 6. Emit a single TAP `ok()` with a verbose message and `diag()` traces for + * every intermediate value so a failure can be debugged without attaching + * a protocol analyzer. + * + * The suite keeps running after a failure so that a single run surfaces every + * broken scenario at once. */ +#include #include +#include +#include #include -#include #include #include @@ -22,147 +58,384 @@ using std::pair; using std::string; using std::vector; -// By default the following capabilities should be present -std::vector def_capabilities { - CLIENT_MULTI_RESULTS, - CLIENT_MULTI_STATEMENTS, - CLIENT_PS_MULTI_RESULTS, - CLIENT_PLUGIN_AUTH, - CLIENT_REMEMBER_OPTIONS +/** + * @struct cap_entry_t + * @brief One entry in the capability-name dictionary. + * + * The test is more useful if failures log capability *names* rather than raw + * integers. `KNOWN_CAPS` is a table of (bit, textual_name) pairs used by + * @ref caps_to_string to produce human readable diagnostics. + */ +struct cap_entry_t { + uint64_t bit; ///< Single-bit capability mask (e.g. CLIENT_PLUGIN_AUTH). + const char* name; ///< Mnemonic shown in diagnostics. }; -pair check_server_capabilities( - MYSQL* proxy, const vector& exp_conn_caps, bool present -) { - bool caps_match = true; - uint64_t exp_caps = 0; - - for (const uint64_t cap : def_capabilities) { - const bool has_cap = (proxy->server_capabilities & cap) != 0; - caps_match &= has_cap; - exp_caps |= cap; +/** + * @brief Dictionary used to decode a capability bitmask into readable form. + * + * Only contains the bits actually referenced by the test or by the handshake + * code path — it does not need to be exhaustive, but missing bits will simply + * appear as `0x` in `caps_to_string`. + */ +static const cap_entry_t KNOWN_CAPS[] = { + { CLIENT_MYSQL, "CLIENT_MYSQL" }, + { CLIENT_FOUND_ROWS, "CLIENT_FOUND_ROWS" }, + { CLIENT_LONG_FLAG, "CLIENT_LONG_FLAG" }, + { CLIENT_CONNECT_WITH_DB, "CLIENT_CONNECT_WITH_DB" }, + { CLIENT_NO_SCHEMA, "CLIENT_NO_SCHEMA" }, + { CLIENT_COMPRESS, "CLIENT_COMPRESS" }, + { CLIENT_ODBC, "CLIENT_ODBC" }, + { CLIENT_LOCAL_FILES, "CLIENT_LOCAL_FILES" }, + { CLIENT_IGNORE_SPACE, "CLIENT_IGNORE_SPACE" }, + { CLIENT_PROTOCOL_41, "CLIENT_PROTOCOL_41" }, + { CLIENT_INTERACTIVE, "CLIENT_INTERACTIVE" }, + { CLIENT_SSL, "CLIENT_SSL" }, + { CLIENT_IGNORE_SIGPIPE, "CLIENT_IGNORE_SIGPIPE" }, + { CLIENT_TRANSACTIONS, "CLIENT_TRANSACTIONS" }, + { CLIENT_RESERVED, "CLIENT_RESERVED" }, + { CLIENT_SECURE_CONNECTION, "CLIENT_SECURE_CONNECTION" }, + { CLIENT_MULTI_STATEMENTS, "CLIENT_MULTI_STATEMENTS" }, + { CLIENT_MULTI_RESULTS, "CLIENT_MULTI_RESULTS" }, + { CLIENT_PS_MULTI_RESULTS, "CLIENT_PS_MULTI_RESULTS" }, + { CLIENT_PLUGIN_AUTH, "CLIENT_PLUGIN_AUTH" }, + { CLIENT_CONNECT_ATTRS, "CLIENT_CONNECT_ATTRS" }, + { CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA, "CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA" }, + { CLIENT_CAN_HANDLE_EXPIRED_PASSWORDS, "CLIENT_CAN_HANDLE_EXPIRED_PASSWORDS" }, + { CLIENT_SESSION_TRACKING, "CLIENT_SESSION_TRACKING" }, + { CLIENT_DEPRECATE_EOF, "CLIENT_DEPRECATE_EOF" }, + { CLIENT_ZSTD_COMPRESSION, "CLIENT_ZSTD_COMPRESSION" }, + { CLIENT_SSL_VERIFY_SERVER_CERT, "CLIENT_SSL_VERIFY_SERVER_CERT" }, + { CLIENT_REMEMBER_OPTIONS, "CLIENT_REMEMBER_OPTIONS" }, +}; - if (!has_cap) { - diag("Missing expected DEFAULT capability: %ld", cap); - } +/** + * @brief Build a human readable list of the bits set in @p caps. + * + * Every bit mentioned in @ref KNOWN_CAPS is emitted by name; every remaining + * bit not covered by the dictionary is printed as `0x` so nothing is + * silently dropped. + * + * @param caps Raw 32-bit capability bitmask (as returned by the server). + * @return Formatted string, e.g. + * `CLIENT_LONG_PASSWORD | CLIENT_PROTOCOL_41 | 0x04000000`. + * Returns the literal `""` if @p caps == 0. + */ +static string caps_to_string(uint64_t caps) { + if (caps == 0) { + return ""; } - for (const uint64_t exp_cap : exp_conn_caps) { - bool this_check_ok = false; - if (present) { - this_check_ok = (proxy->server_capabilities & exp_cap) != 0; - exp_caps |= exp_cap; - } else { - this_check_ok = (proxy->server_capabilities & exp_cap) == 0; - exp_caps &= ~exp_cap; - } - caps_match &= this_check_ok; + std::ostringstream oss; + bool first = true; + uint64_t remaining = caps; - if (!this_check_ok) { - diag("Missing expected CONDITIONAL capability: %ld", exp_cap); + for (const cap_entry_t& e : KNOWN_CAPS) { + if ((caps & e.bit) != 0) { + if (!first) oss << " | "; + oss << e.name; + first = false; + remaining &= ~e.bit; } } - return { caps_match, exp_caps }; + if (remaining != 0) { + char buf[32]; + snprintf(buf, sizeof(buf), "0x%08llx", + static_cast(remaining)); + if (!first) oss << " | "; + oss << buf; + } + + return oss.str(); } -int test_proxy_capabilites(const CommandLine& cl, MYSQL* admin) { - // Test 1: disable CLIENT_DEPRECATE_EOF and CLIENT_SESSION_TRACKING - MYSQL_QUERY(admin, "SET mysql-enable_client_deprecate_eof=0"); - MYSQL_QUERY(admin, "SET mysql-enable_client_session_tracking=0"); - MYSQL_QUERY(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); +/** + * @brief Capability bits ProxySQL MUST advertise in every greeting, regardless + * of any runtime toggle. + * + * These match what a real MySQL 8.x server advertises in its Server Greeting + * and are relied upon by several clients (JDBC in particular — see issue + * #4023). Regressing any of them typically manifests as + * `java.lang.ArrayIndexOutOfBoundsException` on the client side. + * + * This list is checked against every scenario via @ref verify_caps. + */ +static const vector BASELINE_PRESENT_CAPS = { + CLIENT_MULTI_STATEMENTS, + CLIENT_MULTI_RESULTS, + CLIENT_PS_MULTI_RESULTS, + CLIENT_PLUGIN_AUTH, + CLIENT_REMEMBER_OPTIONS, +}; - MYSQL* proxy = mysql_init(NULL); +/** + * @struct caps_expectation_t + * @brief Expected state of the toggle-controlled capability bits in a scenario. + * + * A test scenario supplies one `caps_expectation_t` describing ONLY the bits + * it cares to toggle. Bits in @ref present_caps MUST be set in + * `mysql->server_capabilities`; bits in @ref absent_caps MUST be cleared. Bits + * appearing in neither list are ignored here because ProxySQL's baseline + * greeting also flips bits like `CLIENT_COMPRESS`, `CLIENT_SSL`, etc. whose + * values depend on global configuration unrelated to the scenario. + * + * In addition to the scenario-specific expectation, @ref BASELINE_PRESENT_CAPS + * is always checked — see @ref verify_caps. + */ +struct caps_expectation_t { + vector present_caps; ///< Bits that MUST be set (scenario-specific). + vector absent_caps; ///< Bits that MUST be clear (scenario-specific). +}; - if (!mysql_real_connect(proxy, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) { - fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxy)); - return EXIT_FAILURE; +/** + * @brief Compare actual vs expected capability bits and emit diagnostics. + * + * Verifies three things against @p actual_caps (coming from + * `mysql->server_capabilities`): + * 1. Every bit in @ref BASELINE_PRESENT_CAPS is set. + * 2. Every bit in `exp.present_caps` is set. + * 3. Every bit in `exp.absent_caps` is cleared. + * + * The function never stops on the first mismatch: every deviation is reported + * via `diag()` so one failed run produces a complete picture of what is wrong. + * + * @param actual_caps Bitmask returned by the server (already 32 bit wide). + * @param exp Expectation describing required/forbidden bits + * specific to the scenario (on top of the baseline). + * @return `true` if every required bit is set and every forbidden + * bit is clear; `false` otherwise. + */ +static bool verify_caps(uint64_t actual_caps, const caps_expectation_t& exp) { + bool ok_flag = true; + + for (const uint64_t bit : BASELINE_PRESENT_CAPS) { + const bool has = (actual_caps & bit) != 0; + diag(" [baseline ] %-40s -> %s", + caps_to_string(bit).c_str(), has ? "OK" : "MISSING"); + if (!has) ok_flag = false; } - pair caps_res { check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF, CLIENT_SESSION_TRACKING }, false) }; - uint64_t ext_caps = (proxy->server_capabilities >> 16) << 16; - - mysql_close(proxy); - - ok( - caps_res.first, "ProxySQL greeting should return the expected capabilities with deprecate_eof and session_tracking disabled - Exp: '%ld', Act: '%ld'", - caps_res.second, ext_caps - ); + for (const uint64_t bit : exp.present_caps) { + const bool has = (actual_caps & bit) != 0; + diag(" [expect present] %-40s -> %s", + caps_to_string(bit).c_str(), has ? "OK" : "MISSING"); + if (!has) ok_flag = false; + } - // Test 2: enable CLIENT_DEPRECATE_EOF and CLIENT_SESSION_TRACKING - MYSQL_QUERY(admin, "SET mysql-enable_client_deprecate_eof=1"); - MYSQL_QUERY(admin, "SET mysql-enable_client_session_tracking=1"); - MYSQL_QUERY(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); + for (const uint64_t bit : exp.absent_caps) { + const bool has = (actual_caps & bit) != 0; + diag(" [expect absent ] %-40s -> %s", + caps_to_string(bit).c_str(), has ? "UNEXPECTED" : "OK"); + if (has) ok_flag = false; + } - proxy = mysql_init(NULL); - proxy->options.client_flag |= CLIENT_DEPRECATE_EOF; + return ok_flag; +} - if (!mysql_real_connect(proxy, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) { - fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxy)); - return EXIT_FAILURE; +/** + * @brief Apply the two runtime toggles under test and reload them. + * + * Runs three admin statements: + * - `SET mysql-enable_client_deprecate_eof = <0|1>` + * - `SET mysql-enable_client_session_tracking = <0|1>` + * - `LOAD MYSQL VARIABLES TO RUNTIME` + * + * @param admin Already-connected admin MYSQL* handle. + * @param deprecate_eof Desired value for `mysql-enable_client_deprecate_eof`. + * @param session_tracking Desired value for `mysql-enable_client_session_tracking`. + * @return `true` on success, `false` if any statement failed + * (the error is printed via `diag()` before returning). + */ +static bool apply_runtime_toggles(MYSQL* admin, bool deprecate_eof, bool session_tracking) { + char stmt[256]; + + snprintf(stmt, sizeof(stmt), + "SET mysql-enable_client_deprecate_eof=%d", deprecate_eof ? 1 : 0); + diag(" admin> %s", stmt); + if (mysql_query(admin, stmt)) { + diag(" admin query failed: %s", mysql_error(admin)); + return false; } - caps_res = check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF, CLIENT_SESSION_TRACKING }, true); - ext_caps = (proxy->server_capabilities >> 16) << 16; - - ok( - caps_res.first, "ProxySQL greeting should return the expected capabilities with deprecate_eof and session_tracking enabled - Exp: '%ld', Act: '%ld'", - caps_res.second, ext_caps - ); + snprintf(stmt, sizeof(stmt), + "SET mysql-enable_client_session_tracking=%d", session_tracking ? 1 : 0); + diag(" admin> %s", stmt); + if (mysql_query(admin, stmt)) { + diag(" admin query failed: %s", mysql_error(admin)); + return false; + } - mysql_close(proxy); + const char* reload = "LOAD MYSQL VARIABLES TO RUNTIME"; + diag(" admin> %s", reload); + if (mysql_query(admin, reload)) { + diag(" admin query failed: %s", mysql_error(admin)); + return false; + } - // Test 3: enable CLIENT_DEPRECATE_EOF only (session_tracking disabled) - MYSQL_QUERY(admin, "SET mysql-enable_client_deprecate_eof=1"); - MYSQL_QUERY(admin, "SET mysql-enable_client_session_tracking=0"); - MYSQL_QUERY(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); + return true; +} - proxy = mysql_init(NULL); - proxy->options.client_flag |= CLIENT_DEPRECATE_EOF; +/** + * @brief Open a fresh client connection to ProxySQL and log the outcome. + * + * The returned handle is already connected via `mysql_real_connect`. The + * caller owns it and MUST `mysql_close` it. On failure the function logs the + * mariadb client error and returns `nullptr`. + * + * @param cl Parsed CommandLine describing host/port/user/password. + * @param client_flag Bitmask passed as `options.client_flag` *before* + * `mysql_real_connect`. Used to mirror what a regular + * client would advertise (e.g. `CLIENT_DEPRECATE_EOF`). + * @return Connected MYSQL* on success; `nullptr` on failure. + */ +static MYSQL* open_client(const CommandLine& cl, uint64_t client_flag) { + MYSQL* proxy = mysql_init(NULL); + if (proxy == NULL) { + diag(" mysql_init returned NULL"); + return NULL; + } - if (!mysql_real_connect(proxy, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) { - fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxy)); - return EXIT_FAILURE; + if (client_flag != 0) { + proxy->options.client_flag |= client_flag; + diag(" client advertises flag(s): %s", caps_to_string(client_flag).c_str()); } - caps_res = check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF }, true); - caps_res.first &= (check_server_capabilities(proxy, { CLIENT_SESSION_TRACKING }, false).first); - ext_caps = (proxy->server_capabilities >> 16) << 16; + diag(" connecting to %s:%d as '%s'", cl.host, cl.port, cl.username); + if (!mysql_real_connect(proxy, cl.host, cl.username, cl.password, + NULL, cl.port, NULL, 0)) { + diag(" mysql_real_connect failed: %s", mysql_error(proxy)); + mysql_close(proxy); + return NULL; + } - ok( - caps_res.first, "ProxySQL greeting with deprecate_eof enabled and session_tracking disabled - Exp: '%ld', Act: '%ld'", - caps_res.second, ext_caps - ); + return proxy; +} - mysql_close(proxy); +/** + * @struct scenario_t + * @brief One permutation of the runtime toggles plus its expectation. + * + * Keeping the four scenarios in a table makes the main loop small and the + * list of cases easy to extend: adding a fifth scenario only requires a new + * row here, not more code. + */ +struct scenario_t { + const char* name; ///< Short label used in TAP + diag. + bool deprecate_eof; ///< Value of the runtime toggle. + bool session_tracking; ///< Value of the runtime toggle. + uint64_t client_flag; ///< Extra `client_flag` the client advertises before connect. + caps_expectation_t expectation; ///< Bits required/forbidden in the greeting. +}; - // Test 4: enable CLIENT_SESSION_TRACKING only (deprecate_eof disabled) - MYSQL_QUERY(admin, "SET mysql-enable_client_deprecate_eof=0"); - MYSQL_QUERY(admin, "SET mysql-enable_client_session_tracking=1"); - MYSQL_QUERY(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); +/** + * @brief Execute every scenario once, emitting one TAP assertion per run. + * + * The function: + * - calls @ref apply_runtime_toggles to push the variables, + * - calls @ref open_client to connect, + * - reads `proxy->server_capabilities`, + * - prints a full before/after trace via `diag()`, + * - calls @ref verify_caps and forwards the verdict to `ok()`. + * + * @param cl Parsed CommandLine (host/port/credentials). + * @param admin Already-connected admin MYSQL* handle. + * @return `EXIT_SUCCESS` if every connection attempt succeeded (the TAP + * framework tracks per-assertion pass/fail separately). + */ +static int run_scenarios(const CommandLine& cl, MYSQL* admin) { + const scenario_t scenarios[] = { + { + "both_off", + /* deprecate_eof */ false, + /* session_tracking */ false, + /* client_flag */ 0, + /* expectation */ { + /* present */ {}, + /* absent */ { CLIENT_DEPRECATE_EOF, CLIENT_SESSION_TRACKING }, + }, + }, + { + "both_on", + /* deprecate_eof */ true, + /* session_tracking */ true, + /* client_flag */ CLIENT_DEPRECATE_EOF, + /* expectation */ { + /* present */ { CLIENT_DEPRECATE_EOF, CLIENT_SESSION_TRACKING }, + /* absent */ {}, + }, + }, + { + "eof_on_tracking_off", + /* deprecate_eof */ true, + /* session_tracking */ false, + /* client_flag */ CLIENT_DEPRECATE_EOF, + /* expectation */ { + /* present */ { CLIENT_DEPRECATE_EOF }, + /* absent */ { CLIENT_SESSION_TRACKING }, + }, + }, + { + "eof_off_tracking_on", + /* deprecate_eof */ false, + /* session_tracking */ true, + /* client_flag */ 0, + /* expectation */ { + /* present */ { CLIENT_SESSION_TRACKING }, + /* absent */ { CLIENT_DEPRECATE_EOF }, + }, + }, + }; + + int scenario_idx = 0; + for (const scenario_t& s : scenarios) { + ++scenario_idx; + diag("================================================================"); + diag("Scenario %d/%zu: %s (deprecate_eof=%d, session_tracking=%d)", + scenario_idx, + sizeof(scenarios) / sizeof(scenarios[0]), + s.name, s.deprecate_eof ? 1 : 0, s.session_tracking ? 1 : 0); + diag("================================================================"); + + if (!apply_runtime_toggles(admin, s.deprecate_eof, s.session_tracking)) { + ok(false, "scenario '%s': failed to apply runtime toggles", s.name); + continue; + } - proxy = mysql_init(NULL); + MYSQL* proxy = open_client(cl, s.client_flag); + if (proxy == NULL) { + ok(false, "scenario '%s': failed to connect to ProxySQL", s.name); + continue; + } - if (!mysql_real_connect(proxy, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) { - fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxy)); - return EXIT_FAILURE; - } + const uint64_t caps = proxy->server_capabilities; + diag(" server_capabilities (raw): 0x%08llx (%llu)", + static_cast(caps), + static_cast(caps)); + diag(" server_capabilities (decoded): %s", caps_to_string(caps).c_str()); - caps_res = check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF }, false); - caps_res.first &= (check_server_capabilities(proxy, { CLIENT_SESSION_TRACKING }, true).first); - ext_caps = (proxy->server_capabilities >> 16) << 16; + const bool verdict = verify_caps(caps, s.expectation); - ok( - caps_res.first, "ProxySQL greeting with deprecate_eof disabled and session_tracking enabled - Exp: '%ld', Act: '%ld'", - caps_res.second, ext_caps - ); + ok(verdict, + "scenario '%s' (deprecate_eof=%d, session_tracking=%d): greeting capabilities match expectation", + s.name, s.deprecate_eof ? 1 : 0, s.session_tracking ? 1 : 0); - mysql_close(proxy); + mysql_close(proxy); + } return EXIT_SUCCESS; } +/** + * @brief Test entry point. + * + * Establishes the admin connection once, plans the TAP counter, then hands + * over to @ref run_scenarios. Admin handle is closed before returning. + */ int main(int argc, char** argv) { + (void) argc; + (void) argv; + CommandLine cl; plan(4); @@ -172,16 +445,26 @@ int main(int argc, char** argv) { return EXIT_FAILURE; } + diag("ProxySQL connection target: %s:%d (user=%s)", + cl.host, cl.port, cl.username); + diag("ProxySQL admin target: %s:%d (user=%s)", + cl.admin_host, cl.admin_port, cl.admin_username); + MYSQL* admin = mysql_init(NULL); + if (admin == NULL) { + diag("mysql_init for admin returned NULL"); + return EXIT_FAILURE; + } - if (!mysql_real_connect(admin, cl.host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0)) { - fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin)); + if (!mysql_real_connect(admin, cl.admin_host, cl.admin_username, + cl.admin_password, NULL, cl.admin_port, NULL, 0)) { + diag("admin mysql_real_connect failed: %s", mysql_error(admin)); + mysql_close(admin); return EXIT_FAILURE; } - test_proxy_capabilites(cl, admin); + run_scenarios(cl, admin); mysql_close(admin); - return exit_status(); } From 6b311b7014567eada827f6c0476d149df103a6d7 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 17 Apr 2026 15:51:32 +0000 Subject: [PATCH 4/5] Restore greeting upper-word caps via local 'extended_capabilities' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous commit (536c9d916) OR-ed the dropped upper-word bits into 'mysql_thread___server_capabilities', which made them leak into '(*myds)->myconn->options.server_capabilities' — a behaviour delta vs. pre-8c6a6444d. Revert that and reinstate the original pattern: a local 'extended_capabilities' whose only consumer is 'upper_word'. Seed it with the hardcoded baseline (CLIENT_MULTI_*, CLIENT_PLUGIN_AUTH, CLIENT_REMEMBER_OPTIONS) and fold in any other upper-word bits currently set in 'server_capabilities' (CLIENT_DEPRECATE_EOF, CLIENT_SESSION_TRACKING, CLIENT_ZSTD_COMPRESSION, ...) so the greeting stays in sync with the per-session / per-toggle state — preserving 8c6a6444d's zstd advertisement while restoring the #4023 baseline. --- lib/MySQL_Protocol.cpp | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/lib/MySQL_Protocol.cpp b/lib/MySQL_Protocol.cpp index 0d9f41915..c8747fe38 100644 --- a/lib/MySQL_Protocol.cpp +++ b/lib/MySQL_Protocol.cpp @@ -1088,13 +1088,6 @@ bool MySQL_Protocol::generate_pkt_initial_handshake(bool send, void **ptr, unsig } mysql_thread___server_capabilities |= CLIENT_LONG_FLAG; mysql_thread___server_capabilities |= CLIENT_MYSQL | CLIENT_PLUGIN_AUTH | CLIENT_RESERVED; - // Upper-word capabilities that ProxySQL always advertises, matching real - // MySQL server greetings (see issue #4023). These were previously supplied - // via a separate 'extended_capabilities' local and were accidentally - // dropped during the zstd refactor in 8c6a6444d. - mysql_thread___server_capabilities |= - CLIENT_MULTI_STATEMENTS | CLIENT_MULTI_RESULTS | - CLIENT_PS_MULTI_RESULTS | CLIENT_REMEMBER_OPTIONS; if (mysql_thread___enable_client_deprecate_eof) { mysql_thread___server_capabilities |= CLIENT_DEPRECATE_EOF; } else { @@ -1125,8 +1118,21 @@ bool MySQL_Protocol::generate_pkt_initial_handshake(bool send, void **ptr, unsig uint8_t uint8_charset = ci->nr & 255; memcpy(_ptr+l,&uint8_charset, sizeof(uint8_charset)); l+=sizeof(uint8_charset); memcpy(_ptr+l,&server_status, sizeof(server_status)); l+=sizeof(server_status); - // Copy the upper 16 capability bits from the effective server capability mask. - uint16_t upper_word = static_cast(server_capabilities >> 16); + // Upper-word ('capability_flags_2') capabilities advertised in the greeting. + // Baseline bits match what real MySQL servers advertise (see issue #4023); + // they were accidentally dropped during the zstd refactor in 8c6a6444d. + // The remaining upper-word bits (CLIENT_DEPRECATE_EOF, CLIENT_SESSION_TRACKING, + // CLIENT_ZSTD_COMPRESSION, ...) are folded in from 'server_capabilities' so + // the greeting stays in sync with the per-session / per-toggle state. + // 'extended_capabilities' is intentionally a local: it must NOT leak into + // '(*myds)->myconn->options.server_capabilities' nor into the low-word + // memcpy above, which record the per-connection state rather than the full + // greeting. + uint32_t extended_capabilities = + CLIENT_MULTI_RESULTS | CLIENT_MULTI_STATEMENTS | CLIENT_PS_MULTI_RESULTS | + CLIENT_PLUGIN_AUTH | CLIENT_REMEMBER_OPTIONS; + extended_capabilities |= server_capabilities & 0xFFFF0000u; + uint16_t upper_word = static_cast(extended_capabilities >> 16); memcpy(_ptr+l, static_cast(&upper_word), sizeof(upper_word)); l += sizeof(upper_word); // Copy the 'auth_plugin_data_len'. Hardcoded due to 'CLIENT_PLUGIN_AUTH' always enabled and reported // as 'mysql_native_password'. From 4d251c80ec0d9026175d2525e6ffce9d7f1cd52d Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 17 Apr 2026 16:23:27 +0000 Subject: [PATCH 5/5] Revert 'enable_client_session_tracking' variable; keep greeting fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #5621 mis-diagnosed "CLIENT_SESSION_TRACKING missing from the greeting" as a long-standing configuration gap and proposed a new 'mysql-enable_client_session_tracking' variable analogous to 'mysql-enable_client_deprecate_eof'. On closer inspection this was in fact a regression introduced by commit 8c6a6444d (zstd refactor), which dropped the hard-coded upper-word bits from the greeting. The right fix is simply to restore those hard-coded bits — commits 536c9d916 and 6b311b701 already do that via the 'extended_capabilities' local in 'generate_pkt_initial_handshake'. The new variable adds no value on top of that: the hard-coded bits cannot be disabled through 'mysql-server_capabilities' (which is OR-combined with the greeting), and there is no protocol hazard motivating a per-instance off-switch for CLIENT_SESSION_TRACKING, unlike CLIENT_DEPRECATE_EOF. Revert the variable (MySQL_Thread.h / proxysql_structs.h / MySQL_Thread.cpp / MySQL_Protocol.cpp) and restore CLIENT_SESSION_TRACKING to the 'extended_capabilities' literal, matching v3.0.6 exactly. Keep the test's doxygen / diag / baseline-check rewrite from the investigation — it's strictly better coverage and will catch any future regression of the hard-coded bits early. Test plan reduced to 2 scenarios (deprecate_eof on/off) since session_tracking is now unconditionally advertised. --- include/MySQL_Thread.h | 1 - include/proxysql_structs.h | 2 - lib/MySQL_Protocol.cpp | 44 +++-- lib/MySQL_Thread.cpp | 4 - .../tests/test_greeting_capabilities-t.cpp | 160 ++++++++---------- 5 files changed, 89 insertions(+), 122 deletions(-) diff --git a/include/MySQL_Thread.h b/include/MySQL_Thread.h index bbae7ea09..6725ff730 100644 --- a/include/MySQL_Thread.h +++ b/include/MySQL_Thread.h @@ -584,7 +584,6 @@ class MySQL_Threads_Handler bool kill_backend_connection_when_disconnect; bool client_session_track_gtid; bool enable_client_deprecate_eof; - bool enable_client_session_tracking; bool enable_server_deprecate_eof; bool enable_load_data_local_infile; bool log_mysql_warnings_enabled; diff --git a/include/proxysql_structs.h b/include/proxysql_structs.h index 4036c6b1d..6f466ec14 100644 --- a/include/proxysql_structs.h +++ b/include/proxysql_structs.h @@ -1325,7 +1325,6 @@ __thread char * mysql_thread___default_variables[SQL_NAME_LAST_LOW_WM]; __thread int mysql_thread___query_digests_grouping_limit; __thread int mysql_thread___query_digests_groups_grouping_limit; __thread bool mysql_thread___enable_client_deprecate_eof; -__thread bool mysql_thread___enable_client_session_tracking; __thread bool mysql_thread___enable_server_deprecate_eof; __thread bool mysql_thread___log_mysql_warnings_enabled; __thread bool mysql_thread___enable_load_data_local_infile; @@ -1658,7 +1657,6 @@ extern __thread char * mysql_thread___default_variables[SQL_NAME_LAST_LOW_WM]; extern __thread int mysql_thread___query_digests_grouping_limit; extern __thread int mysql_thread___query_digests_groups_grouping_limit; extern __thread bool mysql_thread___enable_client_deprecate_eof; -extern __thread bool mysql_thread___enable_client_session_tracking; extern __thread bool mysql_thread___enable_server_deprecate_eof; extern __thread bool mysql_thread___log_mysql_warnings_enabled; extern __thread bool mysql_thread___enable_load_data_local_infile; diff --git a/lib/MySQL_Protocol.cpp b/lib/MySQL_Protocol.cpp index c8747fe38..67994c52c 100644 --- a/lib/MySQL_Protocol.cpp +++ b/lib/MySQL_Protocol.cpp @@ -1093,11 +1093,6 @@ bool MySQL_Protocol::generate_pkt_initial_handshake(bool send, void **ptr, unsig } else { mysql_thread___server_capabilities &= ~CLIENT_DEPRECATE_EOF; } - if (mysql_thread___enable_client_session_tracking) { - mysql_thread___server_capabilities |= CLIENT_SESSION_TRACKING; - } else { - mysql_thread___server_capabilities &= ~CLIENT_SESSION_TRACKING; - } uint32_t server_capabilities = mysql_thread___server_capabilities; if (deprecate_eof_active && mysql_thread___enable_client_deprecate_eof) { server_capabilities |= CLIENT_DEPRECATE_EOF; @@ -1119,18 +1114,18 @@ bool MySQL_Protocol::generate_pkt_initial_handshake(bool send, void **ptr, unsig memcpy(_ptr+l,&uint8_charset, sizeof(uint8_charset)); l+=sizeof(uint8_charset); memcpy(_ptr+l,&server_status, sizeof(server_status)); l+=sizeof(server_status); // Upper-word ('capability_flags_2') capabilities advertised in the greeting. - // Baseline bits match what real MySQL servers advertise (see issue #4023); - // they were accidentally dropped during the zstd refactor in 8c6a6444d. - // The remaining upper-word bits (CLIENT_DEPRECATE_EOF, CLIENT_SESSION_TRACKING, - // CLIENT_ZSTD_COMPRESSION, ...) are folded in from 'server_capabilities' so - // the greeting stays in sync with the per-session / per-toggle state. - // 'extended_capabilities' is intentionally a local: it must NOT leak into - // '(*myds)->myconn->options.server_capabilities' nor into the low-word - // memcpy above, which record the per-connection state rather than the full - // greeting. + // These match what real MySQL servers advertise (see issue #4023) and were + // accidentally dropped during the zstd refactor in 8c6a6444d; this local + // restores the baseline. 'extended_capabilities' is intentionally a local: + // it must NOT leak into '(*myds)->myconn->options.server_capabilities' nor + // into the low-word memcpy above, which record per-connection state rather + // than the full greeting. Per-session/per-toggle upper-word bits (e.g. + // CLIENT_DEPRECATE_EOF when 'deprecate_eof_active', CLIENT_ZSTD_COMPRESSION + // when 'have_compress') are folded in from 'server_capabilities' so the + // greeting stays in sync with their runtime state. uint32_t extended_capabilities = CLIENT_MULTI_RESULTS | CLIENT_MULTI_STATEMENTS | CLIENT_PS_MULTI_RESULTS | - CLIENT_PLUGIN_AUTH | CLIENT_REMEMBER_OPTIONS; + CLIENT_PLUGIN_AUTH | CLIENT_SESSION_TRACKING | CLIENT_REMEMBER_OPTIONS; extended_capabilities |= server_capabilities & 0xFFFF0000u; uint16_t upper_word = static_cast(extended_capabilities >> 16); memcpy(_ptr+l, static_cast(&upper_word), sizeof(upper_word)); l += sizeof(upper_word); @@ -1607,19 +1602,18 @@ bool MySQL_Protocol::PPHR_2(unsigned char *pkt, unsigned int len, bool& ret, MyP if (vars1.capabilities & CLIENT_MULTI_STATEMENTS) { vars1.capabilities |= CLIENT_MULTI_RESULTS; } - // Enforce disabling 'CLIENT_DEPRECATE_EOF' and 'CLIENT_SESSION_TRACKING' from the - // supported capabilities when explicitly disabled via - // 'mysql_thread___enable_client_deprecate_eof' / 'mysql_thread___enable_client_session_tracking'. - // Downstream logic that treats the connection as supporting either capability relies on - // the 'client_flag' field of 'MySQL_Connection::options'. This is the second step of a - // two-step enforcement: the first step omits these bits from 'server_capabilities' - // during the initial handshake (see 'generate_pkt_initial_handshake'). + // we enforce disabling 'CLIENT_DEPRECATE_EOF' from the supported capabilities + // in case it's explicitly disabled by global variable 'mysql_thread___enable_client_deprecate_eof'. + // This is because further checks to actually threat the connection as a connection + // supporting 'CLIENT_DEPRECATE_EOF' rely in 'client_flag' field from + // 'MySQL_Connection::options'. + // This is the second step for ensuring that the connection is being handling + // in both ProxySQL and client side as a connection without 'CLIENT_DEPRECATE_EOF' support. + // First step is replying to client during initial handshake (in 'generate_pkt_initial_handshake') + // specifying no 'CLIENT_DEPRECATE_EOF' support in 'server_capabilities'. if (!mysql_thread___enable_client_deprecate_eof) { vars1.capabilities &= ~CLIENT_DEPRECATE_EOF; } - if (!mysql_thread___enable_client_session_tracking) { - vars1.capabilities &= ~CLIENT_SESSION_TRACKING; - } (*myds)->myconn->options.client_flag = vars1.capabilities; pkt += sizeof(uint32_t); vars1.max_pkt = CPY4(pkt); diff --git a/lib/MySQL_Thread.cpp b/lib/MySQL_Thread.cpp index 151806f46..079f108dc 100644 --- a/lib/MySQL_Thread.cpp +++ b/lib/MySQL_Thread.cpp @@ -358,7 +358,6 @@ static char * mysql_thread_variables_names[]= { (char *)"connect_timeout_server", (char *)"connect_timeout_server_max", (char *)"enable_client_deprecate_eof", - (char *)"enable_client_session_tracking", (char *)"enable_server_deprecate_eof", (char *)"enable_load_data_local_infile", (char *)"eventslog_filename", @@ -1442,7 +1441,6 @@ MySQL_Threads_Handler::MySQL_Threads_Handler() { variables.query_digests_grouping_limit = 3; variables.query_digests_groups_grouping_limit= 10; // changed in 2.6.0 , was 0 variables.enable_client_deprecate_eof=true; - variables.enable_client_session_tracking=true; variables.enable_server_deprecate_eof=true; variables.enable_load_data_local_infile=false; variables.log_mysql_warnings_enabled=false; @@ -2507,7 +2505,6 @@ char ** MySQL_Threads_Handler::get_variables_list() { VariablesPointers_bool["connection_warming"] = make_tuple(&variables.connection_warming, false); VariablesPointers_bool["default_reconnect"] = make_tuple(&variables.default_reconnect, false); VariablesPointers_bool["enable_client_deprecate_eof"] = make_tuple(&variables.enable_client_deprecate_eof, false); - VariablesPointers_bool["enable_client_session_tracking"] = make_tuple(&variables.enable_client_session_tracking, false); VariablesPointers_bool["enable_server_deprecate_eof"] = make_tuple(&variables.enable_server_deprecate_eof, false); #ifdef PROXYSQLFFTO VariablesPointers_bool["ffto_enabled"] = make_tuple(&variables.ffto_enabled, false); @@ -4746,7 +4743,6 @@ void MySQL_Thread::refresh_variables() { REFRESH_VARIABLE_BOOL(servers_stats); REFRESH_VARIABLE_BOOL(default_reconnect); REFRESH_VARIABLE_BOOL(enable_client_deprecate_eof); - REFRESH_VARIABLE_BOOL(enable_client_session_tracking); REFRESH_VARIABLE_BOOL(enable_server_deprecate_eof); REFRESH_VARIABLE_BOOL(enable_load_data_local_infile); REFRESH_VARIABLE_BOOL(log_mysql_warnings_enabled); diff --git a/test/tap/tests/test_greeting_capabilities-t.cpp b/test/tap/tests/test_greeting_capabilities-t.cpp index d0bf4b408..43b1805b5 100644 --- a/test/tap/tests/test_greeting_capabilities-t.cpp +++ b/test/tap/tests/test_greeting_capabilities-t.cpp @@ -11,32 +11,37 @@ * 32-bit capability bitmask that tells the client which protocol features the * server supports (see MySQL Client/Server Protocol documentation). * - * A subset of those capability bits are controlled by runtime ProxySQL - * variables and can therefore be toggled at will: + * Most of those bits are hard-coded to always-on values that match what a + * real MySQL server advertises — see @ref BASELINE_PRESENT_CAPS. One bit, + * `CLIENT_DEPRECATE_EOF`, is controlled at runtime by + * `mysql-enable_client_deprecate_eof`, because enabling it changes the wire + * protocol (no EOF packets) and not every client is ready for that. * - * - CLIENT_DEPRECATE_EOF -> `mysql-enable_client_deprecate_eof` - * - CLIENT_SESSION_TRACKING -> `mysql-enable_client_session_tracking` - * - * This test exercises all four combinations of those two toggles and checks - * the resulting capability bitmask returned by ProxySQL. + * Regression protection + * --------------------- + * The hard-coded bits were accidentally dropped during the zstd support + * refactor (commit `8c6a6444d`, shipped in v3.0.7). This regression manifested + * as `java.lang.ArrayIndexOutOfBoundsException` on JDBC clients relying on + * `CLIENT_SESSION_TRACKING` (see issues #4023 and #5621). The baseline check + * in @ref verify_caps is specifically designed to catch any future removal of + * those bits early. * * Test strategy * ------------- - * For each combination `{deprecate_eof, session_tracking}` in - * `{off,off} / {on,on} / {on,off} / {off,on}`: + * The test runs two scenarios covering the `mysql-enable_client_deprecate_eof` + * toggle (`on` / `off`). Each scenario: * - * 1. Set the two variables on the ProxySQL admin interface. - * 2. Apply the variables to runtime (`LOAD MYSQL VARIABLES TO RUNTIME`). - * 3. Open a fresh client connection against ProxySQL (mysql_real_connect). - * 4. Read `mysql->server_capabilities` (populated by libmariadb from the + * 1. Sets the variable on the ProxySQL admin interface. + * 2. Applies it to runtime (`LOAD MYSQL VARIABLES TO RUNTIME`). + * 3. Opens a fresh client connection against ProxySQL (`mysql_real_connect`). + * 4. Reads `mysql->server_capabilities` (populated by libmariadb from the * greeting packet). - * 5. Check that every capability in the expectation's `present_caps` is set - * and every capability in `absent_caps` is cleared. - * 6. Emit a single TAP `ok()` with a verbose message and `diag()` traces for - * every intermediate value so a failure can be debugged without attaching - * a protocol analyzer. + * 5. Checks that every bit in @ref BASELINE_PRESENT_CAPS is set, plus the + * scenario-specific expectation on `CLIENT_DEPRECATE_EOF`. + * 6. Emits a TAP `ok()` plus `diag()` traces for every intermediate value so + * a failure can be debugged without attaching a protocol analyzer. * - * The suite keeps running after a failure so that a single run surfaces every + * The suite keeps running after a failure so a single run surfaces every * broken scenario at once. */ @@ -151,21 +156,30 @@ static string caps_to_string(uint64_t caps) { } /** - * @brief Capability bits ProxySQL MUST advertise in every greeting, regardless - * of any runtime toggle. + * @brief Capability bits ProxySQL MUST advertise in every greeting. + * + * These are hard-coded into the `extended_capabilities` local in + * `MySQL_Protocol::generate_pkt_initial_handshake` and match what a real + * MySQL 8.x server advertises. Several clients rely on them — in particular + * JDBC needs `CLIENT_SESSION_TRACKING` to parse GTID data from OK packets, + * and PS_MULTI_RESULTS is required for stored procedures returning multiple + * result sets. * - * These match what a real MySQL 8.x server advertises in its Server Greeting - * and are relied upon by several clients (JDBC in particular — see issue - * #4023). Regressing any of them typically manifests as - * `java.lang.ArrayIndexOutOfBoundsException` on the client side. + * These bits are intentionally NOT configurable through + * `mysql-server_capabilities`: that admin variable is OR-combined with the + * greeting, so it can only *add* bits, never remove them — precisely so a + * misconfigured runtime cannot strip protocol-critical capabilities. * - * This list is checked against every scenario via @ref verify_caps. + * Regressing any of these bits typically manifests as + * `java.lang.ArrayIndexOutOfBoundsException` on JDBC clients (issue #4023) + * or as clients silently losing session-tracking data (issue #5621). */ static const vector BASELINE_PRESENT_CAPS = { CLIENT_MULTI_STATEMENTS, CLIENT_MULTI_RESULTS, CLIENT_PS_MULTI_RESULTS, CLIENT_PLUGIN_AUTH, + CLIENT_SESSION_TRACKING, CLIENT_REMEMBER_OPTIONS, }; @@ -175,8 +189,8 @@ static const vector BASELINE_PRESENT_CAPS = { * * A test scenario supplies one `caps_expectation_t` describing ONLY the bits * it cares to toggle. Bits in @ref present_caps MUST be set in - * `mysql->server_capabilities`; bits in @ref absent_caps MUST be cleared. Bits - * appearing in neither list are ignored here because ProxySQL's baseline + * `mysql->server_capabilities`; bits in @ref absent_caps MUST be cleared. + * Bits appearing in neither list are ignored here because ProxySQL's baseline * greeting also flips bits like `CLIENT_COMPRESS`, `CLIENT_SSL`, etc. whose * values depend on global configuration unrelated to the scenario. * @@ -234,20 +248,18 @@ static bool verify_caps(uint64_t actual_caps, const caps_expectation_t& exp) { } /** - * @brief Apply the two runtime toggles under test and reload them. + * @brief Apply the `mysql-enable_client_deprecate_eof` toggle and reload. * - * Runs three admin statements: + * Runs two admin statements: * - `SET mysql-enable_client_deprecate_eof = <0|1>` - * - `SET mysql-enable_client_session_tracking = <0|1>` * - `LOAD MYSQL VARIABLES TO RUNTIME` * - * @param admin Already-connected admin MYSQL* handle. - * @param deprecate_eof Desired value for `mysql-enable_client_deprecate_eof`. - * @param session_tracking Desired value for `mysql-enable_client_session_tracking`. - * @return `true` on success, `false` if any statement failed - * (the error is printed via `diag()` before returning). + * @param admin Already-connected admin MYSQL* handle. + * @param deprecate_eof Desired value for `mysql-enable_client_deprecate_eof`. + * @return `true` on success, `false` if any statement failed + * (the error is printed via `diag()` before returning). */ -static bool apply_runtime_toggles(MYSQL* admin, bool deprecate_eof, bool session_tracking) { +static bool apply_deprecate_eof(MYSQL* admin, bool deprecate_eof) { char stmt[256]; snprintf(stmt, sizeof(stmt), @@ -258,14 +270,6 @@ static bool apply_runtime_toggles(MYSQL* admin, bool deprecate_eof, bool session return false; } - snprintf(stmt, sizeof(stmt), - "SET mysql-enable_client_session_tracking=%d", session_tracking ? 1 : 0); - diag(" admin> %s", stmt); - if (mysql_query(admin, stmt)) { - diag(" admin query failed: %s", mysql_error(admin)); - return false; - } - const char* reload = "LOAD MYSQL VARIABLES TO RUNTIME"; diag(" admin> %s", reload); if (mysql_query(admin, reload)) { @@ -314,16 +318,14 @@ static MYSQL* open_client(const CommandLine& cl, uint64_t client_flag) { /** * @struct scenario_t - * @brief One permutation of the runtime toggles plus its expectation. + * @brief One permutation of the `deprecate_eof` toggle plus its expectation. * - * Keeping the four scenarios in a table makes the main loop small and the - * list of cases easy to extend: adding a fifth scenario only requires a new - * row here, not more code. + * Keeping the scenarios in a table makes the main loop small and the list of + * cases easy to extend: adding a new row here is all that's needed. */ struct scenario_t { const char* name; ///< Short label used in TAP + diag. bool deprecate_eof; ///< Value of the runtime toggle. - bool session_tracking; ///< Value of the runtime toggle. uint64_t client_flag; ///< Extra `client_flag` the client advertises before connect. caps_expectation_t expectation; ///< Bits required/forbidden in the greeting. }; @@ -332,7 +334,7 @@ struct scenario_t { * @brief Execute every scenario once, emitting one TAP assertion per run. * * The function: - * - calls @ref apply_runtime_toggles to push the variables, + * - calls @ref apply_deprecate_eof to push the variable, * - calls @ref open_client to connect, * - reads `proxy->server_capabilities`, * - prints a full before/after trace via `diag()`, @@ -346,43 +348,21 @@ struct scenario_t { static int run_scenarios(const CommandLine& cl, MYSQL* admin) { const scenario_t scenarios[] = { { - "both_off", - /* deprecate_eof */ false, - /* session_tracking */ false, - /* client_flag */ 0, - /* expectation */ { + "deprecate_eof_off", + /* deprecate_eof */ false, + /* client_flag */ 0, + /* expectation */ { /* present */ {}, - /* absent */ { CLIENT_DEPRECATE_EOF, CLIENT_SESSION_TRACKING }, - }, - }, - { - "both_on", - /* deprecate_eof */ true, - /* session_tracking */ true, - /* client_flag */ CLIENT_DEPRECATE_EOF, - /* expectation */ { - /* present */ { CLIENT_DEPRECATE_EOF, CLIENT_SESSION_TRACKING }, - /* absent */ {}, + /* absent */ { CLIENT_DEPRECATE_EOF }, }, }, { - "eof_on_tracking_off", - /* deprecate_eof */ true, - /* session_tracking */ false, - /* client_flag */ CLIENT_DEPRECATE_EOF, - /* expectation */ { + "deprecate_eof_on", + /* deprecate_eof */ true, + /* client_flag */ CLIENT_DEPRECATE_EOF, + /* expectation */ { /* present */ { CLIENT_DEPRECATE_EOF }, - /* absent */ { CLIENT_SESSION_TRACKING }, - }, - }, - { - "eof_off_tracking_on", - /* deprecate_eof */ false, - /* session_tracking */ true, - /* client_flag */ 0, - /* expectation */ { - /* present */ { CLIENT_SESSION_TRACKING }, - /* absent */ { CLIENT_DEPRECATE_EOF }, + /* absent */ {}, }, }, }; @@ -391,14 +371,14 @@ static int run_scenarios(const CommandLine& cl, MYSQL* admin) { for (const scenario_t& s : scenarios) { ++scenario_idx; diag("================================================================"); - diag("Scenario %d/%zu: %s (deprecate_eof=%d, session_tracking=%d)", + diag("Scenario %d/%zu: %s (deprecate_eof=%d)", scenario_idx, sizeof(scenarios) / sizeof(scenarios[0]), - s.name, s.deprecate_eof ? 1 : 0, s.session_tracking ? 1 : 0); + s.name, s.deprecate_eof ? 1 : 0); diag("================================================================"); - if (!apply_runtime_toggles(admin, s.deprecate_eof, s.session_tracking)) { - ok(false, "scenario '%s': failed to apply runtime toggles", s.name); + if (!apply_deprecate_eof(admin, s.deprecate_eof)) { + ok(false, "scenario '%s': failed to apply runtime toggle", s.name); continue; } @@ -417,8 +397,8 @@ static int run_scenarios(const CommandLine& cl, MYSQL* admin) { const bool verdict = verify_caps(caps, s.expectation); ok(verdict, - "scenario '%s' (deprecate_eof=%d, session_tracking=%d): greeting capabilities match expectation", - s.name, s.deprecate_eof ? 1 : 0, s.session_tracking ? 1 : 0); + "scenario '%s' (deprecate_eof=%d): greeting capabilities match expectation", + s.name, s.deprecate_eof ? 1 : 0); mysql_close(proxy); } @@ -438,7 +418,7 @@ int main(int argc, char** argv) { CommandLine cl; - plan(4); + plan(2); if (cl.getEnv()) { diag("Failed to get the required environmental variables.");