pg_restore --format= option(without any value) should report an error as pg_dump is reporting an error

Started by Mahendra Singh Thalor9 months ago1 messages
#1Mahendra Singh Thalor
mahi6run@gmail.com
1 attachment(s)

Hi,
With "pg_restore --format=", we are not giving any error because in code,
we are checking length of arg but pg_dump is reporting an error for the
same option.

For the consistency purpose, pg_dump and pg_restore both should report an
error for the test case below.

*Ex: (output after this patch)but before this patch, below command is
passing.*
/pg_restore x1 -d postgres -j 10 -C --verbose --format=
pg_restore: error: unrecognized archive format ""; please specify "c", "d",
or "t"

Here, I am attaching a patch which is fixing the same. I added 2 TAP tests
also for invalid options.

*Note:* We have 2 more options in pg_restore code which validate the option
if arg has non zero length. I will prepare patches for both(--host and
--port). We need to add some validate function also for both these options.

--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com

Attachments:

v01-pg_restore-format-option-should-validate-all-values.patchapplication/octet-stream; name=v01-pg_restore-format-option-should-validate-all-values.patchDownload
From b553405e50b25ffcf9d408e21d1f926c6452748d Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <mahi6run@gmail.com>
Date: Sun, 13 Apr 2025 19:17:57 +0530
Subject: [PATCH] pg_restore --format option should validate all values

With "pg_restore --format=", we are not giving any error because in code, we are checking
length of arg but pg_dump is troughing an error for the same option.

Ex: (after this patch)-- before this patch, below command is passing.
/pg_restore  x1 -d postgres -j 10 -C --verbose --format=
pg_restore: error: unrecognized archive format ""; please specify "c", "d", or "t"
---
 src/bin/pg_dump/pg_restore.c   |  3 +--
 src/bin/pg_dump/t/001_basic.pl | 10 ++++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)
 mode change 100644 => 100755 src/bin/pg_dump/t/001_basic.pl

diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index ff4bb320fc9..c86d593df92 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -217,8 +217,7 @@ main(int argc, char **argv)
 				opts->filename = pg_strdup(optarg);
 				break;
 			case 'F':
-				if (strlen(optarg) != 0)
-					opts->formatName = pg_strdup(optarg);
+				opts->formatName = pg_strdup(optarg);
 				break;
 			case 'g':
 				/* restore only global.dat file from directory */
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
old mode 100644
new mode 100755
index 84ca25e17d6..67b97ec5b92
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -201,6 +201,16 @@ command_fails_like(
 	qr/\Qpg_restore: error: unrecognized archive format "garbage";\E/,
 	'pg_dump: unrecognized archive format');
 
+command_fails_like(
+	[ 'pg_restore', '-f -', '--format='],
+	qr/\Qpg_restore: error: unrecognized archive format "";\E/,
+	'pg_dump: unrecognized archive format empty string');
+
+command_fails_like(
+	[ 'pg_restore', '-f -', '-F', 'p' ],
+	qr/\Qpg_restore: error: archive format "p" is not supported; please use psql\E/,
+	'pg_dump: unrecognized archive format p|plain');
+
 command_fails_like(
 	[ 'pg_dump', '--on-conflict-do-nothing' ],
 	qr/pg_dump: error: option --on-conflict-do-nothing requires option --inserts, --rows-per-insert, or --column-inserts/,
-- 
2.39.3