postgres.git / summary / log / commit / refs

commit    fd7d7b719137b5c427681a50c0a0ac2d745b68bd
Author:   Michael Paquier <michael@paquier.xyz>
Date:     Thu Jul 03 02:14:20 2025 +0000

    Improve checks for GUC recovery_target_timeline
    
    Currently check_recovery_target_timeline() converts any value that is
    not "current", "latest", or a valid integer to 0.  So, for example, the
    following configuration added to postgresql.conf followed by a startup:
    recovery_target_timeline = 'bogus'
    recovery_target_timeline = '9999999999'
    
    ...  results in the following error patterns:
    FATAL:  22023: recovery target timeline 0 does not exist
    FATAL:  22023: recovery target timeline 1410065407 does not exist
    
    This is confusing, because the server does not reflect the intention of
    the user, and just reports incorrect data unrelated to the GUC.
    
    The origin of the problem is that we do not perform a range check in the
    GUC value passed-in for recovery_target_timeline.  This commit improves
    the situation by using strtou64() and by providing stricter range
    checks.  Some test cases are added for the cases of an incorrect, an
    upper-bound and a lower-bound timeline value, checking the sanity of the
    reports based on the contents of the server logs.
    
    Author: David Steele <david@pgmasters.net>
    Discussion: https://postgr.es/m/e5d472c7-e9be-4710-8dc4-ebe721b62cea@pgbackrest.org


 src/backend/access/transam/xlogrecovery.c   | 18 +++++++++--
 src/test/recovery/t/003_recovery_targets.pl | 50 +++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 6ce979f2d8b..93d38914854 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -4994,13 +4994,25 @@ check_recovery_target_timeline(char **newval, void **extra, GucSource source)
 		rttg = RECOVERY_TARGET_TIMELINE_LATEST;
 	else
 	{
+		char	   *endp;
+		uint64		timeline;
+
 		rttg = RECOVERY_TARGET_TIMELINE_NUMERIC;
 
 		errno = 0;
-		strtoul(*newval, NULL, 0);
-		if (errno == EINVAL || errno == ERANGE)
+		timeline = strtou64(*newval, &endp, 0);
+
+		if (*endp != '\0' || errno == EINVAL || errno == ERANGE)
+		{
+			GUC_check_errdetail("\"%s\" is not a valid number.",
+								"recovery_target_timeline");
+			return false;
+		}
+
+		if (timeline < 1 || timeline > PG_UINT32_MAX)
 		{
-			GUC_check_errdetail("\"recovery_target_timeline\" is not a valid number.");
+			GUC_check_errdetail("\"%s\" must be between %u and %u.",
+								"recovery_target_timeline", 1, UINT_MAX);
 			return false;
 		}
 	}
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 0ae2e982727..f2109efa9b1 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -187,4 +187,54 @@ ok( $logfile =~
 	  qr/FATAL: .* recovery ended before configured recovery target was reached/,
 	'recovery end before target reached is a fatal error');
 
+# Invalid timeline target
+$node_standby = PostgreSQL::Test::Cluster->new('standby_9');
+$node_standby->init_from_backup($node_primary, 'my_backup',
+	has_restoring => 1);
+$node_standby->append_conf('postgresql.conf',
+	"recovery_target_timeline = 'bogus'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid timeline target (bogus value)');
+
+my $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_timeline = '0'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid timeline target (lower bound check)');
+
+$log_start =
+  $node_standby->wait_for_log("must be between 1 and 4294967295", $log_start);
+
+# Timeline target out of max range
+$node_standby->append_conf('postgresql.conf',
+	"recovery_target_timeline = '4294967296'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid timeline target (upper bound check)');
+
+$log_start =
+  $node_standby->wait_for_log("must be between 1 and 4294967295", $log_start);
+
 done_testing();


[parent: 0da29e4cb161]