agora inbox for pgsql-committers@postgresql.org
help / color / mirror / Atom feedpgsql: Move tar detection and compression logic to common.
4+ messages / 2 participants
[nested] [flat]
* pgsql: Move tar detection and compression logic to common.
@ 2026-03-20 19:32 Andrew Dunstan <andrew@dunslane.net>
2026-07-22 16:23 ` Re: pgsql: Move tar detection and compression logic to common. Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Dunstan @ 2026-03-20 19:32 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Move tar detection and compression logic to common.
Consolidate tar archive identification and compression-type detection
logic into a shared location. Currently used by pg_basebackup and
pg_verifybackup, this functionality is also required for upcoming
pg_waldump enhancements.
This change promotes code reuse and simplifies maintenance across
frontend tools.
Author: Amul Sul <sulamul@gmail.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Jakub Wartak <jakub.wartak@enterprisedb.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
discussion: https://postgr.es/m/CAAJ_b94bqdWN3h2J-PzzzQ2Npbwct5ZQHggn_QoYGhC2rn-=WQ@mail.gmail.com
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/c8a350a439826267186c187dbfbf1f839f7521aa
Modified Files
--------------
src/bin/pg_basebackup/pg_basebackup.c | 36 ++++++++++---------------------
src/bin/pg_verifybackup/pg_verifybackup.c | 12 +----------
src/common/compression.c | 30 ++++++++++++++++++++++++++
src/include/common/compression.h | 2 ++
4 files changed, 44 insertions(+), 36 deletions(-)
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pgsql: Move tar detection and compression logic to common.
2026-03-20 19:32 pgsql: Move tar detection and compression logic to common. Andrew Dunstan <andrew@dunslane.net>
@ 2026-07-22 16:23 ` Robert Haas <robertmhaas@gmail.com>
2026-07-22 19:28 ` Re: pgsql: Move tar detection and compression logic to common. Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 4+ messages in thread
From: Robert Haas @ 2026-07-22 16:23 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>; amul sul <sulamul@gmail.com>
On Fri, Mar 20, 2026 at 3:32 PM Andrew Dunstan <andrew@dunslane.net> wrote:
> Move tar detection and compression logic to common.
>
> Consolidate tar archive identification and compression-type detection
> logic into a shared location. Currently used by pg_basebackup and
> pg_verifybackup, this functionality is also required for upcoming
> pg_waldump enhancements.
>
> https://git.postgresql.org/pg/commitdiff/c8a350a439826267186c187dbfbf1f839f7521aa
Unfortunately, this commit not only consolidated the logic but also
changed the behavior at both of the then-existing call sites for the
worse.
I attach a patch to undo those behavior changes, which I intend to
commit and back-patch to v19.
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
[application/octet-stream] v1-0001-Undo-inadvertent-loosening-of-archive-filename-ch.patch (6.0K, ../../CA+TgmoYJY8FkoeYKGF_YF1S6uOK7fd0Bd3zrw0XY_oZXbmVFpQ@mail.gmail.com/2-v1-0001-Undo-inadvertent-loosening-of-archive-filename-ch.patch)
download | inline diff:
From 86fb08d25c74e8f4c3bcdeb8b0ac75e5de8ccd95 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Wed, 22 Jul 2026 11:57:46 -0400
Subject: [PATCH v1] Undo inadvertent loosening of archive filename checking.
Commit c8a350a439826267186c187dbfbf1f839f7521aa attempted to consolidate
code for identify possibly-compressed tar archives by suffix into a new
function parse_tar_compress_algorithm(). Unfortunately, the refactoring
wasn't perfect, and slightly changed the behavior at both existing call
sites.
In CreateBackupStreamer(), the previous code required the filename to
consist of more than just a suffix, so the aforementioned commit had the
effect of allowing pg_basebackup to accept a file from the server whose
entire name was something like .tar.gz -- which should never happen, but
let's reject it as previous releases did.
In precheck_tar_backup_file(), the previous code required the suffix to
be immediately adjacent to the prefix already checked, so the commit
in question allowed pg_verifybackup to accept not only filenames like
base.tar.gz but also filenames like baseFOOBARBAZ.tar.gz. While such
filenames are perhaps unlikely, rejecting them is correct, so let's go
back to that behavior.
---
src/bin/pg_basebackup/pg_basebackup.c | 4 ++--
src/bin/pg_verifybackup/pg_verifybackup.c | 2 +-
src/bin/pg_waldump/pg_waldump.c | 5 ++--
src/common/compression.c | 28 +++++++++++++++++------
src/include/common/compression.h | 2 +-
5 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8a599fc9869..12fc752bff5 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1083,8 +1083,8 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
inject_manifest = (format == 't' && strcmp(basedir, "-") == 0 && manifest);
/* Check whether it is a tar archive and its compression type */
- is_tar = parse_tar_compress_algorithm(archive_name,
- &compressed_tar_algorithm);
+ is_tar = (parse_tar_compress_algorithm(archive_name,
+ &compressed_tar_algorithm) > 0);
/* Is this any kind of compressed tar? */
is_compressed_tar = (is_tar &&
diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c
index 05385a91e88..90a3625130d 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.c
+++ b/src/bin/pg_verifybackup/pg_verifybackup.c
@@ -972,7 +972,7 @@ precheck_tar_backup_file(verifier_context *context, char *relpath,
}
/* Now, check the compression type of the tar */
- if (!parse_tar_compress_algorithm(suffix, &compress_algorithm))
+ if (parse_tar_compress_algorithm(suffix, &compress_algorithm) != 0)
{
report_backup_error(context,
"file \"%s\" is not expected in a tar format backup",
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index cf760d8b236..ffe6e8a6bca 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1242,7 +1242,7 @@ main(int argc, char **argv)
if (waldir != NULL)
{
/* Check whether the path looks like a tar archive by its extension */
- if (parse_tar_compress_algorithm(waldir, &compression))
+ if (parse_tar_compress_algorithm(waldir, &compression) >= 0)
{
split_path(waldir, &private.archive_dir, &private.archive_name);
}
@@ -1286,7 +1286,8 @@ main(int argc, char **argv)
pg_fatal("could not open directory \"%s\": %m", waldir);
}
- if (fname != NULL && parse_tar_compress_algorithm(fname, &compression))
+ if (fname != NULL &&
+ parse_tar_compress_algorithm(fname, &compression) >= 0)
{
private.archive_dir = waldir;
private.archive_name = fname;
diff --git a/src/common/compression.c b/src/common/compression.c
index ae2089d9406..e78913ad9dc 100644
--- a/src/common/compression.c
+++ b/src/common/compression.c
@@ -42,33 +42,47 @@ static bool expect_boolean_value(char *keyword, char *value,
pg_compress_specification *result);
/*
- * Look up a compression algorithm by archive file extension. Returns true and
- * sets *algorithm if the extension is recognized. Otherwise returns false.
+ * Look up a compression algorithm by archive file extension. Sets *algorithm
+ * and returns the length of the non-extension portion of the filename, or -1
+ * if the filename does not end with a recognized tar extension.
*/
-bool
+int
parse_tar_compress_algorithm(const char *fname, pg_compress_algorithm *algorithm)
{
- size_t fname_len = strlen(fname);
+ int fname_len = strlen(fname);
if (fname_len >= 4 &&
strcmp(fname + fname_len - 4, ".tar") == 0)
+ {
*algorithm = PG_COMPRESSION_NONE;
+ return fname_len - 4;
+ }
else if (fname_len >= 4 &&
strcmp(fname + fname_len - 4, ".tgz") == 0)
+ {
*algorithm = PG_COMPRESSION_GZIP;
+ return fname_len - 4;
+ }
else if (fname_len >= 7 &&
strcmp(fname + fname_len - 7, ".tar.gz") == 0)
+ {
*algorithm = PG_COMPRESSION_GZIP;
+ return fname_len - 7;
+ }
else if (fname_len >= 8 &&
strcmp(fname + fname_len - 8, ".tar.lz4") == 0)
+ {
*algorithm = PG_COMPRESSION_LZ4;
+ return fname_len - 8;
+ }
else if (fname_len >= 8 &&
strcmp(fname + fname_len - 8, ".tar.zst") == 0)
+ {
*algorithm = PG_COMPRESSION_ZSTD;
- else
- return false;
+ return fname_len - 8;
+ }
- return true;
+ return -1;
}
/*
diff --git a/src/include/common/compression.h b/src/include/common/compression.h
index f99c747cdd3..adbe668a105 100644
--- a/src/include/common/compression.h
+++ b/src/include/common/compression.h
@@ -41,7 +41,7 @@ typedef struct pg_compress_specification
extern void parse_compress_options(const char *option, char **algorithm,
char **detail);
-extern bool parse_tar_compress_algorithm(const char *fname,
+extern int parse_tar_compress_algorithm(const char *fname,
pg_compress_algorithm *algorithm);
extern bool parse_compress_algorithm(char *name, pg_compress_algorithm *algorithm);
extern const char *get_compress_algorithm_name(pg_compress_algorithm algorithm);
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pgsql: Move tar detection and compression logic to common.
2026-03-20 19:32 pgsql: Move tar detection and compression logic to common. Andrew Dunstan <andrew@dunslane.net>
2026-07-22 16:23 ` Re: pgsql: Move tar detection and compression logic to common. Robert Haas <robertmhaas@gmail.com>
@ 2026-07-22 19:28 ` Andrew Dunstan <andrew@dunslane.net>
2026-07-22 19:55 ` Re: pgsql: Move tar detection and compression logic to common. Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Dunstan @ 2026-07-22 19:28 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>; amul sul <sulamul@gmail.com>
On Wed, Jul 22, 2026 at 12:23 PM Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Mar 20, 2026 at 3:32 PM Andrew Dunstan <andrew@dunslane.net>
> wrote:
> > Move tar detection and compression logic to common.
> >
> > Consolidate tar archive identification and compression-type detection
> > logic into a shared location. Currently used by pg_basebackup and
> > pg_verifybackup, this functionality is also required for upcoming
> > pg_waldump enhancements.
> >
> >
> https://git.postgresql.org/pg/commitdiff/c8a350a439826267186c187dbfbf1f839f7521aa
>
> Unfortunately, this commit not only consolidated the logic but also
> changed the behavior at both of the then-existing call sites for the
> worse.
>
> I attach a patch to undo those behavior changes, which I intend to
> commit and back-patch to v19.
>
>
Thanks, Robert. The change in precheck_tar_backup could possibly do with a
comment like "Make sure there's nothing between the expected prefix and
suffix". I had to look hard to see what it was doing.
cheers
andrew
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pgsql: Move tar detection and compression logic to common.
2026-03-20 19:32 pgsql: Move tar detection and compression logic to common. Andrew Dunstan <andrew@dunslane.net>
2026-07-22 16:23 ` Re: pgsql: Move tar detection and compression logic to common. Robert Haas <robertmhaas@gmail.com>
2026-07-22 19:28 ` Re: pgsql: Move tar detection and compression logic to common. Andrew Dunstan <andrew@dunslane.net>
@ 2026-07-22 19:55 ` Robert Haas <robertmhaas@gmail.com>
0 siblings, 0 replies; 4+ messages in thread
From: Robert Haas @ 2026-07-22 19:55 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>; amul sul <sulamul@gmail.com>
On Wed, Jul 22, 2026 at 3:28 PM Andrew Dunstan <andrew@dunslane.net> wrote:
> Thanks, Robert. The change in precheck_tar_backup could possibly do with a comment like "Make sure there's nothing between the expected prefix and suffix". I had to look hard to see what it was doing.
Cool. I'll add some kind of a comment there before committing. Thanks
for the quick response.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-07-22 19:55 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-20 19:32 pgsql: Move tar detection and compression logic to common. Andrew Dunstan <andrew@dunslane.net>
2026-07-22 16:23 ` Robert Haas <robertmhaas@gmail.com>
2026-07-22 19:28 ` Andrew Dunstan <andrew@dunslane.net>
2026-07-22 19:55 ` Robert Haas <robertmhaas@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox