From 3975b1296174218c5e25e2b02bca88cb3c26ee63 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 9 Jan 2023 09:03:30 +0100 Subject: [PATCH v4] Common function for percent placeholder replacement There are a number of places where a shell command is constructed with percent-placeholders (like %x). It's cumbersome to have to open-code this several times. This factors out this logic into a separate function. This also allows us to ensure consistency for and document some subtle behaviors, such as what to do with unrecognized placeholders. Discussion: https://www.postgresql.org/message-id/flat/5238bbed-0b01-83a6-d4b2-7eb0562a054e%40enterprisedb.com --- .../basebackup_to_shell/basebackup_to_shell.c | 56 +------ src/backend/access/transam/xlogarchive.c | 45 +----- src/backend/libpq/be-secure-common.c | 38 +---- src/backend/postmaster/shell_archive.c | 58 ++------ src/common/Makefile | 1 + src/common/archive.c | 81 +---------- src/common/meson.build | 1 + src/common/percentrepl.c | 137 ++++++++++++++++++ src/fe_utils/archive.c | 2 - src/include/common/percentrepl.h | 18 +++ 10 files changed, 190 insertions(+), 247 deletions(-) create mode 100644 src/common/percentrepl.c create mode 100644 src/include/common/percentrepl.h diff --git a/contrib/basebackup_to_shell/basebackup_to_shell.c b/contrib/basebackup_to_shell/basebackup_to_shell.c index 8d583550b5..29f5069d42 100644 --- a/contrib/basebackup_to_shell/basebackup_to_shell.c +++ b/contrib/basebackup_to_shell/basebackup_to_shell.c @@ -12,6 +12,7 @@ #include "access/xact.h" #include "backup/basebackup_target.h" +#include "common/percentrepl.h" #include "miscadmin.h" #include "storage/fd.h" #include "utils/acl.h" @@ -208,59 +209,8 @@ static char * shell_construct_command(const char *base_command, const char *filename, const char *target_detail) { - StringInfoData buf; - const char *c; - - initStringInfo(&buf); - for (c = base_command; *c != '\0'; ++c) - { - /* Anything other than '%' is copied verbatim. */ - if (*c != '%') - { - appendStringInfoChar(&buf, *c); - continue; - } - - /* Any time we see '%' we eat the following character as well. */ - ++c; - - /* - * The following character determines what we insert here, or may - * cause us to throw an error. - */ - if (*c == '%') - { - /* '%%' is replaced by a single '%' */ - appendStringInfoChar(&buf, '%'); - } - else if (*c == 'f') - { - /* '%f' is replaced by the filename */ - appendStringInfoString(&buf, filename); - } - else if (*c == 'd') - { - /* '%d' is replaced by the target detail */ - appendStringInfoString(&buf, target_detail); - } - else if (*c == '\0') - { - /* Incomplete escape sequence, expected a character afterward */ - ereport(ERROR, - errcode(ERRCODE_SYNTAX_ERROR), - errmsg("shell command ends unexpectedly after escape character \"%%\"")); - } - else - { - /* Unknown escape sequence */ - ereport(ERROR, - errcode(ERRCODE_SYNTAX_ERROR), - errmsg("shell command contains unexpected escape sequence \"%c\"", - *c)); - } - } - - return buf.data; + return replace_percent_placeholders(base_command, "basebackup_to_shell.command", + "df", target_detail, filename); } /* diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 76abc74c67..f911e8c3a6 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -23,6 +23,7 @@ #include "access/xlog_internal.h" #include "access/xlogarchive.h" #include "common/archive.h" +#include "common/percentrepl.h" #include "miscadmin.h" #include "pgstat.h" #include "postmaster/startup.h" @@ -291,11 +292,8 @@ void ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOnSignal, uint32 wait_event_info) { - char xlogRecoveryCmd[MAXPGPATH]; + char *xlogRecoveryCmd; char lastRestartPointFname[MAXPGPATH]; - char *dp; - char *endp; - const char *sp; int rc; XLogSegNo restartSegNo; XLogRecPtr restartRedoPtr; @@ -316,42 +314,7 @@ ExecuteRecoveryCommand(const char *command, const char *commandName, /* * construct the command to be executed */ - dp = xlogRecoveryCmd; - endp = xlogRecoveryCmd + MAXPGPATH - 1; - *endp = '\0'; - - for (sp = command; *sp; sp++) - { - if (*sp == '%') - { - switch (sp[1]) - { - case 'r': - /* %r: filename of last restartpoint */ - sp++; - strlcpy(dp, lastRestartPointFname, endp - dp); - dp += strlen(dp); - break; - case '%': - /* convert %% to a single % */ - sp++; - if (dp < endp) - *dp++ = *sp; - break; - default: - /* otherwise treat the % as not special */ - if (dp < endp) - *dp++ = *sp; - break; - } - } - else - { - if (dp < endp) - *dp++ = *sp; - } - } - *dp = '\0'; + xlogRecoveryCmd = replace_percent_placeholders(command, commandName, "r", lastRestartPointFname); ereport(DEBUG3, (errmsg_internal("executing %s \"%s\"", commandName, command))); @@ -364,6 +327,8 @@ ExecuteRecoveryCommand(const char *command, const char *commandName, rc = system(xlogRecoveryCmd); pgstat_report_wait_end(); + pfree(xlogRecoveryCmd); + if (rc != 0) { /* diff --git a/src/backend/libpq/be-secure-common.c b/src/backend/libpq/be-secure-common.c index f6078c91c3..ab5e2dfa2b 100644 --- a/src/backend/libpq/be-secure-common.c +++ b/src/backend/libpq/be-secure-common.c @@ -22,6 +22,7 @@ #include #include +#include "common/percentrepl.h" #include "common/string.h" #include "libpq/libpq.h" #include "storage/fd.h" @@ -39,8 +40,7 @@ int run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf, int size) { int loglevel = is_server_start ? ERROR : LOG; - StringInfoData command; - char *p; + char *command; FILE *fh; int pclose_rc; size_t len = 0; @@ -49,37 +49,15 @@ run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf, Assert(size > 0); buf[0] = '\0'; - initStringInfo(&command); + command = replace_percent_placeholders(ssl_passphrase_command, "ssl_passphrase_command", "p", prompt); - for (p = ssl_passphrase_command; *p; p++) - { - if (p[0] == '%') - { - switch (p[1]) - { - case 'p': - appendStringInfoString(&command, prompt); - p++; - break; - case '%': - appendStringInfoChar(&command, '%'); - p++; - break; - default: - appendStringInfoChar(&command, p[0]); - } - } - else - appendStringInfoChar(&command, p[0]); - } - - fh = OpenPipeStream(command.data, "r"); + fh = OpenPipeStream(command, "r"); if (fh == NULL) { ereport(loglevel, (errcode_for_file_access(), errmsg("could not execute command \"%s\": %m", - command.data))); + command))); goto error; } @@ -91,7 +69,7 @@ run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf, ereport(loglevel, (errcode_for_file_access(), errmsg("could not read from command \"%s\": %m", - command.data))); + command))); goto error; } } @@ -111,7 +89,7 @@ run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf, ereport(loglevel, (errcode_for_file_access(), errmsg("command \"%s\" failed", - command.data), + command), errdetail_internal("%s", wait_result_to_str(pclose_rc)))); goto error; } @@ -120,7 +98,7 @@ run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf, len = pg_strip_crlf(buf); error: - pfree(command.data); + pfree(command); return len; } diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c index 6f98414a40..5d97c6df64 100644 --- a/src/backend/postmaster/shell_archive.c +++ b/src/backend/postmaster/shell_archive.c @@ -18,6 +18,7 @@ #include #include "access/xlog.h" +#include "common/percentrepl.h" #include "pgstat.h" #include "postmaster/pgarch.h" @@ -44,58 +45,17 @@ shell_archive_configured(void) static bool shell_archive_file(const char *file, const char *path) { - char xlogarchcmd[MAXPGPATH]; - char *dp; - char *endp; - const char *sp; + char *xlogarchcmd; + char *nativePath = NULL; int rc; - /* - * construct the command to be executed - */ - dp = xlogarchcmd; - endp = xlogarchcmd + MAXPGPATH - 1; - *endp = '\0'; - - for (sp = XLogArchiveCommand; *sp; sp++) + if (path) { - if (*sp == '%') - { - switch (sp[1]) - { - case 'p': - /* %p: relative path of source file */ - sp++; - strlcpy(dp, path, endp - dp); - make_native_path(dp); - dp += strlen(dp); - break; - case 'f': - /* %f: filename of source file */ - sp++; - strlcpy(dp, file, endp - dp); - dp += strlen(dp); - break; - case '%': - /* convert %% to a single % */ - sp++; - if (dp < endp) - *dp++ = *sp; - break; - default: - /* otherwise treat the % as not special */ - if (dp < endp) - *dp++ = *sp; - break; - } - } - else - { - if (dp < endp) - *dp++ = *sp; - } + nativePath = pstrdup(path); + make_native_path(nativePath); } - *dp = '\0'; + + xlogarchcmd = replace_percent_placeholders(XLogArchiveCommand, "archive_command", "fp", file, nativePath); ereport(DEBUG3, (errmsg_internal("executing archive command \"%s\"", @@ -155,6 +115,8 @@ shell_archive_file(const char *file, const char *path) return false; } + pfree(xlogarchcmd); + elog(DEBUG1, "archived write-ahead log file \"%s\"", file); return true; } diff --git a/src/common/Makefile b/src/common/Makefile index 898701fae1..113029bf7b 100644 --- a/src/common/Makefile +++ b/src/common/Makefile @@ -65,6 +65,7 @@ OBJS_COMMON = \ kwlookup.o \ link-canary.o \ md5_common.o \ + percentrepl.o \ pg_get_line.o \ pg_lzcompress.o \ pg_prng.o \ diff --git a/src/common/archive.c b/src/common/archive.c index 1466f67ea6..fb95006671 100644 --- a/src/common/archive.c +++ b/src/common/archive.c @@ -20,7 +20,7 @@ #endif #include "common/archive.h" -#include "lib/stringinfo.h" +#include "common/percentrepl.h" /* * BuildRestoreCommand @@ -41,81 +41,14 @@ BuildRestoreCommand(const char *restoreCommand, const char *xlogfname, const char *lastRestartPointFname) { - StringInfoData result; - const char *sp; + char *nativePath = NULL; - /* - * Build the command to be executed. - */ - initStringInfo(&result); - - for (sp = restoreCommand; *sp; sp++) + if (xlogpath) { - if (*sp == '%') - { - switch (sp[1]) - { - case 'p': - { - char *nativePath; - - /* %p: relative path of target file */ - if (xlogpath == NULL) - { - pfree(result.data); - return NULL; - } - sp++; - - /* - * This needs to use a placeholder to not modify the - * input with the conversion done via - * make_native_path(). - */ - nativePath = pstrdup(xlogpath); - make_native_path(nativePath); - appendStringInfoString(&result, - nativePath); - pfree(nativePath); - break; - } - case 'f': - /* %f: filename of desired file */ - if (xlogfname == NULL) - { - pfree(result.data); - return NULL; - } - sp++; - appendStringInfoString(&result, xlogfname); - break; - case 'r': - /* %r: filename of last restartpoint */ - if (lastRestartPointFname == NULL) - { - pfree(result.data); - return NULL; - } - sp++; - appendStringInfoString(&result, - lastRestartPointFname); - break; - case '%': - /* convert %% to a single % */ - sp++; - appendStringInfoChar(&result, *sp); - break; - default: - /* otherwise treat the % as not special */ - appendStringInfoChar(&result, *sp); - break; - } - } - else - { - appendStringInfoChar(&result, *sp); - } + nativePath = pstrdup(xlogpath); + make_native_path(nativePath); } - return result.data; + return replace_percent_placeholders(restoreCommand, "restore_command", "frp", + xlogfname, lastRestartPointFname, nativePath); } diff --git a/src/common/meson.build b/src/common/meson.build index a1fc398d8e..41bd58ebdf 100644 --- a/src/common/meson.build +++ b/src/common/meson.build @@ -17,6 +17,7 @@ common_sources = files( 'kwlookup.c', 'link-canary.c', 'md5_common.c', + 'percentrepl.c', 'pg_get_line.c', 'pg_lzcompress.c', 'pg_prng.c', diff --git a/src/common/percentrepl.c b/src/common/percentrepl.c new file mode 100644 index 0000000000..25cfd0b2d4 --- /dev/null +++ b/src/common/percentrepl.c @@ -0,0 +1,137 @@ +/*------------------------------------------------------------------------- + * + * percentrepl.c + * Common routines to replace percent placeholders in strings + * + * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * + * IDENTIFICATION + * src/common/percentrepl.c + * + *------------------------------------------------------------------------- + */ + +#ifndef FRONTEND +#include "postgres.h" +#else +#include "postgres_fe.h" +#include "common/logging.h" +#endif + +#include "common/percentrepl.h" +#include "lib/stringinfo.h" + +/* + * replace_percent_placeholders + * + * Replace percent-letter placeholders in input string with the supplied + * values. For example, to replace %f with foo and %b with bar, call + * + * replace_percent_placeholders(instr, "bf", bar, foo); + * + * The return value is palloc'd. + * + * "%%" is replaced by a single "%". + * + * This throws an error for an unsupported placeholder or a "%" at the end of + * the input string. + * + * A value may be NULL. If the corresponding placeholder is found in the + * input string, it will be treated as if an unsupported placeholder was used. + * This allows callers to share a "letters" specification but vary the + * actually supported placeholders at run time. + * + * This functions is meant for cases where all the values are readily + * available or cheap to compute and most invocations will use most values + * (for example for archive_command). Also, it requires that all values are + * strings. It won't be a good match for things like log prefixes or prompts + * that use a mix of data types and any invocation will only use a few of the + * possible values. + * + * param_name is the name of the underlying GUC parameter, for error + * reporting. At the moment, this function is only used for GUC parameters. + * If other kinds of uses were added, the error reporting would need to be + * revised. + */ +char * +replace_percent_placeholders(const char *instr, const char *param_name, const char *letters, ...) +{ + StringInfoData result; + + initStringInfo(&result); + + for (const char *sp = instr; *sp; sp++) + { + if (*sp == '%') + { + if (sp[1] == '%') + { + /* Convert %% to a single % */ + sp++; + appendStringInfoChar(&result, *sp); + } + else if (sp[1] == '\0') + { + /* Incomplete escape sequence, expected a character afterward */ +#ifdef FRONTEND + pg_log_error("invalid value for parameter \"%s\": \"%s\"", param_name, instr); + pg_log_error_detail("String ends unexpectedly after escape character \"%%\"."); + exit(1); +#else + ereport(ERROR, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid value for parameter \"%s\": \"%s\"", param_name, instr), + errdetail("String ends unexpectedly after escape character \"%%\".")); +#endif + } + else + { + /* Look up placeholder character */ + bool found = false; + va_list ap; + + sp++; + + va_start(ap, letters); + for (const char *lp = letters; *lp; lp++) + { + char *val = va_arg(ap, char *); + + if (*sp == *lp) + { + if (val) + { + appendStringInfoString(&result, val); + found = true; + } + /* If val is NULL, we will report an error. */ + break; + } + } + va_end(ap); + if (!found) + { + /* Unknown escape sequence */ +#ifdef FRONTEND + pg_log_error("invalid value for parameter \"%s\": \"%s\"", param_name, instr); + pg_log_error_detail("String contains unexpected escape sequence \"%c\".", *sp); + exit(1); +#else + ereport(ERROR, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid value for parameter \"%s\": \"%s\"", param_name, instr), + errdetail("String contains unexpected escape sequence \"%c\".", *sp)); +#endif + } + } + } + else + { + appendStringInfoChar(&result, *sp); + } + } + + return result.data; +} diff --git a/src/fe_utils/archive.c b/src/fe_utils/archive.c index aab813c102..eb1c930ae7 100644 --- a/src/fe_utils/archive.c +++ b/src/fe_utils/archive.c @@ -48,8 +48,6 @@ RestoreArchivedFile(const char *path, const char *xlogfname, xlogRestoreCmd = BuildRestoreCommand(restoreCommand, xlogpath, xlogfname, NULL); - if (xlogRestoreCmd == NULL) - pg_fatal("cannot use restore_command with %%r placeholder"); /* * Execute restore_command, which should copy the missing file from diff --git a/src/include/common/percentrepl.h b/src/include/common/percentrepl.h new file mode 100644 index 0000000000..2dd88db144 --- /dev/null +++ b/src/include/common/percentrepl.h @@ -0,0 +1,18 @@ +/*------------------------------------------------------------------------- + * + * percentrepl.h + * Common routines to replace percent placeholders in strings + * + * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/common/percentrepl.h + * + *------------------------------------------------------------------------- + */ +#ifndef PERCENTREPL_H +#define PERCENTREPL_H + +extern char *replace_percent_placeholders(const char *instr, const char *param_name, const char *letters, ...); + +#endif /* PERCENTREPL_H */ base-commit: 3c569049b7b502bb4952483d19ce622ff0af5fd6 -- 2.39.0