From e367114429e09696fce01dd6f256e58a2c858734 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Mon, 4 Nov 2024 10:11:05 -0500
Subject: [PATCH] pg_combinebackup: Error if incremental file exists in full
 backup.

Suppose that you run a command like "pg_combinebackup b1 b2 -o output",
but both b1 and b2 contain an INCREMENTAL.$something file in a directory
that is expected to contain relation files. This is an error, but the
previous code would not detect the problem and instead write a garbage
full file named $something to the output directory. This commit adds
code to detect the error and a test case to verify the behavior.

It's difficult to imagine that this will ever happen unless someone
is intentionally trying to break incremental backup, but per discussion,
let's consider that the lack of adequate sanity checking in this area is
a bug and back-patch to v17, where incremental backup was introduced.

Patch by me, reviewed by Bertrand Drouvot and Amul Sul.

Discussion: http://postgr.es/m/CA+TgmoaD7dBYPqe7kMtO0dyto7rd0rUh7joh=JPUSaFszKY6Pg@mail.gmail.com
---
 src/bin/pg_combinebackup/meson.build          |  1 +
 src/bin/pg_combinebackup/reconstruct.c        |  8 +++
 .../pg_combinebackup/t/009_no_full_file.pl    | 66 +++++++++++++++++++
 3 files changed, 75 insertions(+)
 create mode 100644 src/bin/pg_combinebackup/t/009_no_full_file.pl

diff --git a/src/bin/pg_combinebackup/meson.build b/src/bin/pg_combinebackup/meson.build
index d142608e949..550d3503269 100644
--- a/src/bin/pg_combinebackup/meson.build
+++ b/src/bin/pg_combinebackup/meson.build
@@ -36,6 +36,7 @@ tests += {
       't/006_db_file_copy.pl',
       't/007_wal_level_minimal.pl',
       't/008_promote.pl',
+      't/009_no_full_file.pl',
     ],
   }
 }
diff --git a/src/bin/pg_combinebackup/reconstruct.c b/src/bin/pg_combinebackup/reconstruct.c
index ae8a5125263..37ae38b6108 100644
--- a/src/bin/pg_combinebackup/reconstruct.c
+++ b/src/bin/pg_combinebackup/reconstruct.c
@@ -326,11 +326,19 @@ reconstruct_from_incremental_file(char *input_filename,
 	 * result, then forget about performing reconstruction and just copy that
 	 * file in its entirety.
 	 *
+	 * If we have only incremental files, and there's no full file at any
+	 * point in the backup chain, something has gone wrong. Emit an error.
+	 *
 	 * Otherwise, reconstruct.
 	 */
 	if (copy_source != NULL)
 		copy_file(copy_source->filename, output_filename,
 				  &checksum_ctx, copy_method, dry_run);
+	else if (sidx == 0 && source[0]->header_length != 0)
+	{
+		pg_fatal("full backup contains unexpected incremental file \"%s\"",
+				 source[0]->filename);
+	}
 	else
 	{
 		write_reconstructed_file(input_filename, output_filename,
diff --git a/src/bin/pg_combinebackup/t/009_no_full_file.pl b/src/bin/pg_combinebackup/t/009_no_full_file.pl
new file mode 100644
index 00000000000..8a23fa7a4cf
--- /dev/null
+++ b/src/bin/pg_combinebackup/t/009_no_full_file.pl
@@ -0,0 +1,66 @@
+# Copyright (c) 2021-2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings FATAL => 'all';
+use File::Copy;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Can be changed to test the other modes.
+my $mode = $ENV{PG_TEST_PG_COMBINEBACKUP_MODE} || '--copy';
+
+note "testing using mode $mode";
+
+# Set up a new database instance.
+my $primary = PostgreSQL::Test::Cluster->new('primary');
+$primary->init(has_archiving => 1, allows_streaming => 1);
+$primary->append_conf('postgresql.conf', 'summarize_wal = on');
+$primary->start;
+
+# Take a full backup.
+my $backup1path = $primary->backup_dir . '/backup1';
+$primary->command_ok(
+	[ 'pg_basebackup', '-D', $backup1path, '--no-sync', '-cfast' ],
+	"full backup");
+
+# Take an incremental backup.
+my $backup2path = $primary->backup_dir . '/backup2';
+$primary->command_ok(
+	[
+		'pg_basebackup', '-D', $backup2path, '--no-sync', '-cfast',
+		'--incremental', $backup1path . '/backup_manifest'
+	],
+	"incremental backup");
+
+# Find an incremental file in the incremental backup for which there is a full
+# file in the full backup. When we find one, replace the full file with an
+# incremental file.
+my @filelist = grep { /^INCREMENTAL\./ } slurp_dir("$backup2path/base/1");
+my $success = 0;
+for my $iname (@filelist)
+{
+	my $name = $iname;
+	$name =~ s/^INCREMENTAL.//;
+
+	if (-f "$backup1path/base/1/$name")
+	{
+		copy("$backup2path/base/1/$iname", "$backup1path/base/1/$iname")
+			|| die "copy $backup2path/base/1/$iname: $!";
+		unlink("$backup1path/base/1/$name")
+			|| die "unlink $backup1path/base/1/$name: $!";
+		$success = 1;
+		last;
+	}
+}
+
+# pg_combinebackup should fail.
+my $outpath = $primary->backup_dir . '/out';
+$primary->command_fails_like(
+	[
+		'pg_combinebackup', $backup1path, $backup2path, '-o', $outpath,
+	],
+	qr/full backup contains unexpected incremental file/,
+	"pg_combinebackup fails");
+
+done_testing();