From 03911488aa84311ffd94c4543c2ebd9ec2ddf227 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Tue, 20 Sep 2016 17:05:20 +0900 Subject: [PATCH] repmgr: in standby switchover, quote file paths in remotely executed commands Per suggestion from GitHub user sebasmannem (#229) --- repmgr.c | 226 ++++++++++++++++++++++++++++++++++-------------------- repmgr.h | 1 + strutil.c | 31 ++++++++ strutil.h | 3 + 4 files changed, 177 insertions(+), 84 deletions(-) diff --git a/repmgr.c b/repmgr.c index d81ca150..45e8b94f 100644 --- a/repmgr.c +++ b/repmgr.c @@ -54,7 +54,7 @@ #include #include "storage/fd.h" /* for PG_TEMP_FILE_PREFIX */ -#include "pqexpbuffer.h" + #include "log.h" #include "config.h" @@ -3709,6 +3709,8 @@ do_standby_switchover(void) char remote_conninfo[MAXCONNINFO] = ""; char remote_host[MAXLEN]; char remote_data_directory[MAXPGPATH] = ""; + char remote_path[MAXPGPATH] = ""; + int remote_node_id; char remote_node_replication_state[MAXLEN] = ""; char remote_archive_config_dir[MAXLEN]; @@ -3716,7 +3718,7 @@ do_standby_switchover(void) int i, r = 0; - char command[MAXLEN]; + PQExpBufferData remote_command_str; PQExpBufferData command_output; char repmgr_db_cli_params[MAXLEN] = ""; @@ -3815,17 +3817,25 @@ do_standby_switchover(void) log_debug("master's data directory is: %s\n", remote_data_directory); - maxlen_snprintf(command, - "ls %s/PG_VERSION >/dev/null 2>&1 && echo 1 || echo 0", + maxlen_snprintf(remote_path, + "%s/PG_VERSION", remote_data_directory); + + initPQExpBuffer(&remote_command_str); + appendPQExpBuffer(&remote_command_str, "ls "); + appendShellString(&remote_command_str, remote_path); + appendPQExpBuffer(&remote_command_str, " >/dev/null 2>&1 && echo 1 || echo 0"); + initPQExpBuffer(&command_output); (void)remote_command( remote_host, runtime_options.remote_user, - command, + remote_command_str.data, &command_output); + termPQExpBuffer(&remote_command_str); + if (*command_output.data == '1') { log_verbose(LOG_DEBUG, "PG_VERSION found in %s\n", remote_data_directory); @@ -3891,18 +3901,22 @@ do_standby_switchover(void) /* check pg_rewind actually exists on remote */ - maxlen_snprintf(command, - "ls -1 %s >/dev/null 2>&1 && echo 1 || echo 0", - remote_pg_rewind); + initPQExpBuffer(&remote_command_str); + + appendPQExpBuffer(&remote_command_str, "ls "); + appendShellString(&remote_command_str, remote_pg_rewind); + appendPQExpBuffer(&remote_command_str, " >/dev/null 2>&1 && echo 1 || echo 0"); initPQExpBuffer(&command_output); (void)remote_command( remote_host, runtime_options.remote_user, - command, + remote_command_str.data, &command_output); + termPQExpBuffer(&remote_command_str); + if (*command_output.data == '0') { log_err(_("unable to find pg_rewind on the remote server\n")); @@ -3969,25 +3983,29 @@ do_standby_switchover(void) runtime_options.remote_config_file, remote_host); - maxlen_snprintf(command, - "ls -1 %s >/dev/null 2>&1 && echo 1 || echo 0", - runtime_options.remote_config_file); + initPQExpBuffer(&remote_command_str); + appendPQExpBuffer(&remote_command_str, "ls "); + + appendShellString(&remote_command_str, runtime_options.remote_config_file); + appendPQExpBuffer(&remote_command_str, " >/dev/null 2>&1 && echo 1 || echo 0"); initPQExpBuffer(&command_output); (void)remote_command( remote_host, runtime_options.remote_user, - command, + remote_command_str.data, &command_output); + termPQExpBuffer(&remote_command_str); + if (*command_output.data == '0') { log_err(_("unable to find the specified repmgr configuration file on remote server\n")); exit(ERR_BAD_CONFIG); } - termPQExpBuffer(&command_output); + log_verbose(LOG_INFO, _("remote configuration file \"%s\" found on remote server\n"), runtime_options.remote_config_file); @@ -4023,18 +4041,22 @@ do_standby_switchover(void) log_verbose(LOG_INFO, _("checking \"%s\"\n"), config_paths[i]); - maxlen_snprintf(command, - "ls -1 %s >/dev/null 2>&1 && echo 1 || echo 0", - config_paths[i]); + initPQExpBuffer(&remote_command_str); + appendPQExpBuffer(&remote_command_str, "ls "); + + appendShellString(&remote_command_str, config_paths[i]); + appendPQExpBuffer(&remote_command_str, " >/dev/null 2>&1 && echo 1 || echo 0"); initPQExpBuffer(&command_output); (void)remote_command( remote_host, runtime_options.remote_user, - command, + remote_command_str.data, &command_output); + termPQExpBuffer(&remote_command_str); + if (*command_output.data == '1') { strncpy(runtime_options.remote_config_file, config_paths[i], MAXLEN); @@ -4076,23 +4098,27 @@ do_standby_switchover(void) log_verbose(LOG_DEBUG, "remote_archive_config_dir: %s\n", remote_archive_config_dir); - maxlen_snprintf(command, - "%s standby archive-config -f %s --config-archive-dir=%s", - make_pg_path("repmgr"), - runtime_options.remote_config_file, - remote_archive_config_dir); + initPQExpBuffer(&remote_command_str); + appendPQExpBuffer(&remote_command_str, + "%s standby archive-config -f ", + make_pg_path("repmgr")); + appendShellString(&remote_command_str, runtime_options.remote_config_file); + appendPQExpBuffer(&remote_command_str, + " --config-archive-dir="); + appendShellString(&remote_command_str, remote_archive_config_dir); - log_debug("Executing:\n%s\n", command); + log_debug("Executing:\n%s\n", remote_command_str.data); initPQExpBuffer(&command_output); (void)remote_command( remote_host, runtime_options.remote_user, - command, + remote_command_str.data, &command_output); termPQExpBuffer(&command_output); + termPQExpBuffer(&remote_command_str); } /* @@ -4111,22 +4137,25 @@ do_standby_switchover(void) * TODO * - notify repmgrd instances that this is a controlled * event so they don't initiate failover - * - optional "immediate" shutdown? - * -> use -F/--force? */ + initPQExpBuffer(&remote_command_str); + if (*options.stop_command) { - maxlen_snprintf(command, "%s", options.stop_command); + appendPQExpBuffer(&remote_command_str, "%s", options.stop_command); } else { - maxlen_snprintf(command, - "%s -D %s -m %s -W stop >/dev/null 2>&1 && echo 1 || echo 0", - make_pg_path("pg_ctl"), - remote_data_directory, - runtime_options.pg_ctl_mode); + appendPQExpBuffer(&remote_command_str, + "%s -D ", + make_pg_path("pg_ctl")); + appendShellString(&remote_command_str, remote_data_directory); + appendPQExpBuffer(&remote_command_str, + " -m %s -W stop >/dev/null 2>&1 && echo 1 || echo 0", + runtime_options.pg_ctl_mode); } + initPQExpBuffer(&command_output); // XXX handle failure @@ -4134,10 +4163,11 @@ do_standby_switchover(void) (void)remote_command( remote_host, runtime_options.remote_user, - command, + remote_command_str.data, &command_output); termPQExpBuffer(&command_output); + termPQExpBuffer(&remote_command_str); shutdown_success = false; @@ -4153,25 +4183,33 @@ do_standby_switchover(void) if (ping_res == PQPING_NO_RESPONSE) { bool command_success; - /* * directly access the server and check that the * pidfile has gone away so we can be sure the server is actually * shut down and the PQPING_NO_RESPONSE is not due to other issues * such as coincidental network failure. */ - initPQExpBuffer(&command_output); - maxlen_snprintf(command, - "ls %s/postmaster.pid >/dev/null 2>&1 && echo 1 || echo 0", - remote_data_directory); + maxlen_snprintf(remote_path, + "%s/postmaster.pid", + remote_data_directory); + + initPQExpBuffer(&remote_command_str); + + appendPQExpBuffer(&remote_command_str, "ls "); + appendShellString(&remote_command_str, remote_path); + appendPQExpBuffer(&remote_command_str, " >/dev/null 2>&1 && echo 1 || echo 0"); + + initPQExpBuffer(&command_output); command_success = remote_command( remote_host, runtime_options.remote_user, - command, + remote_command_str.data, &command_output); + termPQExpBuffer(&remote_command_str); + if (command_success == true && *command_output.data == '0') { shutdown_success = true; @@ -4214,17 +4252,17 @@ do_standby_switchover(void) if (use_pg_rewind == true) { - PQExpBufferData recovery_done_remove; - /* Execute pg_rewind */ - maxlen_snprintf(command, - "%s -D %s --source-server=\\'%s\\'", - remote_pg_rewind, - remote_data_directory, - options.conninfo); + + initPQExpBuffer(&remote_command_str); + + appendShellString(&remote_command_str, remote_pg_rewind); + appendPQExpBuffer(&remote_command_str, " -D "); + appendShellString(&remote_command_str, remote_data_directory); + appendPQExpBuffer(&remote_command_str, " --source-server=\\'%s\\'", options.conninfo); log_notice("Executing pg_rewind on old master server\n"); - log_debug("pg_rewind command is:\n%s\n", command); + log_debug("pg_rewind command is:\n%s\n", remote_command_str.data); initPQExpBuffer(&command_output); @@ -4233,17 +4271,22 @@ do_standby_switchover(void) (void)remote_command( remote_host, runtime_options.remote_user, - command, + remote_command_str.data, &command_output); termPQExpBuffer(&command_output); + termPQExpBuffer(&remote_command_str); /* Restore any previously archived config files */ - maxlen_snprintf(command, - "%s standby restore-config -D %s --config-archive-dir=%s", - make_pg_path("repmgr"), - remote_data_directory, - remote_archive_config_dir); + initPQExpBuffer(&remote_command_str); + + appendPQExpBuffer(&remote_command_str, + "%s standby restore-config -D ", + make_pg_path("repmgr")); + appendShellString(&remote_command_str, remote_data_directory); + appendPQExpBuffer(&remote_command_str, + " --config-archive-dir="); + appendShellString(&remote_command_str, remote_archive_config_dir); initPQExpBuffer(&command_output); @@ -4252,19 +4295,23 @@ do_standby_switchover(void) (void)remote_command( remote_host, runtime_options.remote_user, - command, + remote_command_str.data, &command_output); termPQExpBuffer(&command_output); + termPQExpBuffer(&remote_command_str); /* remove any recovery.done file copied in by pg_rewind */ + initPQExpBuffer(&remote_command_str); + maxlen_snprintf(remote_path, + "%s/recovery.done", + remote_data_directory); - initPQExpBuffer(&recovery_done_remove); + appendPQExpBuffer(&remote_command_str, "test -e "); + appendShellString(&remote_command_str, remote_path); + appendPQExpBuffer(&remote_command_str, " && rm -f "); + appendShellString(&remote_command_str, remote_path); - appendPQExpBuffer(&recovery_done_remove, - "test -e %s/recovery.done && rm -f %s/recovery.done", - remote_data_directory, - remote_data_directory); initPQExpBuffer(&command_output); // XXX handle failure @@ -4272,14 +4319,11 @@ do_standby_switchover(void) (void)remote_command( remote_host, runtime_options.remote_user, - recovery_done_remove.data, + remote_command_str.data, &command_output); termPQExpBuffer(&command_output); - termPQExpBuffer(&recovery_done_remove); - - - + termPQExpBuffer(&remote_command_str); } else { @@ -4294,28 +4338,36 @@ do_standby_switchover(void) * of pg_rewind. It's preferable to have `repmgr standby follow` start * the remote database as it can access the remote config file * directly. + * + * XXX will not work if runtime_options.remote_config_file is empty! */ - format_db_cli_params(options.conninfo, repmgr_db_cli_params); - maxlen_snprintf(command, - "%s -D %s -f %s %s --rsync-only --force --ignore-external-config-files standby clone", - make_pg_path("repmgr"), - remote_data_directory, - runtime_options.remote_config_file, - repmgr_db_cli_params - ); + initPQExpBuffer(&remote_command_str); - log_debug("Executing:\n%s\n", command); + format_db_cli_params(options.conninfo, repmgr_db_cli_params); + + appendPQExpBuffer(&remote_command_str, + "%s -D ", + make_pg_path("repmgr")); + appendShellString(&remote_command_str, remote_data_directory); + appendPQExpBuffer(&remote_command_str, " -f "); + appendShellString(&remote_command_str, runtime_options.remote_config_file); + appendPQExpBuffer(&remote_command_str, + " %s --rsync-only --force --ignore-external-config-files standby clone", + repmgr_db_cli_params); + + log_debug("Executing:\n%s\n", remote_command_str.data); initPQExpBuffer(&command_output); (void)remote_command( remote_host, runtime_options.remote_user, - command, + remote_command_str.data, &command_output); termPQExpBuffer(&command_output); + termPQExpBuffer(&remote_command_str); } /* @@ -4323,22 +4375,28 @@ do_standby_switchover(void) * the remote server */ format_db_cli_params(options.conninfo, repmgr_db_cli_params); - maxlen_snprintf(command, - "%s -D %s -f %s %s standby follow", - make_pg_path("repmgr"), - remote_data_directory, - runtime_options.remote_config_file, - repmgr_db_cli_params - ); - log_debug("Executing:\n%s\n", command); + initPQExpBuffer(&remote_command_str); + appendPQExpBuffer(&remote_command_str, + "%s -D ", + make_pg_path("repmgr")); + appendShellString(&remote_command_str, remote_data_directory); + appendPQExpBuffer(&remote_command_str, " -f "); + appendShellString(&remote_command_str, runtime_options.remote_config_file); + appendPQExpBuffer(&remote_command_str, + " %s standby follow", + repmgr_db_cli_params); + + log_debug("Executing:\n%s\n", remote_command_str.data); (void)remote_command( remote_host, runtime_options.remote_user, - command, + remote_command_str.data, NULL); + termPQExpBuffer(&remote_command_str); + /* verify that new standby is connected and replicating */ connection_success = false; diff --git a/repmgr.h b/repmgr.h index 152b4fff..bf21e568 100644 --- a/repmgr.h +++ b/repmgr.h @@ -23,6 +23,7 @@ #include #include #include +#include "pqexpbuffer.h" #include "strutil.h" #include "dbutils.h" diff --git a/strutil.c b/strutil.c index 3c565c89..4f116602 100644 --- a/strutil.c +++ b/strutil.c @@ -87,3 +87,34 @@ maxlen_snprintf(char *str, const char *format,...) return retval; } + + +/* + * Adapted from: src/fe_utils/string_utils.c + * + * Function not publicly available before PostgreSQL 9.6. + */ +void +appendShellString(PQExpBuffer buf, const char *str) +{ + const char *p; + + appendPQExpBufferChar(buf, '\''); + for (p = str; *p; p++) + { + if (*p == '\n' || *p == '\r') + { + fprintf(stderr, + _("shell command argument contains a newline or carriage return: \"%s\"\n"), + str); + exit(ERR_BAD_CONFIG); + } + + if (*p == '\'') + appendPQExpBufferStr(buf, "'\"'\"'"); + else + appendPQExpBufferChar(buf, *p); + } + + appendPQExpBufferChar(buf, '\''); +} diff --git a/strutil.h b/strutil.h index afc5abc2..076fdd20 100644 --- a/strutil.h +++ b/strutil.h @@ -22,6 +22,7 @@ #define _STRUTIL_H_ #include +#include "pqexpbuffer.h" #include "errcode.h" @@ -48,4 +49,6 @@ extern int maxlen_snprintf(char *str, const char *format,...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3))); +extern void +appendShellString(PQExpBuffer buf, const char *str); #endif /* _STRUTIL_H_ */