From 0f3c34a5f9503535b046ea5821ae07a9e48c185c Mon Sep 17 00:00:00 2001 From: David Steele Date: Fri, 20 Feb 2026 05:40:28 +0000 Subject: Improve checks for GUC recovery_target_xid Currently check_recovery_target_xid() converts invalid values to 0. So, for example, the following configuration added to postgresql.conf followed by a startup: recovery_target_xid = 'bogus' recovery_target_xid = '1.1' recovery_target_xid = '0' ... does not generate an error but recovery does not complete. There are many values that can prevent recovery from completing but we should at least catch obvious misconfiguration by the user. The origin of the problem is that we do not perform a range check in the GUC value passed-in for recovery_target_xid. This commit improves the situation by using adding end checking to strtou64() and by providing stricter range checks. Some test cases are added for the cases of an incorrect or a lower-bound timeline value, checking the sanity of the reports based on the contents of the server logs. Also add a comment that truncation of the input value is expected since users will generally be using the output from pg_current_xact_id() (or similar) to set recovery_target_xid (just as our tests do). --- src/backend/access/transam/xlogrecovery.c | 22 +++++++++++-- src/test/recovery/t/003_recovery_targets.pl | 34 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index c0c2744d45b..554a8fcfe59 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -5107,11 +5107,29 @@ check_recovery_target_xid(char **newval, void **extra, GucSource source) { TransactionId xid; TransactionId *myextra; + char *endp; errno = 0; - xid = (TransactionId) strtou64(*newval, NULL, 0); - if (errno == EINVAL || errno == ERANGE) + + /* + * This cast will remove the epoch, if any + */ + xid = (TransactionId) strtou64(*newval, &endp, 0); + + if (*endp != '\0' || errno == EINVAL || errno == ERANGE) + { + GUC_check_errdetail("\"%s\" is not a valid number.", + "recovery_target_xid"); + return false; + } + + if (xid < FirstNormalTransactionId) + { + GUC_check_errdetail("\"%s\" without epoch must greater than or equal to %u.", + "recovery_target_xid", + FirstNormalTransactionId); return false; + } myextra = (TransactionId *) guc_malloc(LOG, sizeof(TransactionId)); if (!myextra) diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl index e0df1a23423..1ff3f3c8676 100644 --- a/src/test/recovery/t/003_recovery_targets.pl +++ b/src/test/recovery/t/003_recovery_targets.pl @@ -240,4 +240,38 @@ ok(!$res, 'invalid timeline target (upper bound check)'); $log_start = $node_standby->wait_for_log("must be between 1 and 4294967295", $log_start); +# Invalid xid target +$node_standby = PostgreSQL::Test::Cluster->new('standby_10'); +$node_standby->init_from_backup($node_primary, 'my_backup', + has_restoring => 1); +$node_standby->append_conf('postgresql.conf', + "recovery_target_xid = 'bogus'"); + +$res = run_log( + [ + 'pg_ctl', + '--pgdata' => $node_standby->data_dir, + '--log' => $node_standby->logfile, + 'start', + ]); +ok(!$res, 'invalid xid target (bogus value)'); + +$log_start = $node_standby->wait_for_log("is not a valid number"); + +# Timeline target out of min range +$node_standby->append_conf('postgresql.conf', + "recovery_target_xid = '0'"); + +$res = run_log( + [ + 'pg_ctl', + '--pgdata' => $node_standby->data_dir, + '--log' => $node_standby->logfile, + 'start', + ]); +ok(!$res, 'invalid xid target (lower bound check)'); + +$log_start = $node_standby->wait_for_log( + "without epoch must greater than or equal to 3", $log_start); + done_testing(); -- 2.34.1