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 <lizi.openmind@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAAP6ZkS_3OH3yhhAGK6vu+2V1C2Hv4K6SpRuZL415R-gxjdTSg@mail.gmail.com

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, &param->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 */

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,
+	if (!pg_parse_lsn(PQgetvalue(res, 0, 1), startpos))
-			   &startpos_xrecoff) != 2)
 	{
 		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;
 }

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,
+		if (!pg_parse_lsn(PQgetvalue(res, 0, 1), &lsn_loc))
-					lo;
-
-		if (sscanf(PQgetvalue(res, 0, 1), "%X/%08X", &hi, &lo) != 2)
 		{
 			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 */

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);
@@ -234,28 +235,29 @@ line_starts_with(char *s, char *e, char *match, char **sout)
 }
 
 /*
- * Parse an LSN starting at s and not stopping at or before e. The return value
+ * Parse an LSN token starting at s and ending before the next whitespace
- * is true on success and otherwise false. On success, stores the result into
+ * character or e. The return value is true on success and otherwise false.
- * *lsn and sets *c to the first character that is not part of the LSN.
+ * On success, stores the result into *lsn and sets *c to the first character
+ * after the token.
  */
 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)
-	{
+		*c = token_end;
-		*lsn = ((XLogRecPtr) hi) << 32 | (XLogRecPtr) lo;
-		*c = s + nchars;
-	}
 
 	return success;
 }

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;

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,13 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries)
 	while (!lastline)
 	{
 		char	   *ptr;
+		char	   *token_end;
+		char		save;
 		TimeLineID	tli;
-		uint32		switchpoint_hi;
+		XLogRecPtr	switchpoint;
-		uint32		switchpoint_lo;
+		bool		valid_switchpoint = false;
-		int			nfields;
+		int			nchars;
+		size_t		nspaces;
 
 		fline = bufptr;
 		while (*bufptr && *bufptr != '\n')
@@ -66,16 +70,33 @@ 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 (sscanf(fline, "%u%n", &tli, &nchars) != 1)
-
-		if (nfields < 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;
+		nspaces = strspn(ptr, " \t\n\r\f\v");
+		if (nspaces > 0)
+		{
+			ptr += nspaces;
+
+			/*
+			 * 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';
+			valid_switchpoint = pg_parse_lsn(ptr, &switchpoint);
+			*token_end = save;
+		}
+
+		if (!valid_switchpoint)
 		{
 			pg_log_error("syntax error in history file: %s", fline);
 			pg_log_error_detail("Expected a write-ahead log switchpoint location.");
@@ -96,7 +117,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 */