fix_pg_logicalinspect.patch

application/octet-stream

Filename: fix_pg_logicalinspect.patch
Type: application/octet-stream
Part: 0
Message: Re: BUG #18828: Crash when pg_get_logical_snapshot_meta() passed empty string

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: unified
File+
contrib/pg_logicalinspect/pg_logicalinspect.c 25 10
src/backend/replication/logical/snapbuild.c 7 7
src/include/replication/snapbuild_internal.h 1 1
diff --git a/contrib/pg_logicalinspect/pg_logicalinspect.c b/contrib/pg_logicalinspect/pg_logicalinspect.c
index cd575c6bd36..98f204ee492 100644
--- a/contrib/pg_logicalinspect/pg_logicalinspect.c
+++ b/contrib/pg_logicalinspect/pg_logicalinspect.c
@@ -48,6 +48,25 @@ get_snapbuild_state_desc(SnapBuildState state)
 	return stateDesc;
 }
 
+/*
+ * Extract LSN from the given serialized snapshot file name.
+ */
+static XLogRecPtr
+parse_snapshot_filename(const char *filename)
+{
+	uint32		hi;
+	uint32		lo;
+	XLogRecPtr	lsn;
+
+	if (sscanf(filename, "%X-%X.snap", &hi, &lo) != 2)
+		ereport(ERROR,
+				errmsg("invalid serialized snapshot file name \"%s\"", filename));
+
+	lsn = ((uint64) hi) << 32 | lo;
+
+	return lsn;
+}
+
 /*
  * Retrieve the logical snapshot file metadata.
  */
@@ -60,7 +79,7 @@ pg_get_logical_snapshot_meta(PG_FUNCTION_ARGS)
 	Datum		values[PG_GET_LOGICAL_SNAPSHOT_META_COLS] = {0};
 	bool		nulls[PG_GET_LOGICAL_SNAPSHOT_META_COLS] = {0};
 	TupleDesc	tupdesc;
-	char		path[MAXPGPATH];
+	XLogRecPtr	lsn;
 	int			i = 0;
 	text	   *filename_t = PG_GETARG_TEXT_PP(0);
 
@@ -68,12 +87,10 @@ pg_get_logical_snapshot_meta(PG_FUNCTION_ARGS)
 	if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
 		elog(ERROR, "return type must be a row type");
 
-	sprintf(path, "%s/%s",
-			PG_LOGICAL_SNAPSHOTS_DIR,
-			text_to_cstring(filename_t));
+	lsn = parse_snapshot_filename(text_to_cstring(filename_t));
 
 	/* Validate and restore the snapshot to 'ondisk' */
-	SnapBuildRestoreSnapshot(&ondisk, path, CurrentMemoryContext, false);
+	SnapBuildRestoreSnapshot(&ondisk, lsn, CurrentMemoryContext, false);
 
 	values[i++] = UInt32GetDatum(ondisk.magic);
 	values[i++] = Int64GetDatum((int64) ondisk.checksum);
@@ -97,7 +114,7 @@ pg_get_logical_snapshot_info(PG_FUNCTION_ARGS)
 	Datum		values[PG_GET_LOGICAL_SNAPSHOT_INFO_COLS] = {0};
 	bool		nulls[PG_GET_LOGICAL_SNAPSHOT_INFO_COLS] = {0};
 	TupleDesc	tupdesc;
-	char		path[MAXPGPATH];
+	XLogRecPtr	lsn;
 	int			i = 0;
 	text	   *filename_t = PG_GETARG_TEXT_PP(0);
 
@@ -105,12 +122,10 @@ pg_get_logical_snapshot_info(PG_FUNCTION_ARGS)
 	if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
 		elog(ERROR, "return type must be a row type");
 
-	sprintf(path, "%s/%s",
-			PG_LOGICAL_SNAPSHOTS_DIR,
-			text_to_cstring(filename_t));
+	lsn = parse_snapshot_filename(text_to_cstring(filename_t));
 
 	/* Validate and restore the snapshot to 'ondisk' */
-	SnapBuildRestoreSnapshot(&ondisk, path, CurrentMemoryContext, false);
+	SnapBuildRestoreSnapshot(&ondisk, lsn, CurrentMemoryContext, false);
 
 	values[i++] = CStringGetTextDatum(get_snapbuild_state_desc(ondisk.builder.state));
 	values[i++] = TransactionIdGetDatum(ondisk.builder.xmin);
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index bd0680dcbe5..b64e53de017 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1691,12 +1691,17 @@ out:
  * If 'missing_ok' is true, will not throw an error if the file is not found.
  */
 bool
-SnapBuildRestoreSnapshot(SnapBuildOnDisk *ondisk, const char *path,
+SnapBuildRestoreSnapshot(SnapBuildOnDisk *ondisk, XLogRecPtr lsn,
 						 MemoryContext context, bool missing_ok)
 {
 	int			fd;
 	pg_crc32c	checksum;
 	Size		sz;
+	char		path[MAXPGPATH];
+
+	sprintf(path, "%s/%X-%X.snap",
+			PG_LOGICAL_SNAPSHOTS_DIR,
+			LSN_FORMAT_ARGS(lsn));
 
 	fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
 
@@ -1788,18 +1793,13 @@ static bool
 SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
 {
 	SnapBuildOnDisk ondisk;
-	char		path[MAXPGPATH];
 
 	/* no point in loading a snapshot if we're already there */
 	if (builder->state == SNAPBUILD_CONSISTENT)
 		return false;
 
-	sprintf(path, "%s/%X-%X.snap",
-			PG_LOGICAL_SNAPSHOTS_DIR,
-			LSN_FORMAT_ARGS(lsn));
-
 	/* validate and restore the snapshot to 'ondisk' */
-	if (!SnapBuildRestoreSnapshot(&ondisk, path, builder->context, true))
+	if (!SnapBuildRestoreSnapshot(&ondisk, lsn, builder->context, true))
 		return false;
 
 	/*
diff --git a/src/include/replication/snapbuild_internal.h b/src/include/replication/snapbuild_internal.h
index 081b01b890a..3b915dc8793 100644
--- a/src/include/replication/snapbuild_internal.h
+++ b/src/include/replication/snapbuild_internal.h
@@ -193,7 +193,7 @@ typedef struct SnapBuildOnDisk
 	/* variable amount of TransactionIds follows */
 } SnapBuildOnDisk;
 
-extern bool SnapBuildRestoreSnapshot(SnapBuildOnDisk *ondisk, const char *path,
+extern bool SnapBuildRestoreSnapshot(SnapBuildOnDisk *ondisk, XLogRecPtr lsn,
 									 MemoryContext context, bool missing_ok);
 
 #endif							/* SNAPBUILD_INTERNAL_H */