diff --git a/lib/Admin_Handler.cpp b/lib/Admin_Handler.cpp index 24e361074..22ab397b4 100644 --- a/lib/Admin_Handler.cpp +++ b/lib/Admin_Handler.cpp @@ -5294,42 +5294,74 @@ __end_show_commands: if ((query_no_space_length > 30) && !strncasecmp("IMPORT PGBOUNCER CONFIG FROM ", query_no_space, 29)) { proxy_info("Received %s command\n", query_no_space); - // Parse command: extract file path and flags + // Only allow from admin sessions, not stats + if (sess->session_type != PROXYSQL_SESSION_ADMIN) { + SPA->send_error_msg_to_client(sess, (char *)"IMPORT PGBOUNCER CONFIG requires admin privileges"); + run_query = false; + goto __run_query; + } + + // Parse flags as trailing tokens (after the quoted path). + // Syntax: IMPORT PGBOUNCER CONFIG FROM '/path' [DRY RUN] [IGNORE WARNINGS] + // Extract the quoted path first, then check remaining tokens for flags. std::string cmd_rest = std::string(query_no_space + 29); bool dry_run = false; bool ignore_warnings = false; - // Check for DRY RUN and IGNORE WARNINGS flags (case-insensitive) + // Find the file path (first quoted or unquoted token) + std::string file_path_str; + std::string remaining; { - std::string upper_rest = cmd_rest; - std::transform(upper_rest.begin(), upper_rest.end(), upper_rest.begin(), ::toupper); - if (upper_rest.find("DRY RUN") != std::string::npos) { + size_t start = cmd_rest.find_first_not_of(" \t"); + if (start == std::string::npos) { + SPA->send_error_msg_to_client(sess, (char *)"Missing file path"); + run_query = false; + goto __run_query; + } + if (cmd_rest[start] == '\'' || cmd_rest[start] == '"') { + char quote = cmd_rest[start]; + size_t end = cmd_rest.find(quote, start + 1); + if (end == std::string::npos) { + SPA->send_error_msg_to_client(sess, (char *)"Unterminated quote in file path"); + run_query = false; + goto __run_query; + } + file_path_str = cmd_rest.substr(start + 1, end - start - 1); + remaining = cmd_rest.substr(end + 1); + } else { + size_t end = cmd_rest.find_first_of(" \t", start); + if (end == std::string::npos) { + file_path_str = cmd_rest.substr(start); + } else { + file_path_str = cmd_rest.substr(start, end - start); + remaining = cmd_rest.substr(end); + } + } + } + + // Parse remaining tokens for flags + { + std::string upper_rem = remaining; + std::transform(upper_rem.begin(), upper_rem.end(), upper_rem.begin(), ::toupper); + // Remove extra whitespace for matching + // Check for "DRY RUN" and "IGNORE WARNINGS" as whole tokens + if (upper_rem.find("DRY RUN") != std::string::npos) { dry_run = true; - auto pos = upper_rest.find("DRY RUN"); - cmd_rest.erase(pos, 7); } - upper_rest = cmd_rest; - std::transform(upper_rest.begin(), upper_rest.end(), upper_rest.begin(), ::toupper); - if (upper_rest.find("IGNORE WARNINGS") != std::string::npos) { + if (upper_rem.find("IGNORE WARNINGS") != std::string::npos) { ignore_warnings = true; - auto pos = upper_rest.find("IGNORE WARNINGS"); - cmd_rest.erase(pos, 15); } } - char *path_buf = strdup(cmd_rest.c_str()); - char *file_path = trim_spaces_and_quotes_in_place(path_buf); - // Stage 1: Parse PgBouncer config PgBouncer::Config pgb_config; - bool parse_ok = PgBouncer::parse_config_file(std::string(file_path), pgb_config); + bool parse_ok = PgBouncer::parse_config_file(file_path_str, pgb_config); if (!parse_ok) { - std::string err_msg = "Failed to parse PgBouncer config: " + std::string(file_path); + std::string err_msg = "Failed to parse PgBouncer config: " + file_path_str; for (const auto& e : pgb_config.errors) { err_msg += "\n " + e.file + ":" + std::to_string(e.line) + ": " + e.message; } SPA->send_error_msg_to_client(sess, (char *)err_msg.c_str()); - free(path_buf); run_query = false; goto __run_query; } @@ -5345,16 +5377,15 @@ __end_show_commands: err_msg += "\n ERROR: " + e.message; } if (dry_run) { - err_msg += "\n\n" + PgBouncer::ConfigConverter::format_dry_run(result, file_path, strict); + err_msg += "\n\n" + PgBouncer::ConfigConverter::format_dry_run(result, file_path_str.c_str(), strict); } SPA->send_error_msg_to_client(sess, (char *)err_msg.c_str()); - free(path_buf); run_query = false; goto __run_query; } if (dry_run) { - std::string dry_output = PgBouncer::ConfigConverter::format_dry_run(result, file_path, strict); + std::string dry_output = PgBouncer::ConfigConverter::format_dry_run(result, file_path_str.c_str(), strict); std::string escaped = dry_output; size_t pos = 0; while ((pos = escaped.find('\'', pos)) != std::string::npos) { @@ -5365,27 +5396,35 @@ __end_show_commands: std::string select_q = "SELECT '" + escaped + "' AS dry_run_output"; query = l_strdup(select_q.c_str()); query_length = strlen(query) + 1; - free(path_buf); goto __run_query; } - // Execute: apply the conversion + // Execute atomically: wrap in transaction so partial failure rolls back + SPA->admindb->execute("BEGIN"); + bool exec_ok = true; for (const auto& entry : result.entries) { if (!entry.sql.empty()) { - SPA->admindb->execute(entry.sql.c_str()); + bool rc = SPA->admindb->execute(entry.sql.c_str()); + if (!rc) { + exec_ok = false; + SPA->admindb->execute("ROLLBACK"); + SPA->send_error_msg_to_client(sess, (char *)"Import failed during SQL execution; changes rolled back"); + break; + } } } - - std::string ok_msg = "PgBouncer config imported: " + - std::to_string(result.server_count) + " servers, " + - std::to_string(result.user_count) + " users, " + - std::to_string(result.rule_count) + " query rules, " + - std::to_string(result.variable_count) + " variables"; - if (!result.warnings.empty()) { - ok_msg += " (" + std::to_string(result.warnings.size()) + " warnings)"; + if (exec_ok) { + SPA->admindb->execute("COMMIT"); + std::string ok_msg = "PgBouncer config imported: " + + std::to_string(result.server_count) + " servers, " + + std::to_string(result.user_count) + " users, " + + std::to_string(result.rule_count) + " query rules, " + + std::to_string(result.variable_count) + " variables"; + if (!result.warnings.empty()) { + ok_msg += " (" + std::to_string(result.warnings.size()) + " warnings)"; + } + SPA->send_ok_msg_to_client(sess, (char *)ok_msg.c_str(), 0, query_no_space); } - SPA->send_ok_msg_to_client(sess, (char *)ok_msg.c_str(), 0, query_no_space); - free(path_buf); run_query = false; goto __run_query; } diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp index 57b92005f..17a0bb3b8 100644 --- a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp +++ b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp @@ -375,6 +375,13 @@ void ConfigConverter::convert_hba_rules(const Config& config, bool any_converted = false; + if (!config.hba_rules.empty()) { + result.entries.push_back({ + "DELETE FROM pgsql_firewall_whitelist_rules;", + "Remove existing firewall whitelist rules before importing HBA rules" + }); + } + for (const auto& rule : config.hba_rules) { // Unsupported connection types if (rule.conn_type == "local") { diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp b/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp index 54852069b..5aafb72a5 100644 --- a/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp +++ b/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp @@ -4,6 +4,7 @@ #include #include #include +#include namespace PgBouncer { @@ -57,6 +58,7 @@ bool ConfigParser::parse_int(const std::string& value, int& result) { size_t pos = 0; long v = std::stol(value, &pos); if (pos != value.size()) return false; + if (v < INT_MIN || v > INT_MAX) return false; result = static_cast(v); return true; } catch (...) { @@ -70,6 +72,7 @@ bool ConfigParser::parse_uint(const std::string& value, unsigned int& result) { size_t pos = 0; unsigned long v = std::stoul(value, &pos); if (pos != value.size()) return false; + if (v > UINT_MAX) return false; result = static_cast(v); return true; } catch (...) { @@ -117,24 +120,27 @@ bool ConfigParser::parse_connstr_pairs( // Quoted value: collect until unescaped closing quote // PgBouncer escapes single quotes by doubling: '' ++i; // skip opening quote - std::string raw = "'"; + bool closed = false; while (i < len) { if (connstr[i] == '\'') { if (i + 1 < len && connstr[i + 1] == '\'') { - raw += "''"; + value += '\''; i += 2; } else { // closing quote ++i; + closed = true; break; } } else { - raw += connstr[i]; + value += connstr[i]; ++i; } } - raw += "'"; - value = unquote(raw); + if (!closed) { + errors.push_back({file, line, "unterminated single quote in connection string value for key '" + key + "'"}); + return false; + } } else { // Unquoted value: read until whitespace size_t val_start = i; @@ -503,7 +509,7 @@ bool ConfigParser::parse_ini( { std::ifstream ifs(filepath); if (!ifs.is_open()) { - config.errors.push_back({"", 0, "cannot open file: " + filepath}); + config.errors.push_back({filepath, 0, "cannot open file: " + filepath}); return false; } diff --git a/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp b/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp index 09aa49074..2e25e65f6 100644 --- a/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp +++ b/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp @@ -71,8 +71,8 @@ static std::vector tokenize_upper(const std::string& s) { static std::string query_pools(bool extended) { std::string q = "SELECT " - "'default' AS database, " - "su.username AS user, " + "cp.srv_host AS database, " + "'-' AS user, " "0 AS cl_active, " "0 AS cl_waiting, " "0 AS cl_cancel_req, " @@ -83,20 +83,16 @@ static std::string query_pools(bool extended) { "0 AS sv_login, " "0 AS maxwait, " "0 AS maxwait_us, " - "CASE WHEN su.fast_forward=1 THEN 'session' " - "WHEN su.transaction_persistent=1 THEN 'transaction' " - "ELSE 'statement' END AS pool_mode"; + "'statement' AS pool_mode"; if (extended) { q += ", cp.hostgroup AS hostgroup_id" - ", cp.ConnUsed AS multiplex" + ", 1 AS multiplex" ", cp.Latency_us AS latency_us" ", cp.Queries AS Queries" ", cp.Bytes_data_sent AS Bytes_data_sent" ", cp.Bytes_data_recv AS Bytes_data_recv"; } - q += " FROM stats_pgsql_connection_pool cp" - " JOIN runtime_pgsql_users su ON 1=1" - " GROUP BY su.username"; + q += " FROM stats_pgsql_connection_pool cp"; return q; } diff --git a/lib/pgbouncer_compat/ProxySQL_CLI.cpp b/lib/pgbouncer_compat/ProxySQL_CLI.cpp index fe6e35829..c31af5abb 100644 --- a/lib/pgbouncer_compat/ProxySQL_CLI.cpp +++ b/lib/pgbouncer_compat/ProxySQL_CLI.cpp @@ -68,8 +68,10 @@ static int cmd_import_pgbouncer(int argc, const char* argv[]) { for (const auto& err : result.errors) { std::cerr << " ERROR: " << err.message << "\n"; } - // Still show the dry-run output so the user can see what would have been converted - std::cout << PgBouncer::ConfigConverter::format_dry_run(result, config_path, strict); + // Show dry-run output on stderr only (never stdout — it could be piped to mysql) + if (dry_run) { + std::cerr << "\n" << PgBouncer::ConfigConverter::format_dry_run(result, config_path, strict); + } return 1; } @@ -98,7 +100,7 @@ static int cmd_import_pgbouncer(int argc, const char* argv[]) { if (!entry.comment.empty()) { std::cout << "-- " << entry.comment << "\n"; } - std::cout << entry.sql << ";\n"; + std::cout << entry.sql << "\n"; } if (!result.warnings.empty()) {