Minimal fix for unterminated tar archive problem.

Commit 23a1c6578c87fca0e361c4f5f9a07df5ae1f9858 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
index b24dc848c1b8a90a6737f23961c645f1d3700c61..2fd50b92d923a3f5a7f84569be760bbf64fe0bcf 100644 (file)
--- 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
index 5a9f587dca175f764944021980e2ce3d316caecb..5fded0f4e6f3d1ef35c3040360036d83b7cf22b7 100644 (file)
--- 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
index 169afa5645d8c82591026ca734580e034484b750..30efc03b830c47f8d7f91a38c1daaa24dd076c2d 100644 (file)
--- 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;