aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorMichael Paquier <michael@paquier.xyz>2022-03-01 12:52:25 +0900
committerMichael Paquier <michael@paquier.xyz>2022-03-01 12:52:25 +0900
commita33e17f210547226ada52d2b8af851c3553bb4fa (patch)
treea01e715e2076aa06e8b3abd34252644739e1f4f4 /src
parent7a85073290856554416353a89799a4c04d09b74b (diff)
downloadpostgresql-a33e17f210547226ada52d2b8af851c3553bb4fa.tar.gz
postgresql-a33e17f210547226ada52d2b8af851c3553bb4fa.zip
Rework internal command generation of pg_rewind
pg_rewind generates and executes internally up to two commands to work on the target cluster, depending on the options given by its caller: - postgres -C to retrieve the value of restore_command, when using -c/--restore-target-wal. - postgres --single to enforce recovery once and get the target cluster in a clean shutdown state. Both commands have been applying incorrect quoting rules, which could lead to failures when for example using a target data directory with unexpected characters like CRLFs. Those commands are now generated with PQExpBuffer, making use of string_utils.h to quote those commands as they should. We may extend those commands in the future with more options, so this makes any upcoming additions easier. This is arguably a bug fix, but nobody has complained about the existing code being a problem either, so no backpatch is done. Extracted from a larger patch by the same author. Author: Gunnar "Nick" Bluth Discussion: https://postgr.es/m/7c59265d-ac50-b0aa-ca1e-65e8bd27642a@pro-open.de
Diffstat (limited to 'src')
-rw-r--r--src/bin/pg_rewind/pg_rewind.c43
1 files changed, 33 insertions, 10 deletions
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index efb82a40341..b39b5c1aacc 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -23,6 +23,7 @@
#include "common/restricted_token.h"
#include "common/string.h"
#include "fe_utils/recovery_gen.h"
+#include "fe_utils/string_utils.h"
#include "file_ops.h"
#include "filemap.h"
#include "getopt_long.h"
@@ -1016,8 +1017,8 @@ getRestoreCommand(const char *argv0)
{
int rc;
char postgres_exec_path[MAXPGPATH],
- postgres_cmd[MAXPGPATH],
cmd_output[MAXPGPATH];
+ PQExpBuffer postgres_cmd;
if (!restore_wal)
return;
@@ -1051,11 +1052,19 @@ getRestoreCommand(const char *argv0)
* Build a command able to retrieve the value of GUC parameter
* restore_command, if set.
*/
- snprintf(postgres_cmd, sizeof(postgres_cmd),
- "\"%s\" -D \"%s\" -C restore_command",
- postgres_exec_path, datadir_target);
+ postgres_cmd = createPQExpBuffer();
- if (!pipe_read_line(postgres_cmd, cmd_output, sizeof(cmd_output)))
+ /* path to postgres, properly quoted */
+ appendShellString(postgres_cmd, postgres_exec_path);
+
+ /* add -D switch, with properly quoted data directory */
+ appendPQExpBufferStr(postgres_cmd, " -D ");
+ appendShellString(postgres_cmd, datadir_target);
+
+ /* add -C switch, for restore_command */
+ appendPQExpBufferStr(postgres_cmd, " -C restore_command");
+
+ if (!pipe_read_line(postgres_cmd->data, cmd_output, sizeof(cmd_output)))
exit(1);
(void) pg_strip_crlf(cmd_output);
@@ -1067,6 +1076,8 @@ getRestoreCommand(const char *argv0)
pg_log_debug("using for rewind restore_command = \'%s\'",
restore_command);
+
+ destroyPQExpBuffer(postgres_cmd);
}
@@ -1080,7 +1091,7 @@ ensureCleanShutdown(const char *argv0)
int ret;
#define MAXCMDLEN (2 * MAXPGPATH)
char exec_path[MAXPGPATH];
- char cmd[MAXCMDLEN];
+ PQExpBuffer postgres_cmd;
/* locate postgres binary */
if ((ret = find_other_exec(argv0, "postgres",
@@ -1119,14 +1130,26 @@ ensureCleanShutdown(const char *argv0)
* fsync here. This makes the recovery faster, and the target data folder
* is synced at the end anyway.
*/
- snprintf(cmd, MAXCMDLEN, "\"%s\" --single -F -D \"%s\" template1 < \"%s\"",
- exec_path, datadir_target, DEVNULL);
+ postgres_cmd = createPQExpBuffer();
- if (system(cmd) != 0)
+ /* path to postgres, properly quoted */
+ appendShellString(postgres_cmd, exec_path);
+
+ /* add set of options with properly quoted data directory */
+ appendPQExpBufferStr(postgres_cmd, " --single -F -D ");
+ appendShellString(postgres_cmd, datadir_target);
+
+ /* finish with the database name, and a properly quoted redirection */
+ appendPQExpBufferStr(postgres_cmd, " template1 < ");
+ appendShellString(postgres_cmd, DEVNULL);
+
+ if (system(postgres_cmd->data) != 0)
{
pg_log_error("postgres single-user mode in target cluster failed");
- pg_fatal("Command was: %s", cmd);
+ pg_fatal("Command was: %s", postgres_cmd->data);
}
+
+ destroyPQExpBuffer(postgres_cmd);
}
static void