]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Improve checks for GUC recovery_target_timeline
authorMichael Paquier <michael@paquier.xyz>
Thu, 3 Jul 2025 02:14:20 +0000 (11:14 +0900)
committerMichael Paquier <michael@paquier.xyz>
Thu, 3 Jul 2025 02:14:20 +0000 (11:14 +0900)
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
src/test/recovery/t/003_recovery_targets.pl

index 6ce979f2d8bc4b2ed260a9b482a993893a8d1e55..93d389148549ce5a6655b54098548474b3b8d4a4 100644 (file)
@@ -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;
                }
        }
index 0ae2e98272709254a30bddf0f0e0a5fca6533800..f2109efa9b12d44fbeca52e8619c3760bdff3452 100644 (file)
@@ -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();