Minimal fix for unterminated tar archive problem.

robertmhaas · robertmhaas · commit 57b5a9646d97 · 2021-11-08T16:36:06.000-05:00
Commit 23a1c65 improved pg_basebackup's ability to parse tar archives, but also arranged to parse them only when we need to make some modification to the contents of the archive. That's a problem, because the server doesn't actually terminate tar archives. When the new parsing logic was engaged, pg_basebackup would properly terminate the tar file, but when it was skipped, pg_basebackup would just write whatever it got from the server, meaning that the terminator was missing. Most versions of tar are willing to overlook the missing terminator, but the AIX buildfarm animals were not. Fix by inventing a new kind of bbstreamer that just blindly adds a terminator, and using it whenever we don't parse the tar archive. Discussion: http://postgr.es/m/CA+TgmoZbNzsWwM4BE5Jb_qHncY817DYZwGf+2-7hkMQ27ZwsMQ@mail.gmail.com
diff --git a/src/bin/pg_basebackup/bbstreamer.h b/src/bin/pg_basebackup/bbstreamer.h
@@ -206,6 +206,7 @@ extern bbstreamer *bbstreamer_extractor_new(const char *basepath,
 											void (*report_output_file) (const char *));
 
 extern bbstreamer *bbstreamer_tar_parser_new(bbstreamer *next);
+extern bbstreamer *bbstreamer_tar_terminator_new(bbstreamer *next);
 extern bbstreamer *bbstreamer_tar_archiver_new(bbstreamer *next);
 
 extern bbstreamer *bbstreamer_recovery_injector_new(bbstreamer *next,
diff --git a/src/bin/pg_basebackup/bbstreamer_tar.c b/src/bin/pg_basebackup/bbstreamer_tar.c
@@ -59,6 +59,19 @@ const bbstreamer_ops bbstreamer_tar_archiver_ops = {
 	.free = bbstreamer_tar_archiver_free
 };
 
+static void bbstreamer_tar_terminator_content(bbstreamer *streamer,
+											  bbstreamer_member *member,
+											  const char *data, int len,
+											  bbstreamer_archive_context context);
+static void bbstreamer_tar_terminator_finalize(bbstreamer *streamer);
+static void bbstreamer_tar_terminator_free(bbstreamer *streamer);
+
+const bbstreamer_ops bbstreamer_tar_terminator_ops = {
+	.content = bbstreamer_tar_terminator_content,
+	.finalize = bbstreamer_tar_terminator_finalize,
+	.free = bbstreamer_tar_terminator_free
+};
+
 /*
  * Create a bbstreamer that can parse a stream of content as tar data.
  *
@@ -442,3 +455,62 @@ bbstreamer_tar_archiver_free(bbstreamer *streamer)
 	bbstreamer_free(streamer->bbs_next);
 	pfree(streamer);
 }
+
+/*
+ * Create a bbstreamer that blindly adds two blocks of NUL bytes to the
+ * end of an incomplete tarfile that the server might send us.
+ */
+bbstreamer *
+bbstreamer_tar_terminator_new(bbstreamer *next)
+{
+	bbstreamer *streamer;
+
+	streamer = palloc0(sizeof(bbstreamer));
+	*((const bbstreamer_ops **) &streamer->bbs_ops) =
+		&bbstreamer_tar_terminator_ops;
+	streamer->bbs_next = next;
+
+	return streamer;
+}
+
+/*
+ * Pass all the content through without change.
+ */
+static void
+bbstreamer_tar_terminator_content(bbstreamer *streamer,
+								  bbstreamer_member *member,
+								  const char *data, int len,
+								  bbstreamer_archive_context context)
+{
+	/* Expect unparsed input. */
+	Assert(member == NULL);
+	Assert(context == BBSTREAMER_UNKNOWN);
+
+	/* Just forward it. */
+	bbstreamer_content(streamer->bbs_next, member, data, len, context);
+}
+
+/*
+ * At the end, blindly add the two blocks of NUL bytes which the server fails
+ * to supply.
+ */
+static void
+bbstreamer_tar_terminator_finalize(bbstreamer *streamer)
+{
+	char		buffer[2 * TAR_BLOCK_SIZE];
+
+	memset(buffer, 0, 2 * TAR_BLOCK_SIZE);
+	bbstreamer_content(streamer->bbs_next, NULL, buffer,
+					   2 * TAR_BLOCK_SIZE, BBSTREAMER_UNKNOWN);
+	bbstreamer_finalize(streamer->bbs_next);
+}
+
+/*
+ * Free memory associated with a tar terminator.
+ */
+static void
+bbstreamer_tar_terminator_free(bbstreamer *streamer)
+{
+	bbstreamer_free(streamer->bbs_next);
+	pfree(streamer);
+}
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1073,10 +1073,14 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
 
 	/*
 	 * If we're doing anything that involves understanding the contents of
-	 * the archive, we'll need to parse it.
+	 * the archive, we'll need to parse it. If not, we can skip parsing it,
+	 * but the tar files the server sends are not properly terminated, so
+	 * we'll need to add the terminator here.
 	 */
 	if (must_parse_archive)
 		streamer = bbstreamer_tar_parser_new(streamer);
+	else
+		streamer = bbstreamer_tar_terminator_new(streamer);
 
 	/* Return the results. */
 	*manifest_inject_streamer_p = manifest_inject_streamer;
