Skip to content

Commit a7da567

Browse files
dwsteeleCommitfest Bot
authored andcommitted
Improve verification of recovery_target_timeline GUC.
Currently check_recovery_target_timeline() converts any value that is not current, latest, or a valid integer to 0. So for example: recovery_target_timeline = 'currrent' results in the following error: FATAL: 22023: recovery target timeline 0 does not exist Since there is no range checking for uint32 (but there is a cast from unsigned long) this setting: recovery_target_timeline = '9999999999' results in the following error: FATAL: 22023: recovery target timeline 1410065407 does not exist Improve by adding endptr checking to catch conversion errors and add range checking to exclude values < 2 and greater than UINT_MAX.
1 parent de5aa15 commit a7da567

File tree

2 files changed

+64
-3
lines changed

2 files changed

+64
-3
lines changed

src/backend/access/transam/xlogrecovery.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4994,13 +4994,24 @@ check_recovery_target_timeline(char **newval, void **extra, GucSource source)
49944994
rttg = RECOVERY_TARGET_TIMELINE_LATEST;
49954995
else
49964996
{
4997+
char *endp;
4998+
uint64 timeline;
49974999
rttg = RECOVERY_TARGET_TIMELINE_NUMERIC;
49985000

49995001
errno = 0;
5000-
strtoul(*newval, NULL, 0);
5001-
if (errno == EINVAL || errno == ERANGE)
5002+
timeline = strtou64(*newval, &endp, 0);
5003+
5004+
if (*endp != '\0' || errno == EINVAL || errno == ERANGE)
5005+
{
5006+
GUC_check_errdetail("\"%s\" is not a valid number.",
5007+
"recovery_target_timeline");
5008+
return false;
5009+
}
5010+
5011+
if (timeline < 1 || timeline > UINT_MAX)
50025012
{
5003-
GUC_check_errdetail("\"recovery_target_timeline\" is not a valid number.");
5013+
GUC_check_errdetail("\"%s\" must be between %u and %u.",
5014+
"recovery_target_timeline", 1, UINT_MAX);
50045015
return false;
50055016
}
50065017
}

src/test/recovery/t/003_recovery_targets.pl

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,4 +187,54 @@ sub test_recovery_standby
187187
qr/FATAL: .* recovery ended before configured recovery target was reached/,
188188
'recovery end before target reached is a fatal error');
189189

190+
# Invalid timeline target
191+
$node_standby = PostgreSQL::Test::Cluster->new('standby_9');
192+
$node_standby->init_from_backup($node_primary, 'my_backup',
193+
has_restoring => 1);
194+
$node_standby->append_conf(
195+
'postgresql.conf', "recovery_target_timeline = 'bogus'");
196+
197+
$res = run_log(
198+
[
199+
'pg_ctl',
200+
'--pgdata' => $node_standby->data_dir,
201+
'--log' => $node_standby->logfile,
202+
'start',
203+
]);
204+
ok(!$res, 'invalid timeline target (bogus value)');
205+
206+
my $log_start = $node_standby->wait_for_log("is not a valid number");
207+
208+
# Timeline target out of min range
209+
$node_standby->append_conf(
210+
'postgresql.conf', "recovery_target_timeline = '0'");
211+
212+
$res = run_log(
213+
[
214+
'pg_ctl',
215+
'--pgdata' => $node_standby->data_dir,
216+
'--log' => $node_standby->logfile,
217+
'start',
218+
]);
219+
ok(!$res, 'invalid timeline target (lower bound check)');
220+
221+
$log_start = $node_standby->wait_for_log("must be between 1 and 4294967295",
222+
$log_start);
223+
224+
# Timeline target out of max range
225+
$node_standby->append_conf(
226+
'postgresql.conf', "recovery_target_timeline = '4294967296'");
227+
228+
$res = run_log(
229+
[
230+
'pg_ctl',
231+
'--pgdata' => $node_standby->data_dir,
232+
'--log' => $node_standby->logfile,
233+
'start',
234+
]);
235+
ok(!$res, 'invalid timeline target (upper bound check)');
236+
237+
$log_start = $node_standby->wait_for_log("must be between 1 and 4294967295",
238+
$log_start);
239+
190240
done_testing();

0 commit comments

Comments
 (0)