mirror of
https://github.com/postgres/postgres.git
synced 2025-07-30 11:03:19 +03:00
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
This commit is contained in:
@ -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("\"recovery_target_timeline\" is not a valid number.");
|
||||
GUC_check_errdetail("\"%s\" is not a valid number.",
|
||||
"recovery_target_timeline");
|
||||
return false;
|
||||
}
|
||||
|
||||
if (timeline < 1 || timeline > PG_UINT32_MAX)
|
||||
{
|
||||
GUC_check_errdetail("\"%s\" must be between %u and %u.",
|
||||
"recovery_target_timeline", 1, UINT_MAX);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
@ -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();
|
||||
|
Reference in New Issue
Block a user