diff --git a/HISTORY b/HISTORY index 9db23055..7b905112 100644 --- a/HISTORY +++ b/HISTORY @@ -2,6 +2,8 @@ repmgr: add --version-number command line option (Ian) repmgr: add --terse option to "cluster show"; GitHub #521 (Ian) repmgr: add --dry-run option to "standby promote"; GitHub #522 (Ian) + repmgr: "standby switchover": improve handling of connection URIs when + executing "node rejoin" on the demotion candidate; GitHub #525 (Ian) repmgrd: check binary and extension major versions match; GitHub #515 (Ian) 4.2.1 2018-??-?? diff --git a/compat.c b/compat.c index d3f6ea8a..6d89f0a4 100644 --- a/compat.c +++ b/compat.c @@ -98,9 +98,42 @@ appendShellString(PQExpBuffer buf, const char *str) if (*p == '\'') appendPQExpBufferStr(buf, "'\"'\"'"); + else if (*p == '&') + appendPQExpBufferStr(buf, "\\&"); else appendPQExpBufferChar(buf, *p); } appendPQExpBufferChar(buf, '\''); } + +/* + * Adapted from: src/fe_utils/string_utils.c + */ +void +appendRemoteShellString(PQExpBuffer buf, const char *str) +{ + const char *p; + + appendPQExpBufferStr(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 if (*p == '&') + appendPQExpBufferStr(buf, "\\&"); + else + appendPQExpBufferChar(buf, *p); + } + + appendPQExpBufferStr(buf, "\\'"); +} diff --git a/compat.h b/compat.h index 557f3815..bd2d002c 100644 --- a/compat.h +++ b/compat.h @@ -27,4 +27,6 @@ extern void appendConnStrVal(PQExpBuffer buf, const char *str); extern void appendShellString(PQExpBuffer buf, const char *str); +extern void appendRemoteShellString(PQExpBuffer buf, const char *str); + #endif diff --git a/dbutils.c b/dbutils.c index 712988e7..b73be799 100644 --- a/dbutils.c +++ b/dbutils.c @@ -162,7 +162,8 @@ _establish_db_connection(const char *conninfo, const bool exit_on_error, const b if (parse_success == false) { - log_error(_("unable to pass provided conninfo string:\n %s"), errmsg); + log_error(_("unable to parse provided conninfo string \"%s\""), conninfo); + log_detail("%s", errmsg); free_conninfo_params(&conninfo_params); return NULL; } @@ -653,7 +654,6 @@ parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_lis if (strcmp(option->keyword, "servicefile") == 0) continue; } - param_set(param_list, option->keyword, option->val); } @@ -740,6 +740,41 @@ param_list_to_string(t_conninfo_param_list *param_list) } +/* + * Run a conninfo string through the parser, and pass it back as a normal + * conninfo string. This is mainly intended for converting connection URIs + * to parameter/value conninfo strings. + * + * Caller must free returned pointer. + */ + +char * +normalize_conninfo_string(const char *conninfo_str) +{ + t_conninfo_param_list conninfo_params = T_CONNINFO_PARAM_LIST_INITIALIZER; + bool parse_success = false; + char *normalized_string = NULL; + char *errmsg = NULL; + + initialize_conninfo_params(&conninfo_params, false); + + parse_success = parse_conninfo_string(conninfo_str, &conninfo_params, &errmsg, false); + + if (parse_success == false) + { + log_error(_("unable to parse provided conninfo string \"%s\""), conninfo_str); + log_detail("%s", errmsg); + free_conninfo_params(&conninfo_params); + return NULL; + } + + + normalized_string = param_list_to_string(&conninfo_params); + free_conninfo_params(&conninfo_params); + + return normalized_string; +} + /* * check whether the libpq version in use recognizes the "passfile" parameter * (should be 9.6 and later) diff --git a/dbutils.h b/dbutils.h index d75e3bd4..27b11636 100644 --- a/dbutils.h +++ b/dbutils.h @@ -402,6 +402,7 @@ void param_set_ine(t_conninfo_param_list *param_list, const char *param, const char *param_get(t_conninfo_param_list *param_list, const char *param); bool parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_list, char **errmsg, bool ignore_local_params); char *param_list_to_string(t_conninfo_param_list *param_list); +char *normalize_conninfo_string(const char *conninfo_str); bool has_passfile(void); diff --git a/doc/appendix-release-notes.sgml b/doc/appendix-release-notes.sgml index 711269be..680764e0 100644 --- a/doc/appendix-release-notes.sgml +++ b/doc/appendix-release-notes.sgml @@ -54,6 +54,25 @@ + + Bug fixes + + + + + + &repmgr;: when executing repmgr standby switchover, + prevent escaping issues with connection URIs when executing repmgr node rejoin + on the demotion candidate (GitHub #525). + + + + + + + + + diff --git a/repmgr-action-standby.c b/repmgr-action-standby.c index f2d218a4..798a43ba 100644 --- a/repmgr-action-standby.c +++ b/repmgr-action-standby.c @@ -3704,8 +3704,7 @@ do_standby_switchover(void) initPQExpBuffer(&command_output); - command_success = remote_command( - remote_host, + command_success = remote_command(remote_host, runtime_options.remote_user, remote_command_str.data, &command_output); @@ -4360,10 +4359,27 @@ do_standby_switchover(void) initPQExpBuffer(&remote_command_str); make_remote_repmgr_path(&remote_command_str, &remote_node_record); - appendPQExpBuffer(&remote_command_str, - "%s-d \\'%s\\' node rejoin", - node_rejoin_options.data, - local_node_record.conninfo); + /* + * Here we'll coerce the local node's connection string into + * "param=value" format, in case it's configured in URI format, + * to simplify escaping issues when passing the string to the + * remote node. + */ + { + char *conninfo_normalized = normalize_conninfo_string(local_node_record.conninfo); + + appendPQExpBuffer(&remote_command_str, + "%s-d ", + node_rejoin_options.data); + + appendRemoteShellString(&remote_command_str, + conninfo_normalized); + + appendPQExpBufferStr(&remote_command_str, + " node rejoin"); + + pfree(conninfo_normalized); + } termPQExpBuffer(&node_rejoin_options);