Content-Type: multipart/mixed; boundary="------------UA8BIV200otwZ206Bn5qfIpx"
Message-ID: <e5d472c7-e9be-4710-8dc4-ebe721b62cea@pgbackrest.org>
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: Pg Hackers <pgsql-hackers@postgresql.org>
From: David Steele <david@pgbackrest.org>
Subject: Improve verification of recovery_target_timeline GUC.
Date: Thu, 23 Jan 2025 14:53:39 +0000 (UTC)
Archived-At: 
 <https://www.postgresql.org/message-id/e5d472c7-e9be-4710-8dc4-ebe721b62cea%40pgbackrest.org>
Precedence: bulk

This is a multi-part message in MIME format.
--------------UA8BIV200otwZ206Bn5qfIpx
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hackers,

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 (on 64-bit):

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.

I discovered this while testing on Postgres versions < 12 where

recovery_target_timeline = 'current'

sets the target timeline to 'latest', which is a pretty surprising 
result. I'm only noting it in case somebody is searching the archives 
since the versions in question are EOL. But that odd result made me 
wonder if Postgres >= 12 had a similar problem. While it is not quite so 
confusing as Postgres < 12 I think it still bears improving.

The tests are probably excessive but I needed something to show that the 
verification works as expected.

Regards,
-David

--------------UA8BIV200otwZ206Bn5qfIpx
Content-Type: text/plain; charset=UTF-8; name="timeline-log-v1.patch"
Content-Disposition: attachment; filename="timeline-log-v1.patch"
Content-Transfer-Encoding: base64

RnJvbSBmMTRlMzBiMThjZGUyMTYxMzFiZDNlMDY5ZWU4ZWNkNWRhMzMwMWIwIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBEYXZpZCBTdGVlbGUgPGRhdmlkQHBnbWFzdGVycy5u
ZXQ+CkRhdGU6IEZyaSwgMjAgRGVjIDIwMjQgMTU6MTM6NTkgKzAwMDAKU3ViamVjdDogRml4
IGxvZ2dpbmcgZm9yIGludmFsaWQgcmVjb3ZlcnkgdGltZWxpbmUuCgpJZiB0aGUgcmVxdWVz
dGVkIHJlY292ZXJ5IHRpbWVsaW5lIGlzIG5vdCByZWFjaGFibGUgdGhlIGxvZ2dlZCBjaGVj
a3BvaW50IGFuZApUTEkgc2hvdWxkIHRvIGJlIHRoZSB2YWx1ZXMgcmVhZCBmcm9tIGJhY2t1
cF9sYWJlbCB3aGVuIGJhY2t1cF9sYWJlbCBpcwpwcmVzZW50LgotLS0KIHNyYy9iYWNrZW5k
L2FjY2Vzcy90cmFuc2FtL3hsb2dyZWNvdmVyeS5jIHwgNiArKystLS0KIDEgZmlsZSBjaGFu
Z2VkLCAzIGluc2VydGlvbnMoKyksIDMgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvc3Jj
L2JhY2tlbmQvYWNjZXNzL3RyYW5zYW0veGxvZ3JlY292ZXJ5LmMgYi9zcmMvYmFja2VuZC9h
Y2Nlc3MvdHJhbnNhbS94bG9ncmVjb3ZlcnkuYwppbmRleCBjNjk5NGI3ODI4Mi4uOTlhZmQw
MWJmMzggMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL2FjY2Vzcy90cmFuc2FtL3hsb2dyZWNv
dmVyeS5jCisrKyBiL3NyYy9iYWNrZW5kL2FjY2Vzcy90cmFuc2FtL3hsb2dyZWNvdmVyeS5j
CkBAIC04NDQsMTMgKzg0NCwxMyBAQCBJbml0V2FsUmVjb3ZlcnkoQ29udHJvbEZpbGVEYXRh
ICpDb250cm9sRmlsZSwgYm9vbCAqd2FzU2h1dGRvd25fcHRyLAogCQkgKiB0bGlTd2l0Y2hQ
b2ludCB3aWxsIHRocm93IGFuIGVycm9yIGlmIHRoZSBjaGVja3BvaW50J3MgdGltZWxpbmUg
aXMKIAkJICogbm90IGluIGV4cGVjdGVkVExFcyBhdCBhbGwuCiAJCSAqLwotCQlzd2l0Y2hw
b2ludCA9IHRsaVN3aXRjaFBvaW50KENvbnRyb2xGaWxlLT5jaGVja1BvaW50Q29weS5UaGlz
VGltZUxpbmVJRCwgZXhwZWN0ZWRUTEVzLCBOVUxMKTsKKwkJc3dpdGNocG9pbnQgPSB0bGlT
d2l0Y2hQb2ludChDaGVja1BvaW50VExJLCBleHBlY3RlZFRMRXMsIE5VTEwpOwogCQllcmVw
b3J0KEZBVEFMLAogCQkJCShlcnJtc2coInJlcXVlc3RlZCB0aW1lbGluZSAldSBpcyBub3Qg
YSBjaGlsZCBvZiB0aGlzIHNlcnZlcidzIGhpc3RvcnkiLAogCQkJCQkJcmVjb3ZlcnlUYXJn
ZXRUTEkpLAogCQkJCSBlcnJkZXRhaWwoIkxhdGVzdCBjaGVja3BvaW50IGlzIGF0ICVYLyVY
IG9uIHRpbWVsaW5lICV1LCBidXQgaW4gdGhlIGhpc3Rvcnkgb2YgdGhlIHJlcXVlc3RlZCB0
aW1lbGluZSwgdGhlIHNlcnZlciBmb3JrZWQgb2ZmIGZyb20gdGhhdCB0aW1lbGluZSBhdCAl
WC8lWC4iLAotCQkJCQkJICAgTFNOX0ZPUk1BVF9BUkdTKENvbnRyb2xGaWxlLT5jaGVja1Bv
aW50KSwKLQkJCQkJCSAgIENvbnRyb2xGaWxlLT5jaGVja1BvaW50Q29weS5UaGlzVGltZUxp
bmVJRCwKKwkJCQkJCSAgIExTTl9GT1JNQVRfQVJHUyhDaGVja1BvaW50TG9jKSwKKwkJCQkJ
CSAgIENoZWNrUG9pbnRUTEksCiAJCQkJCQkgICBMU05fRk9STUFUX0FSR1Moc3dpdGNocG9p
bnQpKSkpOwogCX0KIAotLSAKMi4zNC4xCgo=

--------------UA8BIV200otwZ206Bn5qfIpx--