public inbox for [email protected]
help / color / mirror / Atom feedFrom: Mahendra Singh Thalor <[email protected]>
To: Tom Lane <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: Álvaro Herrera <[email protected]>
Cc: Srinath Reddy <[email protected]>
Cc: [email protected]
Subject: Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
Date: Mon, 2 Feb 2026 11:20:28 +0530
Message-ID: <CAKYtNAr0TcS4NWgduwdqTcApTs_RBaiRY+C+WAay3-xXjkDC_w@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<CAKYtNArodApzf7OkF49VS4eO-u_Qwy0Pup3FMTw_QoLi5xeczQ@mail.gmail.com>
<Z_krlRC0ifpW3clw@nathan>
<CAKYtNAq1-Bu1owjQxQbwZiwx+WwPKkcB2wwd02UkRWehsRnDhw@mail.gmail.com>
<CAKYtNArwMfw1OSTSucZz1ei829xpzMKL50rXDDV02-5Z61Yv-w@mail.gmail.com>
<[email protected]>
<[email protected]>
Thanks Tom for the review.
On Thu, 29 Jan 2026 at 21:54, Tom Lane <[email protected]> wrote:
>
> Andrew Dunstan <[email protected]> writes:
> > These patches need a little copy editing (e.g.
> > "check_database_role_names_in_old_cluser" seems to be missing a "t") and
> > the error messages and comments need some tidying, but I think they are
> > basically sound.
Thanks Andrew for the review.
Fixed this typo.
> > Is there any objection to them in principle?
>
> +1 in principle. As you say, there's some tidying needed.
> A couple of points I noted:
>
> 1. check_lfcr_in_objname is about as unmusical a name as I can readily
> imagine. I was thinking about proposing "reject_newline_in_name"
> instead, but really I would drop that subroutine altogether and just
> code the checks in-line, because:
Fixed. As of now, I renamed it to reject_newline_in_name and changed
the error message as per suggestion but I kept this function as
previously suggested by some reviewers but I can remove this if this
looks odd.
>
> 2. I don't think this approach to constructing the error message
> meets our translatability guidelines. Better to just write out
> "role name \"%s\" contains ..." or "database name \"%s\" contains
> ...". We do use the other approach in some cases where it saves
> dozens of repetitive messages, but when there are only ever going
> to be two I'd rather err on the side of translatability.
>
Fixed.
> 3. I do not like the tests added to 040_createuser.pl, as they
> do not verify that the command fails for the expected reason.
>
Fixed. Added test case in .sql file to verify invalid names.
> 4. There's no point in running check_database_role_names_in_old_cluser
> against a v19 or later source server.
Fixed.
>
> regards, tom lane
Here, I am attaching updated patches for the review.
--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com
Attachments:
[application/octet-stream] v08_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch (8.5K, ../CAKYtNAr0TcS4NWgduwdqTcApTs_RBaiRY+C+WAay3-xXjkDC_w@mail.gmail.com/2-v08_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch)
download | inline diff:
From b5091279568339213978adc42d98b1053c17b23d Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Mon, 2 Feb 2026 10:52:29 +0530
Subject: [PATCH 1/2] don't allow newline or carriage return character in name
for database/user/role
While creating database, if database name has any newline or carriage
return character in name, then through error becuase these special
character are not allowed in dbname when dump command is executed.
do same for "CREATE ROLE", "CREATE USER" also.
This will add check for:
"CREATE DATABASE", "CREATE ROLE", "CREATE USER",
"RENAME DATABASE/USER/ROLE"
-------------------------
As we will not allow these \n\r in names, then we will never get these
names in dump also.
If we are dumping from older branch, then we will fail with same old error.
(dump will fail in older branches so no need to add extra handling for dump.)
Also remove comment added by 142c24c23447f212e642a0ffac9af878b93f490d commit.
Remove one test of 8b845520fb0aa50fea7aae44a45cee1b6d87845d commit.
---
src/backend/commands/dbcommands.c | 4 ++++
src/backend/commands/define.c | 17 +++++++++++++++++
src/backend/commands/user.c | 4 ++++
src/bin/pg_dump/t/010_dump_connstr.pl | 14 --------------
src/bin/scripts/t/020_createdb.pl | 12 ++++++++++++
src/fe_utils/string_utils.c | 6 ------
src/include/commands/defrem.h | 1 +
.../modules/unsafe_tests/expected/rolenames.out | 4 ++++
src/test/modules/unsafe_tests/sql/rolenames.sql | 2 ++
9 files changed, 44 insertions(+), 20 deletions(-)
mode change 100644 => 100755 src/bin/pg_dump/t/010_dump_connstr.pl
mode change 100644 => 100755 src/bin/scripts/t/020_createdb.pl
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 87949054f26..a89604c3d52 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -742,6 +742,8 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
createdb_failure_params fparms;
+ reject_newline_in_name(dbname, "database");
+
/* Extract options from the statement node tree */
foreach(option, stmt->options)
{
@@ -1910,6 +1912,8 @@ RenameDatabase(const char *oldname, const char *newname)
int npreparedxacts;
ObjectAddress address;
+ reject_newline_in_name(newname, "database");
+
/*
* Look up the target database's OID, and get exclusive lock on it. We
* need this for the same reasons as DROP DATABASE.
diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
index 4172cc9bacb..bbd06ffd190 100644
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -374,3 +374,20 @@ errorConflictingDefElem(DefElem *defel, ParseState *pstate)
errmsg("conflicting or redundant options"),
parser_errposition(pstate, defel->location));
}
+
+/*
+ * reject_newline_in_name
+ *
+ * If name has a newline or carriage return character then error will be reported
+ * as these special characters are not allowed in names due to shell command error
+ * in dump.
+ */
+void
+reject_newline_in_name(const char *objname, const char *objtype)
+{
+ /* Report error if name has \n or \r character. */
+ if (strpbrk(objname, "\n\r"))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+ errmsg("\"%s\" name \"%s\" contains a newline or carriage return character",objtype, objname));
+}
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 8fb9ea25db0..877c8236fab 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -171,6 +171,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
DefElem *dbypassRLS = NULL;
GrantRoleOptions popt;
+ reject_newline_in_name(stmt->role, "role");
+
/* The defaults can vary depending on the original statement type */
switch (stmt->stmt_type)
{
@@ -1348,6 +1350,8 @@ RenameRole(const char *oldname, const char *newname)
ObjectAddress address;
Form_pg_authid authform;
+ reject_newline_in_name(newname, "role");
+
rel = table_open(AuthIdRelationId, RowExclusiveLock);
dsc = RelationGetDescr(rel);
diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl
old mode 100644
new mode 100755
index dc7a33658db..bf2c3b6d00b
--- a/src/bin/pg_dump/t/010_dump_connstr.pl
+++ b/src/bin/pg_dump/t/010_dump_connstr.pl
@@ -153,20 +153,6 @@ $node->command_ok(
],
'pg_dumpall --dbname accepts connection string');
-$node->run_log(
- [ 'createdb', '--username' => $src_bootstrap_super, "foo\n\rbar" ]);
-
-# not sufficient to use --roles-only here
-$node->command_fails(
- [
- 'pg_dumpall', '--no-sync',
- '--username' => $src_bootstrap_super,
- '--file' => $discard,
- ],
- 'pg_dumpall with \n\r in database name');
-$node->run_log(
- [ 'dropdb', '--username' => $src_bootstrap_super, "foo\n\rbar" ]);
-
# make a table, so the parallel worker has something to dump
$node->safe_psql(
diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl
old mode 100644
new mode 100755
index 83b0077383a..18ff0254d59
--- a/src/bin/scripts/t/020_createdb.pl
+++ b/src/bin/scripts/t/020_createdb.pl
@@ -241,6 +241,18 @@ $node->command_fails(
],
'fails for invalid locale provider');
+$node->command_fails_like(
+ [ 'createdb', "invalid \n dbname" ],
+ qr(contains a newline or carriage return character),
+ 'fails if database name containing newline character in name'
+);
+
+$node->command_fails_like(
+ [ 'createdb', "invalid \r dbname" ],
+ qr(contains a newline or carriage return character),,
+ 'fails if database name containing carriage return character in name'
+);
+
# Check use of templates with shared dependencies copied from the template.
my ($ret, $stdout, $stderr) = $node->psql(
'foobar2',
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 89ce396ed4f..38fffbd036b 100644
--- a/src/fe_utils/string_utils.c
+++ b/src/fe_utils/string_utils.c
@@ -568,12 +568,6 @@ appendByteaLiteral(PQExpBuffer buf, const unsigned char *str, size_t length,
* Append the given string to the shell command being built in the buffer,
* with shell-style quoting as needed to create exactly one argument.
*
- * Forbid LF or CR characters, which have scant practical use beyond designing
- * security breaches. The Windows command shell is unusable as a conduit for
- * arguments containing LF or CR characters. A future major release should
- * reject those characters in CREATE ROLE and CREATE DATABASE, because use
- * there eventually leads to errors here.
- *
* appendShellString() simply prints an error and dies if LF or CR appears.
* appendShellStringNoError() omits those characters from the result, and
* returns false if there were any.
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 8f4a2d9bbc1..bd930b8905d 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -162,5 +162,6 @@ extern TypeName *defGetTypeName(DefElem *def);
extern int defGetTypeLength(DefElem *def);
extern List *defGetStringList(DefElem *def);
pg_noreturn extern void errorConflictingDefElem(DefElem *defel, ParseState *pstate);
+extern void reject_newline_in_name(const char *objname, const char *objtype);
#endif /* DEFREM_H */
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index 61396b2a805..71c1d935091 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -102,6 +102,10 @@ DETAIL: Role names starting with "pg_" are reserved.
CREATE ROLE "pg_abcdef"; -- error
ERROR: role name "pg_abcdef" is reserved
DETAIL: Role names starting with "pg_" are reserved.
+CREATE ROLE "invalid
+rolename"; -- error
+ERROR: "role" name "invalid
+rolename" contains a newline or carriage return character
CREATE ROLE regress_testrol0 SUPERUSER LOGIN;
CREATE ROLE regress_testrolx SUPERUSER LOGIN;
CREATE ROLE regress_testrol2 SUPERUSER;
diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql
index adac36536db..70b342abeb6 100644
--- a/src/test/modules/unsafe_tests/sql/rolenames.sql
+++ b/src/test/modules/unsafe_tests/sql/rolenames.sql
@@ -75,6 +75,8 @@ CREATE ROLE pg_abc; -- error
CREATE ROLE "pg_abc"; -- error
CREATE ROLE pg_abcdef; -- error
CREATE ROLE "pg_abcdef"; -- error
+CREATE ROLE "invalid
+rolename"; -- error
CREATE ROLE regress_testrol0 SUPERUSER LOGIN;
CREATE ROLE regress_testrolx SUPERUSER LOGIN;
--
2.52.0
[application/octet-stream] v08_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch (3.7K, ../CAKYtNAr0TcS4NWgduwdqTcApTs_RBaiRY+C+WAay3-xXjkDC_w@mail.gmail.com/3-v08_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch)
download | inline diff:
From ccd70705f6468b9e726ab34b415442c15707b02c Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Mon, 2 Feb 2026 11:00:29 +0530
Subject: [PATCH 2/2] add handling to pg_upgrade to report alert for invalid
database, user and role names.
If database/role/user name has any newline or carraige return character
in name, then pg_upgrade will report ALERT for these.
---
---
src/bin/pg_upgrade/check.c | 78 ++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index a8d20a92a98..ecfe45a1b67 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -34,6 +34,7 @@ static void check_new_cluster_replication_slots(void);
static void check_new_cluster_subscription_configuration(void);
static void check_old_cluster_for_valid_slots(void);
static void check_old_cluster_subscription_state(void);
+static void check_database_role_names_in_old_cluster(ClusterInfo *cluster);
/*
* DataTypesUsageChecks - definitions of data type checks for the old cluster
@@ -600,6 +601,14 @@ check_and_dump_old_cluster(void)
*/
check_for_connection_status(&old_cluster);
+ /*
+ * Validate database, user and role names from old cluser. No need to
+ * check in 19 or newver version as newline and carriage return are not
+ * allowed at the creation time of object.
+ */
+ if (GET_MAJOR_VERSION(old_cluster.major_version) < 1900)
+ check_database_role_names_in_old_cluster(&old_cluster);
+
/*
* Extract a list of databases, tables, and logical replication slots from
* the old cluster.
@@ -2500,3 +2509,72 @@ check_old_cluster_subscription_state(void)
else
check_ok();
}
+
+/*
+ * check_database_role_names_in_old_cluster()
+ *
+ * If any database, user or role name has newline or carriage return character
+ * in name, then this will report those as these special characters are not
+ * allowed in these names from v19.
+ */
+static void
+check_database_role_names_in_old_cluster(ClusterInfo *cluster)
+{
+ int i;
+ PGconn *conn_template1;
+ PGresult *res;
+ int ntups;
+ FILE *script = NULL;
+ char output_path[MAXPGPATH];
+ int count = 0;
+
+ prep_status("Checking names of databases and roles");
+
+ snprintf(output_path, sizeof(output_path), "%s/%s",
+ log_opts.basedir,
+ "db_role_invalid_names.txt");
+
+ conn_template1 = connectToServer(cluster, "template1");
+
+ /*
+ * Get database, user/role names from cluster. Can't use
+ * pg_authid because only superusers can view it.
+ */
+ res = executeQueryOrDie(conn_template1,
+ "SELECT datname AS objname, 'database' AS objtype "
+ "FROM pg_catalog.pg_database UNION ALL "
+ "SELECT rolname AS objname, 'role' AS objtype "
+ "FROM pg_catalog.pg_roles ORDER BY 2 ");
+
+ ntups = PQntuples(res);
+ for (i = 0; i < ntups; i++)
+ {
+ char *objname = PQgetvalue(res, i, 0);
+ char *objtype = PQgetvalue(res, i, 1);
+
+ /* If name has \n or \r, then report it. */
+ if (strpbrk(objname, "\n\r"))
+ {
+ if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+ pg_fatal("could not open file \"%s\": %m", output_path);
+
+ fprintf(script, "%d : %s name = \"%s\"\n", ++count, objtype, objname);
+ }
+ }
+
+ PQclear(res);
+ PQfinish(conn_template1);
+
+ if (script)
+ {
+ fclose(script);
+ pg_log(PG_REPORT, "fatal");
+ pg_fatal("All the database and role names should have only valid characters. A newline or \n"
+ "carriage return character is not allowed in these object names. To fix this, please \n"
+ "rename these names with valid names. \n"
+ "To see all %d invalid object names, refer db_role_invalid_names.txt file. \n"
+ " %s", count, output_path);
+ }
+ else
+ check_ok();
+}
--
2.52.0
view thread (10+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
In-Reply-To: <CAKYtNAr0TcS4NWgduwdqTcApTs_RBaiRY+C+WAay3-xXjkDC_w@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox