From d036ba832732596f6d917d3047dfffcf0bc83ec8 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Tue, 7 Apr 2026 09:41:59 +0000 Subject: [PATCH] fix: canonicalize plugin command registration --- lib/ProxySQL_PluginManager.cpp | 78 ++++++++++++++++--- .../tap/tests/unit/plugin_registry_unit-t.cpp | 15 +++- 2 files changed, 80 insertions(+), 13 deletions(-) diff --git a/lib/ProxySQL_PluginManager.cpp b/lib/ProxySQL_PluginManager.cpp index 55cea552b..5e78f9449 100644 --- a/lib/ProxySQL_PluginManager.cpp +++ b/lib/ProxySQL_PluginManager.cpp @@ -1,5 +1,6 @@ #include "ProxySQL_PluginManager.h" +#include #include #include #include @@ -37,14 +38,26 @@ std::string plugin_name(const ProxySQL_PluginDescriptor *descriptor) { } void register_table_service(const ProxySQL_PluginTableDef& def) { - if (g_registry_target != nullptr && !g_registry_target->register_table(def)) { + if (g_registry_target == nullptr) { + proxy_warning("Plugin table registration attempted outside init phase for %s\n", + def.table_name != nullptr ? def.table_name : "(null)"); + return; + } + + if (!g_registry_target->register_table(def)) { proxy_warning("Plugin table registration failed for %s\n", def.table_name != nullptr ? def.table_name : "(null)"); } } void register_command_service(const char* sql, proxysql_plugin_admin_command_cb cb) { - if (g_registry_target != nullptr && !g_registry_target->register_command(sql, cb)) { + if (g_registry_target == nullptr) { + proxy_warning("Plugin command registration attempted outside init phase for %s\n", + sql != nullptr ? sql : "(null)"); + return; + } + + if (!g_registry_target->register_command(sql, cb)) { proxy_warning("Plugin command registration failed for %s\n", sql != nullptr ? sql : "(null)"); } @@ -84,12 +97,42 @@ bool sql_equals_ci(const std::string& lhs, const std::string& rhs) { return strcasecmp(lhs.c_str(), rhs.c_str()) == 0; } -bool has_plugin_command_prefix(const char* sql) { - if (sql == nullptr) { +std::string canonicalize_plugin_command(const std::string& sql) { + size_t start = 0; + size_t end = sql.size(); + while (start < end && std::isspace(static_cast(sql[start]))) { + ++start; + } + while (end > start && + (std::isspace(static_cast(sql[end - 1])) || sql[end - 1] == ';')) { + --end; + } + + std::string normalized {}; + normalized.reserve(end - start); + bool pending_space = false; + for (size_t i = start; i < end; ++i) { + const unsigned char ch = static_cast(sql[i]); + if (std::isspace(ch)) { + pending_space = !normalized.empty(); + continue; + } + if (pending_space) { + normalized.push_back(' '); + pending_space = false; + } + normalized.push_back(static_cast(ch)); + } + + return normalized; +} + +bool has_plugin_command_prefix(const std::string& sql) { + if (sql.empty()) { return false; } - return strncasecmp(sql, kPluginCommandPrefix, sizeof(kPluginCommandPrefix) - 1) == 0; + return strncasecmp(sql.c_str(), kPluginCommandPrefix, sizeof(kPluginCommandPrefix) - 1) == 0; } } // namespace @@ -241,18 +284,19 @@ const std::vector& ProxySQL_PluginManager::tables(Proxy } bool ProxySQL_PluginManager::dispatch_admin_command(const ProxySQL_PluginCommandContext& ctx, const std::string& sql, ProxySQL_PluginCommandResult& result) const { - if (!has_plugin_command_prefix(sql.c_str())) { + const std::string canonical_sql = canonicalize_plugin_command(sql); + if (!has_plugin_command_prefix(canonical_sql)) { return false; } for (const auto& command : commands_) { - if (!sql_equals_ci(command.sql, sql)) { + if (!sql_equals_ci(command.sql, canonical_sql)) { continue; } if (command.cb == nullptr) { return false; } - result = command.cb(ctx, sql.c_str()); + result = command.cb(ctx, canonical_sql.c_str()); return true; } @@ -268,8 +312,9 @@ bool ProxySQL_PluginManager::register_command_for_test(const std::string& sql) { } bool ProxySQL_PluginManager::has_command_for_test(const std::string& sql) const { + const std::string canonical_sql = canonicalize_plugin_command(sql); for (const auto& command : commands_) { - if (sql_equals_ci(command.sql, sql)) { + if (sql_equals_ci(command.sql, canonical_sql)) { return true; } } @@ -293,6 +338,8 @@ bool ProxySQL_PluginManager::register_table(const ProxySQL_PluginTableDef& def) case ProxySQL_PluginDBKind::stats_db: existing_tables = &tables_stats_; break; + default: + return false; } for (const auto& existing : *existing_tables) { @@ -319,23 +366,30 @@ bool ProxySQL_PluginManager::register_table(const ProxySQL_PluginTableDef& def) case ProxySQL_PluginDBKind::stats_db: tables_stats_.push_back(owned_def); break; + default: + return false; } return true; } bool ProxySQL_PluginManager::register_command(const char* sql, proxysql_plugin_admin_command_cb cb) { - if (sql == nullptr || *sql == '\0' || !has_plugin_command_prefix(sql) || cb == nullptr) { + if (sql == nullptr || *sql == '\0' || cb == nullptr) { + return false; + } + + const std::string canonical_sql = canonicalize_plugin_command(sql); + if (canonical_sql.empty() || !has_plugin_command_prefix(canonical_sql)) { return false; } for (const auto& command : commands_) { - if (strcasecmp(command.sql.c_str(), sql) == 0) { + if (sql_equals_ci(command.sql, canonical_sql)) { return false; } } - commands_.push_back({sql, cb}); + commands_.push_back({canonical_sql, cb}); return true; } diff --git a/test/tap/tests/unit/plugin_registry_unit-t.cpp b/test/tap/tests/unit/plugin_registry_unit-t.cpp index 665d53fa0..17efe89d4 100644 --- a/test/tap/tests/unit/plugin_registry_unit-t.cpp +++ b/test/tap/tests/unit/plugin_registry_unit-t.cpp @@ -13,7 +13,7 @@ ProxySQL_PluginCommandResult fake_plugin_command(const ProxySQL_PluginCommandCon } // namespace int main() { - plan(10); + plan(13); ProxySQL_PluginManager mgr; char table_name[] = "mysqlx_users"; @@ -40,10 +40,18 @@ int main() { ok(std::strcmp(mgr.tables(ProxySQL_PluginDBKind::admin_db).front().table_def, "CREATE TABLE mysqlx_users (username VARCHAR NOT NULL PRIMARY KEY)") == 0, "plugin admin table definition is copied"); mgr.register_table_for_test(duplicate_def); ok(mgr.tables(ProxySQL_PluginDBKind::admin_db).size() == static_cast(1), "duplicate table registration is rejected"); + ProxySQL_PluginTableDef invalid_def { + static_cast(255), + "invalid_table", + "CREATE TABLE invalid_table (id INTEGER)" + }; + mgr.register_table_for_test(invalid_def); + ok(mgr.tables(ProxySQL_PluginDBKind::admin_db).size() == static_cast(1), "invalid db kind is rejected"); ok(mgr.tables(ProxySQL_PluginDBKind::config_db).size() == static_cast(0), "config tables start empty"); ok(!mgr.register_command_for_test("SELECT 1"), "unnamespaced admin SQL is rejected"); ok(mgr.register_command("PLUGIN MYSQLX LOAD USERS TO RUNTIME", &fake_plugin_command), "namespaced command registration succeeds"); ok(!mgr.register_command("PLUGIN MYSQLX LOAD USERS TO RUNTIME", &fake_plugin_command), "duplicate namespaced command is rejected"); + ok(!mgr.register_command("PLUGIN MYSQLX LOAD USERS TO RUNTIME ;", &fake_plugin_command), "canonical duplicate command is rejected"); ok(mgr.has_command_for_test("PLUGIN MYSQLX LOAD USERS TO RUNTIME"), "registered command is discoverable"); ProxySQL_PluginCommandResult result { 1, 0, "" }; @@ -53,6 +61,11 @@ int main() { result.rows_affected == 7 && result.message == "mysqlx users loaded", "registered command dispatches callback result"); + ok(mgr.dispatch_admin_command(ctx, "PLUGIN MYSQLX LOAD USERS TO RUNTIME ;", result) && + result.error_code == 0 && + result.rows_affected == 7 && + result.message == "mysqlx users loaded", + "dispatch canonicalizes whitespace and trailing semicolons"); return exit_status(); }