From fab6ab010726333bb17d7c60c7a91cd0e6b33577 Mon Sep 17 00:00:00 2001 From: Zexin Li Date: Fri, 14 Aug 2026 00:37:28 +0000 Subject: [PATCH v2] Use pg_parse_lsn() for server-supplied LSNs Commit d6bf0ab170 introduced pg_parse_lsn() to validate LSNs given on the command line of pg_waldump, pg_recvlogical, and pg_receivewal. The remaining sscanf("%X/%08X") call sites under src/bin parse LSNs that arrive in server responses, timeline history files, and backup_label files. sscanf() accepts several forms that pg_lsn input rejects and can silently continue with a different location than the input text: a first component wider than eight hex digits wraps around, a wider second component is truncated, and leading whitespace, signs, "0x" prefixes, and trailing characters are consumed or ignored. Convert those call sites as well. The two call sites that read a location out of a longer line isolate it by temporarily terminating the string at the next whitespace character, so that they can use pg_parse_lsn() like the others. Each tool keeps its existing error message. Malformed metadata now fails with each tool's existing error instead of silently proceeding with a different location. Two error paths shift: pg_rewind's history-file parser now rejects trailing characters attached to a switchpoint, which used to be ignored, and a malformed backup_label location now fails pg_combinebackup's "could not parse" check rather than its "improper terminator" check. Author: Zexin Li --- src/bin/pg_basebackup/pg_basebackup.c | 16 ++++---------- src/bin/pg_basebackup/receivelog.c | 8 ++----- src/bin/pg_basebackup/streamutil.c | 12 +++------- src/bin/pg_combinebackup/backup_label.c | 17 ++++++++------- src/bin/pg_rewind/libpq_source.c | 7 ++---- src/bin/pg_rewind/timeline.c | 29 ++++++++++++++++++------- 6 files changed, 41 insertions(+), 48 deletions(-) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 12fc752bff5..c3b87a19e76 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -32,6 +32,7 @@ #include "common/file_perm.h" #include "common/file_utils.h" #include "common/logging.h" +#include "common/pg_parse_lsn.h" #include "fe_utils/option_utils.h" #include "fe_utils/recovery_gen.h" #include "getopt_long.h" @@ -482,17 +483,14 @@ reached_end_position(XLogRecPtr segendpos, uint32 timeline, { ssize_t nread; char xlogend[64] = {0}; - uint32 hi, - lo; nread = read(bgpipe[0], xlogend, sizeof(xlogend) - 1); if (nread < 0) pg_fatal("could not read from ready pipe: %m"); - if (sscanf(xlogend, "%X/%08X", &hi, &lo) != 2) + if (!pg_parse_lsn(xlogend, &xlogendptr)) pg_fatal("could not parse write-ahead log location \"%s\"", xlogend); - xlogendptr = ((uint64) hi) << 32 | lo; has_xlogendptr = 1; /* @@ -620,8 +618,6 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier, int wal_compress_level) { logstreamer_param *param; - uint32 hi, - lo; char statusdir[MAXPGPATH]; param = pg_malloc0_object(logstreamer_param); @@ -631,10 +627,9 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier, param->wal_compress_level = wal_compress_level; /* Convert the starting position */ - if (sscanf(startpos, "%X/%08X", &hi, &lo) != 2) + if (!pg_parse_lsn(startpos, ¶m->startptr)) pg_fatal("could not parse write-ahead log location \"%s\"", startpos); - param->startptr = ((uint64) hi) << 32 | lo; /* Round off to even segment position */ param->startptr -= XLogSegmentOffset(param->startptr, WalSegSz); @@ -2216,8 +2211,6 @@ BaseBackup(char *compression_algorithm, char *compression_detail, * casting to a different size on WIN64. */ intptr_t bgchild_handle = bgchild; - uint32 hi, - lo; #endif if (verbose) @@ -2243,10 +2236,9 @@ BaseBackup(char *compression_algorithm, char *compression_detail, * value directly in the variable, and then set the flag that says * it's there. */ - if (sscanf(xlogend, "%X/%08X", &hi, &lo) != 2) + if (!pg_parse_lsn(xlogend, &xlogendptr)) pg_fatal("could not parse write-ahead log location \"%s\"", xlogend); - xlogendptr = ((uint64) hi) << 32 | lo; InterlockedIncrement(&has_xlogendptr); /* First wait for the thread to exit */ diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c index faa60711b1b..77a2b4458b3 100644 --- a/src/bin/pg_basebackup/receivelog.c +++ b/src/bin/pg_basebackup/receivelog.c @@ -20,6 +20,7 @@ #include "access/xlog_internal.h" #include "common/logging.h" +#include "common/pg_parse_lsn.h" #include "libpq-fe.h" #include "libpq/protocol.h" #include "receivelog.h" @@ -704,9 +705,6 @@ error: static bool ReadEndOfStreamingResult(PGresult *res, XLogRecPtr *startpos, uint32 *timeline) { - uint32 startpos_xlogid, - startpos_xrecoff; - /*---------- * The result set consists of one row and two columns, e.g: * @@ -727,14 +725,12 @@ ReadEndOfStreamingResult(PGresult *res, XLogRecPtr *startpos, uint32 *timeline) } *timeline = atoi(PQgetvalue(res, 0, 0)); - if (sscanf(PQgetvalue(res, 0, 1), "%X/%08X", &startpos_xlogid, - &startpos_xrecoff) != 2) + if (!pg_parse_lsn(PQgetvalue(res, 0, 1), startpos)) { pg_log_error("could not parse next timeline's starting point \"%s\"", PQgetvalue(res, 0, 1)); return false; } - *startpos = ((uint64) startpos_xlogid << 32) | startpos_xrecoff; return true; } diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c index 8fcd690f155..8086fde84db 100644 --- a/src/bin/pg_basebackup/streamutil.c +++ b/src/bin/pg_basebackup/streamutil.c @@ -21,6 +21,7 @@ #include "common/connect.h" #include "common/file_perm.h" #include "common/logging.h" +#include "common/pg_parse_lsn.h" #include "common/string.h" #include "datatype/timestamp.h" #include "port/pg_bswap.h" @@ -410,8 +411,6 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli, XLogRecPtr *startpos, char **db_name) { PGresult *res; - uint32 hi, - lo; /* Check connection existence */ Assert(conn != NULL); @@ -445,7 +444,7 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli, /* Get LSN start position if necessary */ if (startpos != NULL) { - if (sscanf(PQgetvalue(res, 0, 2), "%X/%08X", &hi, &lo) != 2) + if (!pg_parse_lsn(PQgetvalue(res, 0, 2), startpos)) { pg_log_error("could not parse write-ahead log location \"%s\"", PQgetvalue(res, 0, 2)); @@ -453,7 +452,6 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli, PQclear(res); return false; } - *startpos = ((uint64) hi) << 32 | lo; } /* Get database name, only available in 9.4 and newer versions */ @@ -549,17 +547,13 @@ GetSlotInformation(PGconn *conn, const char *slot_name, /* restart LSN */ if (!PQgetisnull(res, 0, 1)) { - uint32 hi, - lo; - - if (sscanf(PQgetvalue(res, 0, 1), "%X/%08X", &hi, &lo) != 2) + if (!pg_parse_lsn(PQgetvalue(res, 0, 1), &lsn_loc)) { pg_log_error("could not parse restart_lsn \"%s\" for replication slot \"%s\"", PQgetvalue(res, 0, 1), slot_name); PQclear(res); return false; } - lsn_loc = ((uint64) hi) << 32 | lo; } /* current TLI */ diff --git a/src/bin/pg_combinebackup/backup_label.c b/src/bin/pg_combinebackup/backup_label.c index b757e772b92..fd8f493a262 100644 --- a/src/bin/pg_combinebackup/backup_label.c +++ b/src/bin/pg_combinebackup/backup_label.c @@ -17,6 +17,7 @@ #include "backup_label.h" #include "common/file_perm.h" #include "common/logging.h" +#include "common/pg_parse_lsn.h" #include "write_manifest.h" static int get_eol_offset(StringInfo buf); @@ -242,20 +243,20 @@ static bool parse_lsn(char *s, char *e, XLogRecPtr *lsn, char **c) { char save = *e; - int nchars; + char *token_end; + char save_token_end; bool success; - unsigned hi; - unsigned lo; *e = '\0'; - success = (sscanf(s, "%X/%08X%n", &hi, &lo, &nchars) == 2); + token_end = s + strcspn(s, " \t\n\r\f\v"); + save_token_end = *token_end; + *token_end = '\0'; + success = pg_parse_lsn(s, lsn); + *token_end = save_token_end; *e = save; if (success) - { - *lsn = ((XLogRecPtr) hi) << 32 | (XLogRecPtr) lo; - *c = s + nchars; - } + *c = token_end; return success; } diff --git a/src/bin/pg_rewind/libpq_source.c b/src/bin/pg_rewind/libpq_source.c index 216755b3ddb..abfbd312580 100644 --- a/src/bin/pg_rewind/libpq_source.c +++ b/src/bin/pg_rewind/libpq_source.c @@ -11,6 +11,7 @@ #include "catalog/pg_type_d.h" #include "common/connect.h" +#include "common/pg_parse_lsn.h" #include "file_ops.h" #include "filemap.h" #include "lib/stringinfo.h" @@ -209,17 +210,13 @@ libpq_get_current_wal_insert_lsn(rewind_source *source) { PGconn *conn = ((libpq_source *) source)->conn; XLogRecPtr result; - uint32 hi; - uint32 lo; char *val; val = run_simple_query(conn, "SELECT pg_current_wal_insert_lsn()"); - if (sscanf(val, "%X/%08X", &hi, &lo) != 2) + if (!pg_parse_lsn(val, &result)) pg_fatal("unrecognized result \"%s\" for current WAL insert location", val); - result = ((uint64) hi) << 32 | lo; - pg_free(val); return result; diff --git a/src/bin/pg_rewind/timeline.c b/src/bin/pg_rewind/timeline.c index dda06eaa0bc..85f088ef684 100644 --- a/src/bin/pg_rewind/timeline.c +++ b/src/bin/pg_rewind/timeline.c @@ -10,6 +10,7 @@ #include "postgres_fe.h" #include "access/timeline.h" +#include "common/pg_parse_lsn.h" #include "pg_rewind.h" /* @@ -44,10 +45,12 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries) while (!lastline) { char *ptr; + char *token_end; + char save; TimeLineID tli; - uint32 switchpoint_hi; - uint32 switchpoint_lo; - int nfields; + XLogRecPtr switchpoint; + bool success; + int nchars; fline = bufptr; while (*bufptr && *bufptr != '\n') @@ -66,16 +69,26 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries) if (*ptr == '\0' || *ptr == '#') continue; - nfields = sscanf(fline, "%u\t%X/%08X", &tli, &switchpoint_hi, &switchpoint_lo); - - if (nfields < 1) + if (sscanf(fline, "%u%n", &tli, &nchars) != 1) { /* expect a numeric timeline ID as first field of line */ pg_log_error("syntax error in history file: %s", fline); pg_log_error_detail("Expected a numeric timeline ID."); exit(1); } - if (nfields != 3) + + /* the switchpoint location follows, separated by whitespace */ + ptr = fline + nchars; + ptr += strspn(ptr, " \t\n\r\f\v"); + + /* isolate the location from the rest of the line before parsing it */ + token_end = ptr + strcspn(ptr, " \t\n\r\f\v"); + save = *token_end; + *token_end = '\0'; + success = pg_parse_lsn(ptr, &switchpoint); + *token_end = save; + + if (!success) { pg_log_error("syntax error in history file: %s", fline); pg_log_error_detail("Expected a write-ahead log switchpoint location."); @@ -96,7 +109,7 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries) entry = &entries[nlines - 1]; entry->tli = tli; entry->begin = prevend; - entry->end = ((uint64) (switchpoint_hi)) << 32 | (uint64) switchpoint_lo; + entry->end = switchpoint; prevend = entry->end; /* we ignore the remainder of each line */ -- 2.34.1