public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v6 07/10] is vs are plural
30+ messages / 7 participants
[nested] [flat]

* [PATCH v6 07/10] is vs are plural
@ 2019-04-04 23:57 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 30+ messages in thread

From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw)

Should backpatch to v12
---
 doc/src/sgml/ref/alter_table.sgml | 2 +-
 doc/src/sgml/sources.sgml         | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index b2eb7097a9..fa848e0bdf 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       from the parent table will be created in the partition, if they don't
       already exist.
       If any of the <literal>CHECK</literal> constraints of the table being
-      attached is marked <literal>NO INHERIT</literal>, the command will fail;
+      attached are marked <literal>NO INHERIT</literal>, the command will fail;
       such constraints must be recreated without the
       <literal>NO INHERIT</literal> clause.
      </para>
diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml
index 283c3e0357..12704c6fdf 100644
--- a/doc/src/sgml/sources.sgml
+++ b/doc/src/sgml/sources.sgml
@@ -466,8 +466,8 @@ Hint:       the addendum
     enough for error messages.  Detail and hint messages can be relegated to a
     verbose mode, or perhaps a pop-up error-details window.  Also, details and
     hints would normally be suppressed from the server log to save
-    space. Reference to implementation details is best avoided since users
-    aren't expected to know the details.
+    space. References to implementation details are best avoided since users
+    aren't expected to know them.
    </para>
 
   </simplesect>
-- 
2.17.0


--C+ts3FVlLX8+P6JN
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch"



^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
@ 2025-03-20 18:09 Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Mahendra Singh Thalor @ 2025-03-20 18:09 UTC (permalink / raw)
  To: Srinath Reddy <[email protected]>; +Cc: [email protected]

On Thu, 30 Jan 2025 at 16:47, Srinath Reddy <[email protected]> wrote:
>
>
>
> On Wed, Jan 29, 2025 at 9:55 PM Mahendra Singh Thalor <[email protected]>
wrote:
>>
>> Hi,
>> While doing some testing with pg_dumpall, I noticed one weird behaviour.
>>
>> While we create the database, we are allowing the database name with a
new line (if name is in double quote).
>> For example:
>>>
>>> postgres=# create database "dbstr1;
>>> dbstr 2";
>>> CREATE DATABASE
>>> postgres=#
>>
>> Here, the database name is in 2 lines.
>>
>> With the help of pg_dumpall, I tried to dump but I am getting an error
for the new line.
>>
>>> --
>>> -- Database "dbstr1;
>>> dbstr 2" dump
>>> --
>>>
>>> shell command argument contains a newline or carriage return: "
dbname='dbstr1;
>>> dbstr 2'"
>>
>>
>> After this message, we are stopping the dump.
>
>
> I have reproduced and verified the same.The reason is in runPgDump during
appendShellString for forming the pg_dump command , in
appendShellStringNoError we are considering the string as invalid if it has
'\n' and '\r'.
>
>>
>>
>> I think, if we are allowing new lines in the db name, then we should
dump it.
>

In another thread[1], we have some discussions regarding \n\r in dbname and
they think that we should fix it.

As per code,

> *
>  * 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.
>  */
> void
> appendShellString(PQExpBuffer buf, const char *str)
> {
>     if (!appendShellStringNoError(buf, str))
>     {
>         fprintf(stderr,
>                 _("shell command argument contains a newline or carriage
> return: \"%s\"\n"),
>                 str);
>         exit(EXIT_FAILURE);
>     }
> }
>

Here, we are mentioning that in future majar releases, we should reject
\n\r in *CREATE ROLE and CREATE DATABASE.*

Above comment was added in 2016.

> commit 142c24c23447f212e642a0ffac9af878b93f490d
> Author: Noah Misch <[email protected]>
> Date:   Mon Aug 8 10:07:46 2016 -0400
>
>     Reject, in pg_dumpall, names containing CR or LF.
>
>     These characters prematurely terminate Windows shell command
> processing,
>     causing the shell to execute a prefix of the intended command.  The
>     chief alternative to rejecting these characters was to bypass the
>     Windows shell with CreateProcess(), but the ability to use such names
>     has little value.  Back-patch to 9.1 (all supported versions).
>
>     This change formally revokes support for these characters in database
>     names and roles names.  Don't document this; the error message is
>     self-explanatory, and too few users would benefit.  A future major
>     release may forbid creation of databases and roles so named.  For now,
>     check only at known weak points in pg_dumpall.  Future commits will,
>     without notice, reject affected names from other frontend programs.
>
>     Also extend the restriction to pg_dumpall --dbname=CONNSTR arguments
> and
>     --file arguments.  Unlike the effects on role name arguments and
>     database names, this does not reflect a broad policy change.  A
>     migration to CreateProcess() could lift these two restrictions.
>
>     Reviewed by Peter Eisentraut.
>
>     Security: CVE-2016-5424
>

As per above comments, we can work on a patch which will reject \n\r in
roles and database names.

I will work on this.

[1] : names with \n\r in dbnames
<https://www.postgresql.org/message-id/4ef51faa-993f-46ea-9e68-7baf736c07b8%40dunslane.net;

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


^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
@ 2025-03-24 17:51 ` Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Mahendra Singh Thalor @ 2025-03-24 17:51 UTC (permalink / raw)
  To: Srinath Reddy <[email protected]>; +Cc: [email protected]

On Thu, 20 Mar 2025 at 23:39, Mahendra Singh Thalor <[email protected]>
wrote:
>
> On Thu, 30 Jan 2025 at 16:47, Srinath Reddy <[email protected]> wrote:
> >
> >
> >
> > On Wed, Jan 29, 2025 at 9:55 PM Mahendra Singh Thalor <
[email protected]> wrote:
> >>
> >> Hi,
> >> While doing some testing with pg_dumpall, I noticed one weird
behaviour.
> >>
> >> While we create the database, we are allowing the database name with a
new line (if name is in double quote).
> >> For example:
> >>>
> >>> postgres=# create database "dbstr1;
> >>> dbstr 2";
> >>> CREATE DATABASE
> >>> postgres=#
> >>
> >> Here, the database name is in 2 lines.
> >>
> >> With the help of pg_dumpall, I tried to dump but I am getting an error
for the new line.
> >>
> >>> --
> >>> -- Database "dbstr1;
> >>> dbstr 2" dump
> >>> --
> >>>
> >>> shell command argument contains a newline or carriage return: "
dbname='dbstr1;
> >>> dbstr 2'"
> >>
> >>
> >> After this message, we are stopping the dump.
> >
> >
> > I have reproduced and verified the same.The reason is in runPgDump
during appendShellString for forming the pg_dump command , in
appendShellStringNoError we are considering the string as invalid if it has
'\n' and '\r'.
> >
> >>
> >>
> >> I think, if we are allowing new lines in the db name, then we should
dump it.
> >
>
> In another thread[1], we have some discussions regarding \n\r in dbname
and they think that we should fix it.
>
> As per code,
>>
>> *
>>  * 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.
>>  */
>> void
>> appendShellString(PQExpBuffer buf, const char *str)
>> {
>>     if (!appendShellStringNoError(buf, str))
>>     {
>>         fprintf(stderr,
>>                 _("shell command argument contains a newline or carriage
return: \"%s\"\n"),
>>                 str);
>>         exit(EXIT_FAILURE);
>>     }
>> }
>
>
> Here, we are mentioning that in future majar releases, we should reject
\n\r in CREATE ROLE and CREATE DATABASE.
>
> Above comment was added in 2016.
>>
>> commit 142c24c23447f212e642a0ffac9af878b93f490d
>> Author: Noah Misch <[email protected]>
>> Date:   Mon Aug 8 10:07:46 2016 -0400
>>
>>     Reject, in pg_dumpall, names containing CR or LF.
>>
>>     These characters prematurely terminate Windows shell command
processing,
>>     causing the shell to execute a prefix of the intended command.  The
>>     chief alternative to rejecting these characters was to bypass the
>>     Windows shell with CreateProcess(), but the ability to use such names
>>     has little value.  Back-patch to 9.1 (all supported versions).
>>
>>     This change formally revokes support for these characters in database
>>     names and roles names.  Don't document this; the error message is
>>     self-explanatory, and too few users would benefit.  A future major
>>     release may forbid creation of databases and roles so named.  For
now,
>>     check only at known weak points in pg_dumpall.  Future commits will,
>>     without notice, reject affected names from other frontend programs.
>>
>>     Also extend the restriction to pg_dumpall --dbname=CONNSTR arguments
and
>>     --file arguments.  Unlike the effects on role name arguments and
>>     database names, this does not reflect a broad policy change.  A
>>     migration to CreateProcess() could lift these two restrictions.
>>
>>     Reviewed by Peter Eisentraut.
>>
>>     Security: CVE-2016-5424
>
>
> As per above comments, we can work on a patch which will reject \n\r in
roles and database names.
>
> I will work on this.
>
> [1] : names with \n\r in dbnames
>
> --

Hi,
I tried to do some improvements for database names that have \n or \r in
dbname.

*Solution 1*:
As per code comments in appendShellString function, we can block database
creation with \n or \r.
sol1_v01* patch is doing the same for database creation.

*Solution 2:*
While dumping the database, report WARNING if the database name has \n or
\r and skip dump for a particular database but dump all other databases by
pg_dumpall.
sol2_v01* patch is doing this.

*Solution 3:*
While dumping the database, report FATAL if the database name has \n or \r
and add a hint message in FALAL (rename particular database to dump without
\n\r char).
sol3_v01* is doing the same.

Please review attached patches and let me know feedback.

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


Attachments:

  [application/octet-stream] sol1_v01_block-database-name-with-newline-or-carriage-return.patch (2.1K, ../../CAKYtNArY3-HQ53sCDynnhGqjr9DBi07nKjbj00U0tnsZJBnyoQ@mail.gmail.com/3-sol1_v01_block-database-name-with-newline-or-carriage-return.patch)
  download | inline diff:
From a46017da1ad67054da537e815ef9c7ff40898875 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Fri, 21 Mar 2025 17:41:21 +0530
Subject: [PATCH] block database name with newline or carriage return in name

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.
---
 src/backend/commands/dbcommands.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 5fbbcdaabb1..b0c4e33b634 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -138,6 +138,7 @@ static void CreateDirAndVersionFile(char *dbpath, Oid dbid, Oid tsid,
 static void CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid,
 										Oid src_tsid, Oid dst_tsid);
 static void recovery_create_dbdir(char *path, bool only_tblspc);
+static bool is_name_contain_lfcr(char *name);
 
 /*
  * Create a new database using the WAL_LOG strategy.
@@ -741,6 +742,13 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
 	createdb_failure_params fparms;
 
+	/* Report error if dbname have newline or carriage return in name. */
+	if (is_name_contain_lfcr(dbname))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("database name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in database name"));
+
 	/* Extract options from the statement node tree */
 	foreach(option, stmt->options)
 	{
@@ -3443,3 +3451,22 @@ dbase_redo(XLogReaderState *record)
 	else
 		elog(PANIC, "dbase_redo: unknown op code %u", info);
 }
+
+/*
+ * is_name_contain_lfcr
+ *
+ * If dbame has \n or \r in the name, then will return true.
+ */
+static bool
+is_name_contain_lfcr(char *name)
+{
+	const char *p;
+
+	for (p = name; *p; p++)
+	{
+		if (*p == '\n' || *p == '\r')
+			return true;
+	}
+
+	return false;
+}
-- 
2.39.3



  [application/octet-stream] sol2_vo1_in-dump-report-WARNING-if-dbname-have-n-r-and-skip-dump.patch (1.7K, ../../CAKYtNArY3-HQ53sCDynnhGqjr9DBi07nKjbj00U0tnsZJBnyoQ@mail.gmail.com/4-sol2_vo1_in-dump-report-WARNING-if-dbname-have-n-r-and-skip-dump.patch)
  download | inline diff:
From efdca757b57cbd80518e0d7342c6635fe029d7e4 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Mon, 24 Mar 2025 22:52:47 +0530
Subject: [PATCH] in dump, report WARNING if dbname have \n\r and skip dump for
 particular database

---
 src/bin/pg_dump/pg_dumpall.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 2935cac2c46..83242261d8d 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -81,6 +81,7 @@ static void executeCommand(PGconn *conn, const char *query);
 static void expand_dbname_patterns(PGconn *conn, SimpleStringList *patterns,
 								   SimpleStringList *names);
 static void read_dumpall_filters(const char *filename, SimpleStringList *pattern);
+static bool is_name_contain_lfcr(char *name);
 
 static char pg_dump_bin[MAXPGPATH];
 static const char *progname;
@@ -1596,6 +1597,13 @@ dumpDatabases(PGconn *conn)
 		if (strcmp(dbname, "template0") == 0)
 			continue;
 
+		/* Report warning if database name have \n\r */
+		if (is_name_contain_lfcr(dbname))
+		{
+			pg_log_warning("database name has newline or carriage return so skiping dump for this database.");
+			continue;
+		}
+
 		/* Skip any explicitly excluded database */
 		if (simple_string_list_member(&database_exclude_names, dbname))
 		{
@@ -2068,3 +2076,22 @@ read_dumpall_filters(const char *filename, SimpleStringList *pattern)
 
 	filter_free(&fstate);
 }
+
+/*
+ * is_name_contain_lfcr
+ *
+ * If dbame has \n or \r in the name, then will return true.
+ */
+static bool
+is_name_contain_lfcr(char *name)
+{
+	const char *p;
+
+	for (p = name; *p; p++)
+	{
+		if (*p == '\n' || *p == '\r')
+			return true;
+	}
+
+	return false;
+}
-- 
2.39.3



  [application/octet-stream] sol3_v01_report-fatal-if-database-name-has-n-r.patch (1.8K, ../../CAKYtNArY3-HQ53sCDynnhGqjr9DBi07nKjbj00U0tnsZJBnyoQ@mail.gmail.com/5-sol3_v01_report-fatal-if-database-name-has-n-r.patch)
  download | inline diff:
From fd6bcc84d1684ad65676e0c38fcb5d0bef959b45 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Mon, 24 Mar 2025 23:05:47 +0530
Subject: [PATCH] report fatal, if database name has \n\r

if dbname has \n or \r, then report fatal and add hint to rename
db name.
---
 src/bin/pg_dump/pg_dumpall.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 2935cac2c46..559708b1950 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -81,6 +81,7 @@ static void executeCommand(PGconn *conn, const char *query);
 static void expand_dbname_patterns(PGconn *conn, SimpleStringList *patterns,
 								   SimpleStringList *names);
 static void read_dumpall_filters(const char *filename, SimpleStringList *pattern);
+static bool is_name_contain_lfcr(char *name);
 
 static char pg_dump_bin[MAXPGPATH];
 static const char *progname;
@@ -1596,6 +1597,13 @@ dumpDatabases(PGconn *conn)
 		if (strcmp(dbname, "template0") == 0)
 			continue;
 
+		/* Report fatal if database name have \n\r */
+		if (is_name_contain_lfcr(dbname))
+		{
+			pg_fatal("database name has newline or carriage return so stoping dump. To fix, rename dbname.");
+			continue;
+		}
+
 		/* Skip any explicitly excluded database */
 		if (simple_string_list_member(&database_exclude_names, dbname))
 		{
@@ -2068,3 +2076,22 @@ read_dumpall_filters(const char *filename, SimpleStringList *pattern)
 
 	filter_free(&fstate);
 }
+
+/*
+ * is_name_contain_lfcr
+ *
+ * If dbame has \n or \r in the name, then will return true.
+ */
+static bool
+is_name_contain_lfcr(char *name)
+{
+	const char *p;
+
+	for (p = name; *p; p++)
+	{
+		if (*p == '\n' || *p == '\r')
+			return true;
+	}
+
+	return false;
+}
-- 
2.39.3



^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
@ 2025-03-26 07:04   ` Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Srinath Reddy @ 2025-03-26 07:04 UTC (permalink / raw)
  To: Mahendra Singh Thalor <[email protected]>; +Cc: [email protected]

On Mon, Mar 24, 2025 at 11:21 PM Mahendra Singh Thalor <[email protected]>
wrote:

>
> > As per code,
> >>
> >> *
> >>  * 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.
> >>  */
> >> void
> >> appendShellString(PQExpBuffer buf, const char *str)
> >> {
> >>     if (!appendShellStringNoError(buf, str))
> >>     {
> >>         fprintf(stderr,
> >>                 _("shell command argument contains a newline or
> carriage return: \"%s\"\n"),
> >>                 str);
> >>         exit(EXIT_FAILURE);
> >>     }
> >> }
> >
> >
> > Here, we are mentioning that in future majar releases, we should reject
> \n\r in CREATE ROLE and CREATE DATABASE.
> >
> > Above comment was added in 2016.
> >>
> >> commit 142c24c23447f212e642a0ffac9af878b93f490d
> >> Author: Noah Misch <[email protected]>
> >> Date:   Mon Aug 8 10:07:46 2016 -0400
> >>
> >>     Reject, in pg_dumpall, names containing CR or LF.
> >>
> >>     These characters prematurely terminate Windows shell command
> processing,
> >>     causing the shell to execute a prefix of the intended command.  The
> >>     chief alternative to rejecting these characters was to bypass the
> >>     Windows shell with CreateProcess(), but the ability to use such
> names
> >>     has little value.  Back-patch to 9.1 (all supported versions).
> >>
> >>     This change formally revokes support for these characters in
> database
> >>     names and roles names.  Don't document this; the error message is
> >>     self-explanatory, and too few users would benefit.  A future major
> >>     release may forbid creation of databases and roles so named.  For
> now,
> >>     check only at known weak points in pg_dumpall.  Future commits will,
> >>     without notice, reject affected names from other frontend programs.
> >>
> >>     Also extend the restriction to pg_dumpall --dbname=CONNSTR
> arguments and
> >>     --file arguments.  Unlike the effects on role name arguments and
> >>     database names, this does not reflect a broad policy change.  A
> >>     migration to CreateProcess() could lift these two restrictions.
> >>
> >>     Reviewed by Peter Eisentraut.
> >>
> >>     Security: CVE-2016-5424
> >
> >
> > As per above comments, we can work on a patch which will reject \n\r in
> roles and database names.
> >
> > I will work on this.
> >
> > [1] : names with \n\r in dbnames
> >
> > --
>
> Hi,
> I tried to do some improvements for database names that have \n or \r in
> dbname.
>
> *Solution 1*:
> As per code comments in appendShellString function, we can block database
> creation with \n or \r.
> sol1_v01* patch is doing the same for database creation.
>
> *Solution 2:*
> While dumping the database, report WARNING if the database name has \n or
> \r and skip dump for a particular database but dump all other databases by
> pg_dumpall.
> sol2_v01* patch is doing this.
>
> *Solution 3:*
> While dumping the database, report FATAL if the database name has \n or \r
> and add a hint message in FALAL (rename particular database to dump without
> \n\r char).
> sol3_v01* is doing the same.
>
> Please review attached patches and let me know feedback.
>
>
Hi ,

I have reviewed all solutions but based on the commit message and comments,
it is clear that the goal is to *entirely forbid database names containing
carriage return (CR) or line feed (LF)* characters. *Solution 1 LGTM and
aligns with this approach* by enforcing the restriction at the time of
database creation, ensuring consistency throughout PostgreSQL. This
approach eliminates ambiguity and guarantees that such database names *cannot
be created or dumped with CR or LF.*

To validate this behavior, I have also implemented a *TAP test* for
Solution 1.
Thanks and regards,
Srinath Reddy Sadipiralla,
EnterpriseDB: http://www.enterprisedb.com


Attachments:

  [application/octet-stream] 0001-Add-TAP-test.patch (1.3K, ../../CAFC+b6oiZ9b80jgZnYgWuxeOSkSGRhkb-Opjn59kR171sHE=XA@mail.gmail.com/3-0001-Add-TAP-test.patch)
  download | inline diff:
From b39f281c26f626c17700bca18abaa828c68316ff Mon Sep 17 00:00:00 2001
From: Srinath Reddy Sadipiralla <[email protected]>
Date: Wed, 26 Mar 2025 10:24:54 +0530
Subject: [PATCH 1/1] Add TAP test

---
 src/bin/scripts/t/020_createdb.pl | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl
index a8293390ed..1878177039 100644
--- a/src/bin/scripts/t/020_createdb.pl
+++ b/src/bin/scripts/t/020_createdb.pl
@@ -241,6 +241,22 @@ $node->command_fails(
 	],
 	'fails for invalid locale provider');
 
+# Test creating a database with a newline (\n) in the name
+my $dbname_newline = "invalid\nDB";
+$node->command_fails_like(
+    [ 'createdb', $dbname_newline ],
+    qr/ERROR:  database name contains a newline or carriage return character/,
+    'fails for database name containing newline'
+);
+
+# Test creating a database with a carriage return (\r) in the name
+my $dbname_cr = "invalid\rDB";
+$node->command_fails_like(
+    [ 'createdb', $dbname_cr ],
+    qr/ERROR:  database name contains a newline or carriage return character/,
+    'fails for database name containing carriage return'
+);
+
 # Check use of templates with shared dependencies copied from the template.
 my ($ret, $stdout, $stderr) = $node->psql(
 	'foobar2',
-- 
2.43.0



^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
@ 2025-03-26 12:25     ` Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Andrew Dunstan @ 2025-03-26 12:25 UTC (permalink / raw)
  To: Srinath Reddy <[email protected]>; Mahendra Singh Thalor <[email protected]>; +Cc: [email protected]


On 2025-03-26 We 3:04 AM, Srinath Reddy wrote:
>
> I have reviewed all solutions but based on the commit message and 
> comments, it is clear that the goal is to *entirely forbid database 
> names containing carriage return (CR) or line feed (LF)* characters. 
> *Solution 1 LGTM and aligns with this approach* by enforcing the 
> restriction at the time of database creation, ensuring consistency 
> throughout PostgreSQL. This approach eliminates ambiguity and 
> guarantees that such database names *cannot be created or dumped with 
> CR or LF.*
>
> To validate this behavior, I have also implemented a *TAP test* for 
> Solution 1.
>

You can still create a database with these using "CREATE DATABASE" 
though. Shouldn't we should really be preventing that?


cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com


^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
@ 2025-03-26 12:36       ` Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Srinath Reddy @ 2025-03-26 12:36 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: [email protected]

./psql postgres

Hi,

On Wed, Mar 26, 2025 at 5:55 PM Andrew Dunstan <[email protected]> wrote:

> You can still create a database with these using "CREATE DATABASE" though.
> Shouldn't we should really be preventing that?
>

yes, solution 1 which i mentioned prevents these while we are using "CREATE
DATABASE".

/*
  * Create a new database using the WAL_LOG strategy.
@@ -741,6 +742,13 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
  CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
  createdb_failure_params fparms;

+ /* Report error if dbname have newline or carriage return in name. */
+ if (is_name_contain_lfcr(dbname))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+ errmsg("database name contains a newline or carriage return character"),
+ errhint("newline or carriage return character is not allowed in database
name"));
+

psql (18devel)
Type "help" for help.

postgres=# create database "test
postgres"# lines";
ERROR:  database name contains a newline or carriage return character
HINT:  newline or carriage return character is not allowed in database name


Thanks and regards,
Srinath Reddy Sadipiralla,
EnterpriseDB: http://www.enterprisedb.com

postgres=#\q


^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
@ 2025-03-26 12:52         ` Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Srinath Reddy @ 2025-03-26 12:52 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; Mahendra Singh Thalor <[email protected]>; +Cc: [email protected]

sorry for the noise ,previous response had my editor's formatting,just
resending without that formatting.

./psql postgres

Hi,

On Wed, Mar 26, 2025 at 5:55 PM Andrew Dunstan <[email protected]> wrote:

> You can still create a database with these using "CREATE DATABASE" though.
> Shouldn't we should really be preventing that?
>

yes, solution 1 which I mentioned prevents these while we are using "CREATE
DATABASE".

/*
  * Create a new database using the WAL_LOG strategy.
@@ -741,6 +742,13 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
  CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
  createdb_failure_params fparms;

+ /* Report error if dbname have newline or carriage return in name. */
+ if (is_name_contain_lfcr(dbname))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+ errmsg("database name contains a newline or carriage return character"),
+ errhint("newline or carriage return character is not allowed in database
name"));
+

psql (18devel)
Type "help" for help.

postgres=# create database "test
postgres"# lines";
ERROR:  database name contains a newline or carriage return character
HINT:  newline or carriage return character is not allowed in database name

Thanks and regards,
Srinath Reddy Sadipiralla,
EDB: https://www.enterprisedb.com

postgres=#\q

>


^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
@ 2025-03-27 10:46           ` Andrew Dunstan <[email protected]>
  2025-03-27 11:33             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  0 siblings, 2 replies; 30+ messages in thread

From: Andrew Dunstan @ 2025-03-27 10:46 UTC (permalink / raw)
  To: Srinath Reddy <[email protected]>; Mahendra Singh Thalor <[email protected]>; +Cc: [email protected]


On 2025-03-26 We 8:52 AM, Srinath Reddy wrote:
> sorry for the noise ,previous response had my editor's formatting,just 
> resending without that formatting.
>
> ./psql postgres
>
> Hi,
>
> On Wed, Mar 26, 2025 at 5:55 PM Andrew Dunstan <[email protected]> 
> wrote:
>
>     You can still create a database with these using "CREATE DATABASE"
>     though. Shouldn't we should really be preventing that?
>
>
> yes, solution 1 which I mentioned prevents these while we are 
> using "CREATE DATABASE".
>
> /*
>   * Create a new database using the WAL_LOG strategy.
> @@ -741,6 +742,13 @@ createdb(ParseState *pstate, const CreatedbStmt 
> *stmt)
>   CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
>   createdb_failure_params fparms;
>
> + /* Report error if dbname have newline or carriage return in name. */
> + if (is_name_contain_lfcr(dbname))
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
> + errmsg("database name contains a newline or carriage return character"),
> + errhint("newline or carriage return character is not allowed in 
> database name"));
> +
>
> psql (18devel)
> Type "help" for help.
>
> postgres=# create database "test
> postgres"# lines";
> ERROR:  database name contains a newline or carriage return character
> HINT:  newline or carriage return character is not allowed in database 
> name
>
>


Yes, sorry, I misread the thread. I think we should proceed with options 
1 and 3 i.e. prevent creation of new databases with a CR or LF, and have 
pgdumpall exit with a more useful error message.

Your invention of an is_name_contain_lfcr() function is unnecessary - we 
can just use the standard library function strpbrk() to look for a CR or LF.


cheers


andrew


--
Andrew Dunstan
EDB:https://www.enterprisedb.com


^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
@ 2025-03-27 11:33             ` Srinath Reddy <[email protected]>
  2025-03-27 12:03               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  1 sibling, 1 reply; 30+ messages in thread

From: Srinath Reddy @ 2025-03-27 11:33 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Mahendra Singh Thalor <[email protected]>; [email protected]

./psql postgres

On Thu, Mar 27, 2025 at 4:16 PM Andrew Dunstan <[email protected]> wrote:

> Yes, sorry, I misread the thread. I think we should proceed with options 1
> and 3 i.e. prevent creation of new databases with a CR or LF, and have
> pgdumpall exit with a more useful error message.
>
agreed.

> Your invention of an is_name_contain_lfcr() function is unnecessary - we
> can just use the standard library function strpbrk() to look for a CR or LF.
>
makes sense,but I have a dumb doubt why in appendShellStringNoError() it
still continues even after it found CR or LF? ,AFAIK The reasoning is this
function is designed to silently filter out \n and \r while still producing
a usable shell-safe argument. It informs the caller of the issue (false
return value) but does not abruptly stop execution,then the caller will
decide what to do but every place this function is called they are just
throwing the error.

if we could just break the loop right after we found \n or \r in
appendShellStringNoError() we can also use strpbrk() here and during
creation of new database as you suggested.

thoughts?

May the force be with you,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/

postgres=#\q


^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:33             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
@ 2025-03-27 12:03               ` Andrew Dunstan <[email protected]>
  0 siblings, 0 replies; 30+ messages in thread

From: Andrew Dunstan @ 2025-03-27 12:03 UTC (permalink / raw)
  To: Srinath Reddy <[email protected]>; +Cc: Mahendra Singh Thalor <[email protected]>; [email protected]


On 2025-03-27 Th 7:33 AM, Srinath Reddy wrote:
> ./psql postgres
>
> On Thu, Mar 27, 2025 at 4:16 PM Andrew Dunstan <[email protected]> 
> wrote:
>
>     Yes, sorry, I misread the thread. I think we should proceed with
>     options 1 and 3 i.e. prevent creation of new databases with a CR
>     or LF, and have pgdumpall exit with a more useful error message.
>
> agreed.
>
>     Your invention of an is_name_contain_lfcr() function is
>     unnecessary - we can just use the standard library function
>     strpbrk() to look for a CR or LF.
>
> makes sense,but I have a dumb doubt why in appendShellStringNoError() 
> it still continues even after it found CR or LF? ,AFAIK The reasoning 
> is this function is designed to silently filter out \n and \r while 
> still producing a usable shell-safe argument. It informs the caller of 
> the issue (false return value) but does not abruptly stop 
> execution,then the caller will decide what to do but every place this 
> function is called they are just throwing the error.
>
> if we could just break the loop right after we found \n or \r in 
> appendShellStringNoError() we can also use strpbrk() here and during 
> creation of new database as you suggested.
>
> thoughts?
>
>

I don't know. If you want to submit a patch cleaning it up go ahead. But 
right now I just want to get this original issue cleaned up to go along 
with the pg_dumpall improvements.


cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com


^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
@ 2025-03-27 11:57             ` Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  1 sibling, 1 reply; 30+ messages in thread

From: Mahendra Singh Thalor @ 2025-03-27 11:57 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Srinath Reddy <[email protected]>; [email protected]

On Thu, 27 Mar 2025 at 16:16, Andrew Dunstan <[email protected]> wrote:
>
>
> On 2025-03-26 We 8:52 AM, Srinath Reddy wrote:
>
> sorry for the noise ,previous response had my editor's formatting,just resending without that formatting.
>
> ./psql postgres
>
> Hi,
>
> On Wed, Mar 26, 2025 at 5:55 PM Andrew Dunstan <[email protected]> wrote:
>>
>> You can still create a database with these using "CREATE DATABASE" though. Shouldn't we should really be preventing that?
>
>
> yes, solution 1 which I mentioned prevents these while we are using "CREATE DATABASE".
>
> /*
>   * Create a new database using the WAL_LOG strategy.
> @@ -741,6 +742,13 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
>   CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
>   createdb_failure_params fparms;
>
> + /* Report error if dbname have newline or carriage return in name. */
> + if (is_name_contain_lfcr(dbname))
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
> + errmsg("database name contains a newline or carriage return character"),
> + errhint("newline or carriage return character is not allowed in database name"));
> +
>
> psql (18devel)
> Type "help" for help.
>
> postgres=# create database "test
> postgres"# lines";
> ERROR:  database name contains a newline or carriage return character
> HINT:  newline or carriage return character is not allowed in database name
>
>
>
>
> Yes, sorry, I misread the thread. I think we should proceed with options 1 and 3 i.e. prevent creation of new databases with a CR or LF, and have pgdumpall exit with a more useful error message.
>
> Your invention of an is_name_contain_lfcr() function is unnecessary - we can just use the standard library function strpbrk() to look for a CR or LF.
>
>
> cheers
>

Thanks Andrew and Srinath for feedback.

Yes, we should use the strpbrk function. Fixed.

Here, I am attaching an updated patch which has check in createdb and
RenameDatabase. For older versions, we can add more useful error
message (like: rename database as database has \n\r")

I will add some TAP tests and will make patches for older branches.

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


Attachments:

  [application/octet-stream] v02_block-database-name-with-newline-or-carriage-return.patch (1.9K, ../../CAKYtNAoOrGve4WmkaMgc5Ywo+X1rzifPM_dFjH_Ux2ze5fvdSA@mail.gmail.com/2-v02_block-database-name-with-newline-or-carriage-return.patch)
  download | inline diff:
From cad807ed60f2001c2725003690901fbc870dabd4 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Thu, 27 Mar 2025 17:20:36 +0530
Subject: [PATCH] block database name with newline or carriage return in name

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.

block these in RenameDatabase also.
---
 src/backend/commands/dbcommands.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 5fbbcdaabb1..c8145b0b6e6 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -741,6 +741,13 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
 	createdb_failure_params fparms;
 
+	/* Report error if dbname have newline or carriage return in name. */
+	if (strpbrk(dbname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("database name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in database name"));
+
 	/* Extract options from the statement node tree */
 	foreach(option, stmt->options)
 	{
@@ -1884,6 +1891,13 @@ RenameDatabase(const char *oldname, const char *newname)
 	int			npreparedxacts;
 	ObjectAddress address;
 
+	/* Report error if dbname have newline or carriage return in name. */
+	if (strpbrk(newname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("database new name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in database name"));
+
 	/*
 	 * Look up the target database's OID, and get exclusive lock on it. We
 	 * need this for the same reasons as DROP DATABASE.
-- 
2.39.3



^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
@ 2025-03-27 12:39               ` Andrew Dunstan <[email protected]>
  2025-03-27 12:55                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  0 siblings, 2 replies; 30+ messages in thread

From: Andrew Dunstan @ 2025-03-27 12:39 UTC (permalink / raw)
  To: Mahendra Singh Thalor <[email protected]>; +Cc: Srinath Reddy <[email protected]>; [email protected]


On 2025-03-27 Th 7:57 AM, Mahendra Singh Thalor wrote:
> On Thu, 27 Mar 2025 at 16:16, Andrew Dunstan <[email protected]> wrote:
>>
>> On 2025-03-26 We 8:52 AM, Srinath Reddy wrote:
>>
>> sorry for the noise ,previous response had my editor's formatting,just resending without that formatting.
>>
>> ./psql postgres
>>
>> Hi,
>>
>> On Wed, Mar 26, 2025 at 5:55 PM Andrew Dunstan <[email protected]> wrote:
>>> You can still create a database with these using "CREATE DATABASE" though. Shouldn't we should really be preventing that?
>>
>> yes, solution 1 which I mentioned prevents these while we are using "CREATE DATABASE".
>>
>> /*
>>    * Create a new database using the WAL_LOG strategy.
>> @@ -741,6 +742,13 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
>>    CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
>>    createdb_failure_params fparms;
>>
>> + /* Report error if dbname have newline or carriage return in name. */
>> + if (is_name_contain_lfcr(dbname))
>> + ereport(ERROR,
>> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
>> + errmsg("database name contains a newline or carriage return character"),
>> + errhint("newline or carriage return character is not allowed in database name"));
>> +
>>
>> psql (18devel)
>> Type "help" for help.
>>
>> postgres=# create database "test
>> postgres"# lines";
>> ERROR:  database name contains a newline or carriage return character
>> HINT:  newline or carriage return character is not allowed in database name
>>
>>
>>
>>
>> Yes, sorry, I misread the thread. I think we should proceed with options 1 and 3 i.e. prevent creation of new databases with a CR or LF, and have pgdumpall exit with a more useful error message.
>>
>> Your invention of an is_name_contain_lfcr() function is unnecessary - we can just use the standard library function strpbrk() to look for a CR or LF.
>>
>>
>> cheers
>>
> Thanks Andrew and Srinath for feedback.
>
> Yes, we should use the strpbrk function. Fixed.
>
> Here, I am attaching an updated patch which has check in createdb and
> RenameDatabase. For older versions, we can add more useful error
> message (like: rename database as database has \n\r")
>
> I will add some TAP tests and will make patches for older branches.
>


I don't think we can backpatch this. It's a behaviour change.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
@ 2025-03-27 12:55                 ` Mahendra Singh Thalor <[email protected]>
  1 sibling, 0 replies; 30+ messages in thread

From: Mahendra Singh Thalor @ 2025-03-27 12:55 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Srinath Reddy <[email protected]>; [email protected]

On Thu, 27 Mar 2025 at 18:10, Andrew Dunstan <[email protected]> wrote:
>
>
> On 2025-03-27 Th 7:57 AM, Mahendra Singh Thalor wrote:
> > On Thu, 27 Mar 2025 at 16:16, Andrew Dunstan <[email protected]>
wrote:
> >>
> >> On 2025-03-26 We 8:52 AM, Srinath Reddy wrote:
> >>
> >> sorry for the noise ,previous response had my editor's formatting,just
resending without that formatting.
> >>
> >> ./psql postgres
> >>
> >> Hi,
> >>
> >> On Wed, Mar 26, 2025 at 5:55 PM Andrew Dunstan <[email protected]>
wrote:
> >>> You can still create a database with these using "CREATE DATABASE"
though. Shouldn't we should really be preventing that?
> >>
> >> yes, solution 1 which I mentioned prevents these while we are using
"CREATE DATABASE".
> >>
> >> /*
> >>    * Create a new database using the WAL_LOG strategy.
> >> @@ -741,6 +742,13 @@ createdb(ParseState *pstate, const CreatedbStmt
*stmt)
> >>    CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
> >>    createdb_failure_params fparms;
> >>
> >> + /* Report error if dbname have newline or carriage return in name. */
> >> + if (is_name_contain_lfcr(dbname))
> >> + ereport(ERROR,
> >> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
> >> + errmsg("database name contains a newline or carriage return
character"),
> >> + errhint("newline or carriage return character is not allowed in
database name"));
> >> +
> >>
> >> psql (18devel)
> >> Type "help" for help.
> >>
> >> postgres=# create database "test
> >> postgres"# lines";
> >> ERROR:  database name contains a newline or carriage return character
> >> HINT:  newline or carriage return character is not allowed in database
name
> >>
> >>
> >>
> >>
> >> Yes, sorry, I misread the thread. I think we should proceed with
options 1 and 3 i.e. prevent creation of new databases with a CR or LF, and
have pgdumpall exit with a more useful error message.
> >>
> >> Your invention of an is_name_contain_lfcr() function is unnecessary -
we can just use the standard library function strpbrk() to look for a CR or
LF.
> >>
> >>
> >> cheers
> >>
> > Thanks Andrew and Srinath for feedback.
> >
> > Yes, we should use the strpbrk function. Fixed.
> >
> > Here, I am attaching an updated patch which has check in createdb and
> > RenameDatabase. For older versions, we can add more useful error
> > message (like: rename database as database has \n\r")
> >
> > I will add some TAP tests and will make patches for older branches.
> >
>
>
> I don't think we can backpatch this. It's a behaviour change.

We can add a good error message in pg_dumpall and pg_dump for all older
branches by checking dbname.

> [mst@localhost postgres]$ git diff
> diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
> index 2ea574b0f06..991dd4db2ca 100644
> --- a/src/bin/pg_dump/pg_dumpall.c
> +++ b/src/bin/pg_dump/pg_dumpall.c
> @@ -1611,6 +1611,10 @@ dumpDatabases(PGconn *conn)
>                 if (strcmp(dbname, "template0") == 0)
>                         continue;
>
> +               /* Report fatal if database name have \n\r */
> +               if (strpbrk(dbname, "\n\r"))
> +                       pg_fatal("database name has newline or carriage
> return so stoping dump. To fix, rename dbname.");
> +
>                 /* Skip any explicitly excluded database */
>                 if (simple_string_list_member(&database_exclude_names,
> dbname))
>                 {
> [mst@localhost postgres]$


This will change the error message in older versions. If it is OK to report
more readable errors in back branches, then we can do the above change.

Thoughts?

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


^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
@ 2025-03-27 13:03                 ` Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  1 sibling, 1 reply; 30+ messages in thread

From: Álvaro Herrera @ 2025-03-27 13:03 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Mahendra Singh Thalor <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On 2025-Mar-27, Andrew Dunstan wrote:

> I don't think we can backpatch this. It's a behaviour change.

I agree, we can't.

Also, if we're going to enforce this rule, then pg_upgrade --check needs
to alert users that they have a database name that's no longer valid.
That needs to be part of this patch as well.  We should also ensure that
these are the only problem characters, which IMO means it should add a
test for pg_dumpall that creates a database whose name has all possible
characters and ensures that it is dumpable.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"La virtud es el justo medio entre dos defectos" (Aristóteles)





^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
@ 2025-03-27 20:07                   ` Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Mahendra Singh Thalor @ 2025-03-27 20:07 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On Thu, 27 Mar 2025 at 18:33, Álvaro Herrera <[email protected]>
wrote:
>
> On 2025-Mar-27, Andrew Dunstan wrote:
>
> > I don't think we can backpatch this. It's a behaviour change.
>
> I agree, we can't.
>
> Also, if we're going to enforce this rule, then pg_upgrade --check needs
> to alert users that they have a database name that's no longer valid.
> That needs to be part of this patch as well.  We should also ensure that
> these are the only problem characters, which IMO means it should add a
> test for pg_dumpall that creates a database whose name has all possible
> characters and ensures that it is dumpable.
>
> --
> Álvaro Herrera        Breisgau, Deutschland  —
https://www.EnterpriseDB.com/
> "La virtud es el justo medio entre dos defectos" (Aristóteles)

Thanks Álvaro.

Yes, I also agree with you. "pg_upgrade --check" should alert users
regarding database names because pg_upgrade is failing if we have \n\r in
dbname(old cluster).

*pg_upgrade --check: This is passing but **pg_upgrade is failing.*
............
Checking for new cluster tablespace directories               ok
*Clusters are compatible*

*pg_upgrade*:
.........................
Creating dump of global objects                               ok
Creating dump of database schemas

*shell command argument contains a newline or carriage return:
"dbname='invaliddb'"*

As a part of this patch, we can teach *pg_upgrade* to alert users regarding
database names with invalid characters or we can keep old behavior as
upgrade is already failing without this patch also.

I will try to make a separate patch to teach "*pg_upgrade --check"* to
alert users regarding database/user/role with \n\r in the old cluster.

Here, I am attaching an updated patch for review.

This patch has changes for: CREATE DATABASE, CREATE ROLE, CREATE USER and
RENAME DATABASE/USER/ROLE and have some tests also. (EXCEPT RENAME test
case)

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


Attachments:

  [application/octet-stream] v03_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch (7.4K, ../../CAKYtNAoVKQL5rLD4P4hZXZSnThwO-j4q3Y1vTDHQGjzwC-kUJg@mail.gmail.com/3-v03_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch)
  download | inline diff:
From e7b269675c27a8cd2e785b8bd22a163e1dcaee95 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Fri, 28 Mar 2025 00:20:15 +0530
Subject: [PATCH] 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     | 14 ++++++++++++++
 src/backend/commands/user.c           | 14 ++++++++++++++
 src/bin/pg_dump/t/010_dump_connstr.pl | 14 --------------
 src/bin/scripts/t/020_createdb.pl     | 12 ++++++++++++
 src/bin/scripts/t/040_createuser.pl   |  4 ++++
 src/fe_utils/string_utils.c           |  6 ------
 6 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
 mode change 100644 => 100755 src/bin/scripts/t/040_createuser.pl

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 5fbbcdaabb1..c8145b0b6e6 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -741,6 +741,13 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
 	createdb_failure_params fparms;
 
+	/* Report error if dbname have newline or carriage return in name. */
+	if (strpbrk(dbname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("database name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in database name"));
+
 	/* Extract options from the statement node tree */
 	foreach(option, stmt->options)
 	{
@@ -1884,6 +1891,13 @@ RenameDatabase(const char *oldname, const char *newname)
 	int			npreparedxacts;
 	ObjectAddress address;
 
+	/* Report error if dbname have newline or carriage return in name. */
+	if (strpbrk(newname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("database new name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in database name"));
+
 	/*
 	 * 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/user.c b/src/backend/commands/user.c
index 8ae510c623b..6a103a22197 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -171,6 +171,13 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	DefElem    *dbypassRLS = NULL;
 	GrantRoleOptions popt;
 
+	/* Report error if role name has newline or carriage return in name. */
+	if (strpbrk(stmt->role, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("role name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in role name"));
+
 	/* The defaults can vary depending on the original statement type */
 	switch (stmt->stmt_type)
 	{
@@ -1347,6 +1354,13 @@ RenameRole(const char *oldname, const char *newname)
 	ObjectAddress address;
 	Form_pg_authid authform;
 
+	/* Report error if role name has newline or carriage return in name. */
+	if (strpbrk(newname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("role name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in role name"));
+
 	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 bde6096c60d..5054224fa12
--- 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 a8293390ede..4cc3ad0f5cd
--- 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/ERROR:  database name 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/ERROR:  database name 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/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl
old mode 100644
new mode 100755
index 54af43401bb..fd1b3f83899
--- a/src/bin/scripts/t/040_createuser.pl
+++ b/src/bin/scripts/t/040_createuser.pl
@@ -89,5 +89,9 @@ $node->command_fails(
 		'regress_user3'
 	],
 	'fails for too many non-options');
+$node->command_fails([ 'createuser', "invalid \n username" ],
+	'fails as newline is not allowed in role name');
+$node->command_fails([ 'createuser', "invalid \r username" ],
+	'fails as carraige return is not allowed in role name');
 
 done_testing();
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 130d1020d50..6f25b5286ed 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.
-- 
2.39.3



^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
@ 2025-03-28 11:38                     ` Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
  2025-03-29 18:33                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  0 siblings, 2 replies; 30+ messages in thread

From: Mahendra Singh Thalor @ 2025-03-28 11:38 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On Fri, 28 Mar 2025 at 01:37, Mahendra Singh Thalor <[email protected]>
wrote:
>
> On Thu, 27 Mar 2025 at 18:33, Álvaro Herrera <[email protected]>
wrote:
> >
> > On 2025-Mar-27, Andrew Dunstan wrote:
> >
> > > I don't think we can backpatch this. It's a behaviour change.
> >
> > I agree, we can't.
> >
> > Also, if we're going to enforce this rule, then pg_upgrade --check needs
> > to alert users that they have a database name that's no longer valid.
> > That needs to be part of this patch as well.  We should also ensure that
> > these are the only problem characters, which IMO means it should add a
> > test for pg_dumpall that creates a database whose name has all possible
> > characters and ensures that it is dumpable.
> >
> > --
> > Álvaro Herrera        Breisgau, Deutschland  —
https://www.EnterpriseDB.com/
> > "La virtud es el justo medio entre dos defectos" (Aristóteles)
>
> Thanks Álvaro.
>
> Yes, I also agree with you. "pg_upgrade --check" should alert users
regarding database names because pg_upgrade is failing if we have \n\r in
dbname(old cluster).
>
> pg_upgrade --check: This is passing but pg_upgrade is failing.
> ............
> Checking for new cluster tablespace directories               ok
> *Clusters are compatible*
>
> pg_upgrade:
> .........................
> Creating dump of global objects                               ok
> Creating dump of database schemas
> shell command argument contains a newline or carriage return:
"dbname='invalid
> db'"

In the attached patch, instead of the above error, we will get proper ALERT
for invalid names even with "pg_upgrade --check" also.

*Ex:*

> Performing Consistency Checks
> -----------------------------
> Checking cluster versions                                     ok
> Checking database connection settings                         ok
> Checking names of databases/users/roles                       fatal
>
>
>
>
>
> *All the database names should have only valid characters.  A newline or
> carriage return character is not allowed in database name.  To fix this,
> please rename database names with valid names.
> /home/mst/pg_all/head_pg/postgres/inst/bin/data/pg_upgrade_output.d/20250328T164926.680/db_role_invalid_names.txtFailure,
> exiting*
>

>
> As a part of this patch, we can teach pg_upgrade to alert users regarding
database names with invalid characters or we can keep old behavior as
upgrade is already failing without this patch also.
>
> I will try to make a separate patch to teach "pg_upgrade --check" to
alert users regarding database/user/role with \n\r in the old cluster.
>
> Here, I am attaching an updated patch for review.
>
> This patch has changes for: CREATE DATABASE, CREATE ROLE, CREATE USER and
RENAME DATABASE/USER/ROLE and have some tests also. (EXCEPT RENAME test
case)
>
> --
> Thanks and Regards
> Mahendra Singh Thalor
> EnterpriseDB: http://www.enterprisedb.com

Here, I am attaching updated patches for review.

v04_001* has the changes for CREATE DATABASE/ROLE/USER and
v04_002* has the changes into pg_upgrade to give ALERTS for invalid names.

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


Attachments:

  [application/octet-stream] v04_002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch (4.0K, ../../CAKYtNAqRJZMnt6WAZmpSkg3VMRGZtfz=zX3UZqJmkG3W=wE6fA@mail.gmail.com/3-v04_002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch)
  download | inline diff:
From 10c09f7c57c1e79ccaafe5db1d06ac72e85baac4 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Fri, 28 Mar 2025 16:51:33 +0530
Subject: [PATCH] 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 | 99 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 02d9146e5ed..39e74ad48bb 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -29,6 +29,7 @@ static void check_new_cluster_logical_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_user_role_names_in_old_cluser(ClusterInfo *cluster);
 
 /*
  * DataTypesUsageChecks - definitions of data type checks for the old cluster
@@ -596,6 +597,9 @@ check_and_dump_old_cluster(void)
 	 */
 	check_for_connection_status(&old_cluster);
 
+	/* Validate database, user and role names from old cluser. */
+	check_database_user_role_names_in_old_cluser(&old_cluster);
+
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
@@ -2084,3 +2088,98 @@ check_old_cluster_subscription_state(void)
 	else
 		check_ok();
 }
+
+/*
+ * check_database_user_role_names_in_old_cluser()
+ *
+ * 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 v18.
+ */
+static void
+check_database_user_role_names_in_old_cluser(ClusterInfo *cluster)
+{
+	int			i;
+	PGconn		*conn_template1;
+	PGresult	*res;
+	int			ntups;
+	FILE		*script = NULL;
+	char		output_path[MAXPGPATH];
+
+	prep_status("Checking names of databases/users/roles ");
+
+	snprintf(output_path, sizeof(output_path), "%s/%s",
+			log_opts.basedir,
+			"db_role_invalid_names.txt");
+
+	conn_template1 = connectToServer(cluster, "template1");
+
+	/* get database names */
+	res = executeQueryOrDie(conn_template1,
+			"SELECT datname "
+			"FROM pg_catalog.pg_database");
+
+	ntups = PQntuples(res);
+	for (i = 0; i < ntups; i++)
+	{
+		char	*datname = PQgetvalue(res, i, 0);
+
+		/* If dbname has \n or \r, then report it. */
+		if (strpbrk(datname, "\n\r"))
+		{
+			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+				pg_fatal("could not open file \"%s\": %m", output_path);
+
+			fprintf(script, "database name = %s\n", datname);
+		}
+	}
+
+	PQclear(res);
+
+	if (script)
+	{
+		PQfinish(conn_template1);
+		fclose(script);
+		pg_log(PG_REPORT, "fatal");
+		pg_fatal("All the database names should have only valid characters.  A newline or \n"
+				"carriage return character is not allowed in database name.  To fix this, \n"
+				"please rename database names with valid names. \n"
+				"    %s", output_path);
+	}
+
+	/* Now check for users and roles. */
+	/* Can't use pg_authid because only superusers can view it. */
+	res = executeQueryOrDie(conn_template1,
+			"SELECT rolname "
+			"FROM pg_catalog.pg_roles ");
+
+	ntups = PQntuples(res);
+	for (i = 0; i < ntups; i++)
+	{
+		char	*rolname = PQgetvalue(res, i, 0);
+
+		/* If rolname has \n or \r, then report it. */
+		if (strpbrk(rolname, "\n\r"))
+		{
+			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+				pg_fatal("could not open file \"%s\": %m", output_path);
+
+			fprintf(script, "role name = %s\n", rolname);
+		}
+	}
+
+	PQclear(res);
+	PQfinish(conn_template1);
+
+	if (script)
+	{
+		fclose(script);
+		pg_log(PG_REPORT, "fatal");
+		pg_fatal("All the role/user names should have only valid characters.  A newline or \n"
+				"carriage return characters is not allowed in role/user name.  To fix this, \n"
+				"please rename role/user names with valid names. \n"
+				"    %s", output_path);
+	}
+	else
+		check_ok();
+}
-- 
2.39.3



  [application/octet-stream] v04_001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch (7.4K, ../../CAKYtNAqRJZMnt6WAZmpSkg3VMRGZtfz=zX3UZqJmkG3W=wE6fA@mail.gmail.com/4-v04_001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch)
  download | inline diff:
From e7b269675c27a8cd2e785b8bd22a163e1dcaee95 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Fri, 28 Mar 2025 00:20:15 +0530
Subject: [PATCH] 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     | 14 ++++++++++++++
 src/backend/commands/user.c           | 14 ++++++++++++++
 src/bin/pg_dump/t/010_dump_connstr.pl | 14 --------------
 src/bin/scripts/t/020_createdb.pl     | 12 ++++++++++++
 src/bin/scripts/t/040_createuser.pl   |  4 ++++
 src/fe_utils/string_utils.c           |  6 ------
 6 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
 mode change 100644 => 100755 src/bin/scripts/t/040_createuser.pl

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 5fbbcdaabb1..c8145b0b6e6 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -741,6 +741,13 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
 	createdb_failure_params fparms;
 
+	/* Report error if dbname have newline or carriage return in name. */
+	if (strpbrk(dbname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("database name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in database name"));
+
 	/* Extract options from the statement node tree */
 	foreach(option, stmt->options)
 	{
@@ -1884,6 +1891,13 @@ RenameDatabase(const char *oldname, const char *newname)
 	int			npreparedxacts;
 	ObjectAddress address;
 
+	/* Report error if dbname have newline or carriage return in name. */
+	if (strpbrk(newname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("database new name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in database name"));
+
 	/*
 	 * 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/user.c b/src/backend/commands/user.c
index 8ae510c623b..6a103a22197 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -171,6 +171,13 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	DefElem    *dbypassRLS = NULL;
 	GrantRoleOptions popt;
 
+	/* Report error if role name has newline or carriage return in name. */
+	if (strpbrk(stmt->role, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("role name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in role name"));
+
 	/* The defaults can vary depending on the original statement type */
 	switch (stmt->stmt_type)
 	{
@@ -1347,6 +1354,13 @@ RenameRole(const char *oldname, const char *newname)
 	ObjectAddress address;
 	Form_pg_authid authform;
 
+	/* Report error if role name has newline or carriage return in name. */
+	if (strpbrk(newname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("role name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in role name"));
+
 	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 bde6096c60d..5054224fa12
--- 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 a8293390ede..4cc3ad0f5cd
--- 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/ERROR:  database name 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/ERROR:  database name 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/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl
old mode 100644
new mode 100755
index 54af43401bb..fd1b3f83899
--- a/src/bin/scripts/t/040_createuser.pl
+++ b/src/bin/scripts/t/040_createuser.pl
@@ -89,5 +89,9 @@ $node->command_fails(
 		'regress_user3'
 	],
 	'fails for too many non-options');
+$node->command_fails([ 'createuser', "invalid \n username" ],
+	'fails as newline is not allowed in role name');
+$node->command_fails([ 'createuser', "invalid \r username" ],
+	'fails as carraige return is not allowed in role name');
 
 done_testing();
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 130d1020d50..6f25b5286ed 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.
-- 
2.39.3



^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
@ 2025-03-28 14:43                       ` Nathan Bossart <[email protected]>
  2025-03-29 18:50                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-04-02 11:45                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-04-06 14:50                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
  2025-04-06 16:10                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-04-06 17:21                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
  1 sibling, 5 replies; 30+ messages in thread

From: Nathan Bossart @ 2025-03-28 14:43 UTC (permalink / raw)
  To: Mahendra Singh Thalor <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On Fri, Mar 28, 2025 at 05:08:26PM +0530, Mahendra Singh Thalor wrote:
> Here, I am attaching updated patches for review.
> 
> v04_001* has the changes for CREATE DATABASE/ROLE/USER and
> v04_002* has the changes into pg_upgrade to give ALERTS for invalid names.

In general, +1 for these changes.  Thanks for picking this up.

If these are intended for v18, we probably should have a committer
attached to it soon.  I'm not confident that I'll have time for it,
unfortunately.

+	/* Report error if dbname have newline or carriage return in name. */
+	if (strpbrk(dbname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("database name contains a newline or carriage return character"),
+				errhint("newline or carriage return character is not allowed in database name"));

I think it would be better to move this to a helper function instead of
duplicating this code in several places.

Taking a step back, are we sure that 1) this is the right place to do these
checks and 2) we shouldn't apply the same restrictions to all names?  I'm
wondering if it would be better to add these checks to the grammar instead
of trying to patch up all the various places they are used in the tree.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
@ 2025-03-29 18:50                         ` Srinath Reddy <[email protected]>
  4 siblings, 0 replies; 30+ messages in thread

From: Srinath Reddy @ 2025-03-29 18:50 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Mahendra Singh Thalor <[email protected]>; Álvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]

./psql postgres
postgres=# BEGIN;

Hi,

On Fri, Mar 28, 2025 at 8:13 PM Nathan Bossart <[email protected]>
wrote:

>
> +       /* Report error if dbname have newline or carriage return in name.
> */
> +       if (strpbrk(dbname, "\n\r"))
> +               ereport(ERROR,
> +                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
> +                               errmsg("database name contains a newline
> or carriage return character"),
> +                               errhint("newline or carriage return
> character is not allowed in database name"));
>
> I think it would be better to move this to a helper function instead of
> duplicating this code in several places.
>
agreed,we can do something like this

static void
validate_name(const char *name, const char *object_type)
{
    if (strpbrk(name, "\n\r"))
        ereport(ERROR,
                (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
                errmsg("%s name contains a newline or carriage return
character", object_type),
                errhint("Newline or carriage return character is not
allowed in %s name", object_type));
}

where object_type is database or role/user name
,is src/backend/commands/define.c best to define this function?


>
> Taking a step back, are we sure that 1) this is the right place to do these
> checks and 2) we shouldn't apply the same restrictions to all names?  I'm
> wondering if it would be better to add these checks to the grammar instead
> of trying to patch up all the various places they are used in the tree.
>
> hmm... need to think.

May the force be with you,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/

postgres=# COMMIT;
postgres=# \q


^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
@ 2025-04-02 11:45                         ` Mahendra Singh Thalor <[email protected]>
  2025-04-05 16:48                           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  4 siblings, 1 reply; 30+ messages in thread

From: Mahendra Singh Thalor @ 2025-04-02 11:45 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On Fri, 28 Mar 2025 at 20:13, Nathan Bossart <[email protected]>
wrote:
>
> On Fri, Mar 28, 2025 at 05:08:26PM +0530, Mahendra Singh Thalor wrote:
> > Here, I am attaching updated patches for review.
> >
> > v04_001* has the changes for CREATE DATABASE/ROLE/USER and
> > v04_002* has the changes into pg_upgrade to give ALERTS for invalid
names.
>
> In general, +1 for these changes.  Thanks for picking this up.

Thanks Nathan for quick feedback.

>
> If these are intended for v18, we probably should have a committer
> attached to it soon.  I'm not confident that I'll have time for it,
> unfortunately.

I think Andrew is planning this as a cleanup for "non-text pg_dumpall"
patch. Andrew, please if you have some bandwidth, then please let us know
your feedback for these patches.

>
> +       /* Report error if dbname have newline or carriage return in
name. */
> +       if (strpbrk(dbname, "\n\r"))
> +               ereport(ERROR,
> +
(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
> +                               errmsg("database name contains a newline
or carriage return character"),
> +                               errhint("newline or carriage return
character is not allowed in database name"));
>
> I think it would be better to move this to a helper function instead of
> duplicating this code in several places.

Agreed. Fixed this and as Srinath pointed out, I moved it inside
"src/backend/commands/define.c" file.

>
> Taking a step back, are we sure that 1) this is the right place to do
these
> checks and 2) we shouldn't apply the same restrictions to all names?  I'm
> wondering if it would be better to add these checks to the grammar instead
> of trying to patch up all the various places they are used in the tree.
>

As of now, we made this restriction to only "database/role/user" because
dump is not allowed with these special characters.
yes, we can add these checks in grammar also. (probably we should add
checks in grammar only). If others also feel same, I can try to add these
checks in grammar.

On Sun, 30 Mar 2025 at 00:03, Srinath Reddy <[email protected]> wrote:
>
> ./psql postgres
>
> Hi,
>
> On Fri, Mar 28, 2025 at 5:08 PM Mahendra Singh Thalor <[email protected]>
wrote:
>>
>>
>> v04_001* has the changes for CREATE DATABASE/ROLE/USER and
>> v04_002* has the changes into pg_upgrade to give ALERTS for invalid
names.
>
>
> i have reviewed these patches,in v04_002* i think it would be better if
log all the names like database ,roles/users names then throw error instead
of just logging database names into db_role_invalid_names and throwing
error,which helps the user to see at once all the invalid names and resolve
then again try pg_upgrade,so here i am attaching delta patch for the same
,except this the patches LGTM.
>
Thanks Srinath for the review.

In attached v05_0002* patch, instead of 2 exec commands, I merged into 1
and as per your comment, we will report all the invalid names at once and
additionally we are giving count of invalid names.

*Ex: **pg_upgrade --check:*

> Performing Consistency Checks
> -----------------------------
> Checking cluster versions                                     ok
> Checking database connection settings                         ok
> Checking names of databases/users/roles                       fatal
>
> All the database, role/user names should have only valid characters. A
> newline or
> carriage return character is not allowed in these object names.  To fix
> this, please
> rename these names with valid names.
> To see all 5 invalid object names, refer db_role_user_invalid_names.txt
> file.
>
> /home/mst/pg_all/head_pg/postgres/inst/bin/data/pg_upgrade_output.d/20250402T160610.664/db_role_user_invalid_names.txt
> Failure, exiting
>

 Here, I am attaching updated patches for review.

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


Attachments:

  [application/octet-stream] v05_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch (7.9K, ../../CAKYtNAqRwivAddvZPy79MQ9vqAjTjgJyFzZBcf25pwUqePC6gw@mail.gmail.com/3-v05_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch)
  download | inline diff:
From d45b78d2f3c54287816a474796200f3528f0085e Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Wed, 2 Apr 2025 17:13:30 +0530
Subject: [PATCH] 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         | 19 +++++++++++++++++++
 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/bin/scripts/t/040_createuser.pl   |  4 ++++
 src/fe_utils/string_utils.c           |  6 ------
 src/include/commands/defrem.h         |  1 +
 8 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
 mode change 100644 => 100755 src/bin/scripts/t/040_createuser.pl

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 5fbbcdaabb1..0f11a1607a6 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -741,6 +741,8 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
 	createdb_failure_params fparms;
 
+	check_lfcr_in_objname(dbname, "database");
+
 	/* Extract options from the statement node tree */
 	foreach(option, stmt->options)
 	{
@@ -1884,6 +1886,8 @@ RenameDatabase(const char *oldname, const char *newname)
 	int			npreparedxacts;
 	ObjectAddress address;
 
+	check_lfcr_in_objname(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 5e1b867e6f7..420bfd659b4 100644
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -375,3 +375,22 @@ errorConflictingDefElem(DefElem *defel, ParseState *pstate)
 			errmsg("conflicting or redundant options"),
 			parser_errposition(pstate, defel->location));
 }
+
+/*
+ * check_lfcr_in_objname
+ *
+ * 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
+check_lfcr_in_objname(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 contains a newline or carriage return character", objtype),
+				 errhint("a newline or a carriage return character is not allowed in %s name\n"
+					 "here, given %s name is : \"%s\"", objtype, objtype, objname));
+}
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 0d638e29d00..3be92f8932f 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;
 
+	check_lfcr_in_objname(stmt->role, "role");
+
 	/* The defaults can vary depending on the original statement type */
 	switch (stmt->stmt_type)
 	{
@@ -1347,6 +1349,8 @@ RenameRole(const char *oldname, const char *newname)
 	ObjectAddress address;
 	Form_pg_authid authform;
 
+	check_lfcr_in_objname(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 bde6096c60d..5054224fa12
--- 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 a8293390ede..4cc3ad0f5cd
--- 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/ERROR:  database name 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/ERROR:  database name 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/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl
old mode 100644
new mode 100755
index 54af43401bb..fd1b3f83899
--- a/src/bin/scripts/t/040_createuser.pl
+++ b/src/bin/scripts/t/040_createuser.pl
@@ -89,5 +89,9 @@ $node->command_fails(
 		'regress_user3'
 	],
 	'fails for too many non-options');
+$node->command_fails([ 'createuser', "invalid \n username" ],
+	'fails as newline is not allowed in role name');
+$node->command_fails([ 'createuser', "invalid \r username" ],
+	'fails as carraige return is not allowed in role name');
 
 done_testing();
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 130d1020d50..6f25b5286ed 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 dd22b5efdfd..d1abea500b0 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -161,5 +161,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 check_lfcr_in_objname(const char *objname, const char *objtype);
 
 #endif							/* DEFREM_H */
-- 
2.39.3



  [application/octet-stream] v05_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch (3.5K, ../../CAKYtNAqRwivAddvZPy79MQ9vqAjTjgJyFzZBcf25pwUqePC6gw@mail.gmail.com/4-v05_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch)
  download | inline diff:
From 5d8cd1bcddc98ed179ecf529c7dfbc64d858fa98 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Wed, 2 Apr 2025 16:07:31 +0530
Subject: [PATCH] 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 | 73 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 02d9146e5ed..7bb657dc130 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -29,6 +29,7 @@ static void check_new_cluster_logical_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_user_role_names_in_old_cluser(ClusterInfo *cluster);
 
 /*
  * DataTypesUsageChecks - definitions of data type checks for the old cluster
@@ -596,6 +597,9 @@ check_and_dump_old_cluster(void)
 	 */
 	check_for_connection_status(&old_cluster);
 
+	/* Validate database, user and role names from old cluser. */
+	check_database_user_role_names_in_old_cluser(&old_cluster);
+
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
@@ -2084,3 +2088,72 @@ check_old_cluster_subscription_state(void)
 	else
 		check_ok();
 }
+
+/*
+ * check_database_user_role_names_in_old_cluser()
+ *
+ * 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 v18.
+ */
+static void
+check_database_user_role_names_in_old_cluser(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/users/roles ");
+
+	snprintf(output_path, sizeof(output_path), "%s/%s",
+			log_opts.basedir,
+			"db_role_user_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, role/user 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_user_invalid_names.txt file. \n"
+				"    %s", count, output_path);
+	}
+	else
+		check_ok();
+}
-- 
2.39.3



^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
  2025-04-02 11:45                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
@ 2025-04-05 16:48                           ` Mahendra Singh Thalor <[email protected]>
  0 siblings, 0 replies; 30+ messages in thread

From: Mahendra Singh Thalor @ 2025-04-05 16:48 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Álvaro Herrera <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

Thanks Andrew for the review.

On Sat, 5 Apr 2025 at 20:12, Andrew Dunstan <[email protected]> wrote:
>
>
> On 2025-04-02 We 7:45 AM, Mahendra Singh Thalor wrote:
>
> On Fri, 28 Mar 2025 at 20:13, Nathan Bossart <[email protected]> wrote:
> >
> > On Fri, Mar 28, 2025 at 05:08:26PM +0530, Mahendra Singh Thalor wrote:
> > > Here, I am attaching updated patches for review.
> > >
> > > v04_001* has the changes for CREATE DATABASE/ROLE/USER and
> > > v04_002* has the changes into pg_upgrade to give ALERTS for invalid names.
> >
> > In general, +1 for these changes.  Thanks for picking this up.
>
> Thanks Nathan for quick feedback.
>
> >
> > If these are intended for v18, we probably should have a committer
> > attached to it soon.  I'm not confident that I'll have time for it,
> > unfortunately.
>
> I think Andrew is planning this as a cleanup for "non-text pg_dumpall" patch. Andrew, please if you have some bandwidth, then please let us know your feedback for these patches.
>
> >
> > +       /* Report error if dbname have newline or carriage return in name. */
> > +       if (strpbrk(dbname, "\n\r"))
> > +               ereport(ERROR,
> > +                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
> > +                               errmsg("database name contains a newline or carriage return character"),
> > +                               errhint("newline or carriage return character is not allowed in database name"));
> >
> > I think it would be better to move this to a helper function instead of
> > duplicating this code in several places.
>
> Agreed. Fixed this and as Srinath pointed out, I moved it inside "src/backend/commands/define.c" file.
>
> >
> > Taking a step back, are we sure that 1) this is the right place to do these
> > checks and 2) we shouldn't apply the same restrictions to all names?  I'm
> > wondering if it would be better to add these checks to the grammar instead
> > of trying to patch up all the various places they are used in the tree.
> >
>
> As of now, we made this restriction to only "database/role/user" because dump is not allowed with these special characters.
> yes, we can add these checks in grammar also. (probably we should add checks in grammar only). If others also feel same, I can try to add these checks in grammar.
>
> On Sun, 30 Mar 2025 at 00:03, Srinath Reddy <[email protected]> wrote:
> >
> > ./psql postgres
> >
> > Hi,
> >
> > On Fri, Mar 28, 2025 at 5:08 PM Mahendra Singh Thalor <[email protected]> wrote:
> >>
> >>
> >> v04_001* has the changes for CREATE DATABASE/ROLE/USER and
> >> v04_002* has the changes into pg_upgrade to give ALERTS for invalid names.
> >
> >
> > i have reviewed these patches,in v04_002* i think it would be better if log all the names like database ,roles/users names then throw error instead of just logging database names into db_role_invalid_names and throwing error,which helps the user to see at once all the invalid names and resolve then again try pg_upgrade,so here i am attaching delta patch for the same ,except this the patches LGTM.
> >
> Thanks Srinath for the review.
>
> In attached v05_0002* patch, instead of 2 exec commands, I merged into 1 and as per your comment, we will report all the invalid names at once and additionally we are giving count of invalid names.
>
> Ex: pg_upgrade --check:
>>
>> Performing Consistency Checks
>> -----------------------------
>> Checking cluster versions                                     ok
>> Checking database connection settings                         ok
>> Checking names of databases/users/roles                       fatal
>>
>> All the database, role/user names should have only valid characters. A newline or
>> carriage return character is not allowed in these object names.  To fix this, please
>> rename these names with valid names.
>> To see all 5 invalid object names, refer db_role_user_invalid_names.txt file.
>>     /home/mst/pg_all/head_pg/postgres/inst/bin/data/pg_upgrade_output.d/20250402T160610.664/db_role_user_invalid_names.txt
>> Failure, exiting
>
>
>  Here, I am attaching updated patches for review.
>
>
>
> +void
> +check_lfcr_in_objname(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 contains a newline or carriage return character", objtype),
> +                errhint("a newline or a carriage return character is not allowed in %s name\n"
> +                    "here, given %s name is : \"%s\"", objtype, objtype, objname));
> +}
>
>
> I don't think the errhint here adds much. If we're going to put the offending name in an error message I think it possibly needs to be escaped so it's more obvious where the CR/LF are. This should be in an errdetail rather than an errhint.

Fixed. I replaced errhint with errdetail as "errdetail("invalid %s
name is  \"%s\"", objtype, objname));"

>
> +static void
> +check_database_user_role_names_in_old_cluser(ClusterInfo *cluster)
>
> s/cluser/cluster/
>
> +   prep_status("Checking names of databases/users/roles ");
>
> I would just say "Checking names of databases and roles".

Fixed.

>
>
> +       pg_fatal("All the database, role/user 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_user_invalid_names.txt file. \n"
> +               "    %s", count, output_path);
>
>
> Also needs some cleanup.
>
>
>
> cheers
>
>
> andrew
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com

Here, I am attaching updated patches for review.

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


Attachments:

  [application/octet-stream] v06_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch (7.9K, ../../CAKYtNAqC5pkjmh8UgvbNLtMyEVeKUtDF3_+9dvG9zb8YrhTJQQ@mail.gmail.com/2-v06_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch)
  download | inline diff:
From b6c857fa83ce0b3a2ec09d89f61b8260eb9fa458 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Sat, 5 Apr 2025 22:30:38 +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         | 18 ++++++++++++++++++
 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/bin/scripts/t/040_createuser.pl   |  4 ++++
 src/fe_utils/string_utils.c           |  6 ------
 src/include/commands/defrem.h         |  1 +
 8 files changed, 43 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
 mode change 100644 => 100755 src/bin/scripts/t/040_createuser.pl

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 5fbbcdaabb1..0f11a1607a6 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -741,6 +741,8 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
 	createdb_failure_params fparms;
 
+	check_lfcr_in_objname(dbname, "database");
+
 	/* Extract options from the statement node tree */
 	foreach(option, stmt->options)
 	{
@@ -1884,6 +1886,8 @@ RenameDatabase(const char *oldname, const char *newname)
 	int			npreparedxacts;
 	ObjectAddress address;
 
+	check_lfcr_in_objname(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 5e1b867e6f7..42c69e4599e 100644
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -375,3 +375,21 @@ errorConflictingDefElem(DefElem *defel, ParseState *pstate)
 			errmsg("conflicting or redundant options"),
 			parser_errposition(pstate, defel->location));
 }
+
+/*
+ * check_lfcr_in_objname
+ *
+ * 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
+check_lfcr_in_objname(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 contains a newline or carriage return character", objtype),
+				 errdetail("invalid %s name is  \"%s\"", objtype, objname));
+}
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 0d638e29d00..3be92f8932f 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;
 
+	check_lfcr_in_objname(stmt->role, "role");
+
 	/* The defaults can vary depending on the original statement type */
 	switch (stmt->stmt_type)
 	{
@@ -1347,6 +1349,8 @@ RenameRole(const char *oldname, const char *newname)
 	ObjectAddress address;
 	Form_pg_authid authform;
 
+	check_lfcr_in_objname(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 bde6096c60d..5054224fa12
--- 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 a8293390ede..4cc3ad0f5cd
--- 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/ERROR:  database name 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/ERROR:  database name 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/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl
old mode 100644
new mode 100755
index 54af43401bb..fd1b3f83899
--- a/src/bin/scripts/t/040_createuser.pl
+++ b/src/bin/scripts/t/040_createuser.pl
@@ -89,5 +89,9 @@ $node->command_fails(
 		'regress_user3'
 	],
 	'fails for too many non-options');
+$node->command_fails([ 'createuser', "invalid \n username" ],
+	'fails as newline is not allowed in role name');
+$node->command_fails([ 'createuser', "invalid \r username" ],
+	'fails as carraige return is not allowed in role name');
 
 done_testing();
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 130d1020d50..6f25b5286ed 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 dd22b5efdfd..d1abea500b0 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -161,5 +161,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 check_lfcr_in_objname(const char *objname, const char *objtype);
 
 #endif							/* DEFREM_H */
-- 
2.39.3



  [application/octet-stream] v06_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch (3.5K, ../../CAKYtNAqC5pkjmh8UgvbNLtMyEVeKUtDF3_+9dvG9zb8YrhTJQQ@mail.gmail.com/3-v06_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch)
  download | inline diff:
From d7a35da4b47e3d6ae779c9eb32fcd17813f230a5 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Sat, 5 Apr 2025 22:35:15 +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 | 73 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 8f946c4e3d6..db08fb86030 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -31,6 +31,7 @@ static void check_new_cluster_logical_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_cluser(ClusterInfo *cluster);
 
 /*
  * DataTypesUsageChecks - definitions of data type checks for the old cluster
@@ -598,6 +599,9 @@ check_and_dump_old_cluster(void)
 	 */
 	check_for_connection_status(&old_cluster);
 
+	/* Validate database, user and role names from old cluser. */
+	check_database_role_names_in_old_cluser(&old_cluster);
+
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
@@ -2268,3 +2272,72 @@ check_old_cluster_subscription_state(void)
 	else
 		check_ok();
 }
+
+/*
+ * check_database_role_names_in_old_cluser()
+ *
+ * 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 v18.
+ */
+static void
+check_database_role_names_in_old_cluser(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.39.3



^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
@ 2025-04-06 14:50                         ` Nathan Bossart <[email protected]>
  4 siblings, 0 replies; 30+ messages in thread

From: Nathan Bossart @ 2025-04-06 14:50 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Mahendra Singh Thalor <[email protected]>; Álvaro Herrera <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On Sun, Apr 06, 2025 at 10:24:58AM -0400, Andrew Dunstan wrote:
> On 2025-03-28 Fr 10:43 AM, Nathan Bossart wrote:
>> Taking a step back, are we sure that 1) this is the right place to do these
>> checks and 2) we shouldn't apply the same restrictions to all names?  I'm
>> wondering if it would be better to add these checks to the grammar instead
>> of trying to patch up all the various places they are used in the tree.
> 
> Maybe. I don't think there is time for that for v18, so we'd have to defer
> this for now. I can live with that - it's been like this for a long time.

+1, I think deferring is the right call.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
@ 2025-04-06 16:10                         ` Mahendra Singh Thalor <[email protected]>
  4 siblings, 0 replies; 30+ messages in thread

From: Mahendra Singh Thalor @ 2025-04-06 16:10 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Nathan Bossart <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On Sun, 6 Apr 2025 at 20:53, Álvaro Herrera <[email protected]> wrote:
>
> On 2025-Apr-06, Andrew Dunstan wrote:
>
> > On 2025-03-28 Fr 10:43 AM, Nathan Bossart wrote:
>
> > > Taking a step back, are we sure that 1) this is the right place to do
these
> > > checks and 2) we shouldn't apply the same restrictions to all names?
I'm
> > > wondering if it would be better to add these checks to the grammar
instead
> > > of trying to patch up all the various places they are used in the
tree.
> >
> > Maybe. I don't think there is time for that for v18, so we'd have to
defer
> > this for now. I can live with that - it's been like this for a long
time.
>
> Grumble.  I'd rather introduce a partial restriction now only for names
> that affect the tools failing outright (pg_dumpall in particular), than
> do nothing for this release.  If we feel the need to extend the
> restriction later, that's easy to do and bothers nobody.  We've wanted
> these characters to be forbidden on database names for a long time, but
> nobody has cared as much for their use on other types of names.  I'm not
> even sure we'd support the idea of forbidding them on all names (though
> the SQL standard doesn't allow control chars in identifiers.)
>
> Another point is that we can easily have pg_upgrade check for invalid
> database and role names, but checking *all* names would be more onerous.
>
> I don't like the present implementation though, on translability
> grounds.  I think the error message should appear at each callsite
> rather than be hardcoded in the new function, to avoid string building.
> I think it'd be cleaner if the new function (maybe "name_contains_crlf"
> or "is_identifier_awful") just returned a boolean based on strpbrk(),
> and the callsite throws the error.
>
> I wonder why does the patch restrict both database and role names.  Does
> a user with a newline also cause pg_upgrade to fail?  I mean, this
> thread started with a consideration for database names only, and the
> usage on role names seemed to have appeared out of nowhere in [1].

In my testing, pg_dumpall was not failing with roles/user (\n\r) in my
machine but due to the below comment in code, I restricted roles also.

+++ 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.



> Cheers
>
> [1]
https://postgr.es/m/CAKYtNAoVKQL5rLD4P4hZXZSnThwO-j4q3Y1vTDHQGjzwC-kUJg@mail.gmail.com
>
> --
> Álvaro Herrera         PostgreSQL Developer  —
https://www.EnterpriseDB.com/
> "Aprender sin pensar es inútil; pensar sin aprender, peligroso" (Confucio)

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


^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
@ 2025-04-06 17:21                         ` Tom Lane <[email protected]>
  2025-04-06 17:34                           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  4 siblings, 1 reply; 30+ messages in thread

From: Tom Lane @ 2025-04-06 17:21 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Nathan Bossart <[email protected]>; Mahendra Singh Thalor <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

=?utf-8?Q?=C3=81lvaro?= Herrera <[email protected]> writes:
> Grumble.  I'd rather introduce a partial restriction now only for names
> that affect the tools failing outright (pg_dumpall in particular), than
> do nothing for this release.  If we feel the need to extend the
> restriction later, that's easy to do and bothers nobody.  We've wanted
> these characters to be forbidden on database names for a long time, but
> nobody has cared as much for their use on other types of names.  I'm not
> even sure we'd support the idea of forbidding them on all names (though
> the SQL standard doesn't allow control chars in identifiers.)

I'd be 100% behind forbidding all ASCII control characters in all
identifiers.  I can't see any situation in which that's a good thing,
and I can think of plenty where it's a mistake (eg your editor
decided to change space to tab) or done with underhanded intent.
If we can cite the SQL standard then it's an entirely defensible
restriction.

Having said that, I'm not quite sure where we ought to implement
the restriction, and it's possible that there are multiple places
that would need to check.  I concur that the day before feature
freeze is not a good time to be designing this.  Let's defer.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
  2025-04-06 17:21                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
@ 2025-04-06 17:34                           ` Álvaro Herrera <[email protected]>
  2025-04-06 17:51                             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Álvaro Herrera @ 2025-04-06 17:34 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Nathan Bossart <[email protected]>; Mahendra Singh Thalor <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On 2025-Apr-06, Tom Lane wrote:

> I'd be 100% behind forbidding all ASCII control characters in all
> identifiers.  I can't see any situation in which that's a good thing,
> and I can think of plenty where it's a mistake (eg your editor
> decided to change space to tab) or done with underhanded intent.

Right.

> If we can cite the SQL standard then it's an entirely defensible
> restriction.

We can.  It says (in 5.2 <token> and <separator>)

<regular identifier> ::= <identifier body>
<identifier body> ::= <identifier start> [ <identifier part>... ]
<identifier part> ::= <identifier start> | <identifier extend>
<identifier start> ::= !! See the Syntax Rules.
<identifier extend> ::= !! See the Syntax Rules.

Syntax Rules
1) An <identifier start> is any character in the Unicode General Category
   classes “Lu”, “Ll”, “Lt”, “Lm”, “Lo”, or “Nl”.
      NOTE 112 — The Unicode General Category classes “Lu”, “Ll”, “Lt”, “Lm”,
      “Lo”, and “Nl” are assigned to Unicode characters that are, respectively,
      upper-case letters, lower-case letters, title-case letters, modifier
      letters, other letters, and letter numbers.
2) An <identifier extend> is U+00B7, “Middle Dot”, or any character in the
   Unicode General Category classes “Mn”, “Mc”, “Nd”, or “Pc”.
      NOTE 113 — The Unicode General Category classes “Mn”, “Mc”, “Nd”, and
      “Pc”, are assigned to Unicode characters that are, respectively,
      non-spacing marks, spacing combining marks, decimal numbers, and connector
      punctuations.

The class for control characters is "C", so there are allowed nowhere.

https://www.unicode.org/charts/script/

> Having said that, I'm not quite sure where we ought to implement
> the restriction, and it's possible that there are multiple places
> that would need to check.

Yeah, a general ban on control characters for all identifiers is harder
to implement than a restricted ban, because it probably involves the
lexer, and I'm not sure the resulting "syntax error" type of rejections
are going to be nice enough to users.  A C-function based rejection
seems more convenient at this stage.

> I concur that the day before feature freeze is not a good time to be
> designing this.  Let's defer.

Augh.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"In fact, the basic problem with Perl 5's subroutines is that they're not
crufty enough, so the cruft leaks out into user-defined code instead, by
the Conservation of Cruft Principle."  (Larry Wall, Apocalypse 6)






^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
  2025-04-06 17:21                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
  2025-04-06 17:34                           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
@ 2025-04-06 17:51                             ` Tom Lane <[email protected]>
  2025-04-06 21:10                               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Tom Lane @ 2025-04-06 17:51 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Nathan Bossart <[email protected]>; Mahendra Singh Thalor <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

=?utf-8?Q?=C3=81lvaro?= Herrera <[email protected]> writes:
> On 2025-Apr-06, Tom Lane wrote:
>> If we can cite the SQL standard then it's an entirely defensible
>> restriction.

> We can.  It says (in 5.2 <token> and <separator>)

> <regular identifier> ::= <identifier body>
> <identifier body> ::= <identifier start> [ <identifier part>... ]
> <identifier part> ::= <identifier start> | <identifier extend>
> <identifier start> ::= !! See the Syntax Rules.
> <identifier extend> ::= !! See the Syntax Rules.

Hmm, but that's about non-quoted identifiers, so of course their
character set is pretty restricted.  What's of concern here is
what's allowed in double-quoted identifiers.  AFAICS there's
not much restriction: it can be any <nondoublequote character>,
and SR 7 says

    7) A <nondoublequote character> is any character of the source
    language character set other than a <double quote>.

        NOTE 115 — “source language character set” is defined in
        Subclause 4.10.1, “Host languages”, in ISO/IEC 9075-1.

The referenced bit of 9075-1 is pretty vague too:

    No matter what binding style is chosen, SQL-statements are written
    in an implementation-defined character set, known as the source
    language character set. The source language character set is not
    required to be the same as the character set of any character
    string appearing in SQL-data.

So I'm not really seeing anything there that justifies forbidding any
characters.  However, I still think that if we're going to forbid CR
or LF, we might as well go the whole way and forbid all the ASCII
control characters; none of them are any saner to use in identifiers
than those two.  (I'd be for banning &nbsp; and similar as well, on
the same usability grounds as banning tabs, except that putting an
encoding dependency into this rule will not end well.)

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
  2025-04-06 17:21                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
  2025-04-06 17:34                           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-04-06 17:51                             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
@ 2025-04-06 21:10                               ` Andrew Dunstan <[email protected]>
  2025-04-10 18:28                                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Andrew Dunstan @ 2025-04-06 21:10 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Álvaro Herrera <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Mahendra Singh Thalor <[email protected]>; Srinath Reddy <[email protected]>; [email protected]


On 2025-04-06 Su 1:51 PM, Tom Lane wrote:
> =?utf-8?Q?=C3=81lvaro?= Herrera <[email protected]> writes:
>> On 2025-Apr-06, Tom Lane wrote:
>>> If we can cite the SQL standard then it's an entirely defensible
>>> restriction.
>> We can.  It says (in 5.2 <token> and <separator>)
>> <regular identifier> ::= <identifier body>
>> <identifier body> ::= <identifier start> [ <identifier part>... ]
>> <identifier part> ::= <identifier start> | <identifier extend>
>> <identifier start> ::= !! See the Syntax Rules.
>> <identifier extend> ::= !! See the Syntax Rules.
> Hmm, but that's about non-quoted identifiers, so of course their
> character set is pretty restricted.  What's of concern here is
> what's allowed in double-quoted identifiers.  AFAICS there's
> not much restriction: it can be any <nondoublequote character>,
> and SR 7 says
>
>      7) A <nondoublequote character> is any character of the source
>      language character set other than a <double quote>.
>
>          NOTE 115 — “source language character set” is defined in
>          Subclause 4.10.1, “Host languages”, in ISO/IEC 9075-1.
>
> The referenced bit of 9075-1 is pretty vague too:
>
>      No matter what binding style is chosen, SQL-statements are written
>      in an implementation-defined character set, known as the source
>      language character set. The source language character set is not
>      required to be the same as the character set of any character
>      string appearing in SQL-data.
>
> So I'm not really seeing anything there that justifies forbidding any
> characters.  However, I still think that if we're going to forbid CR
> or LF, we might as well go the whole way and forbid all the ASCII
> control characters; none of them are any saner to use in identifiers
> than those two.  (I'd be for banning &nbsp; and similar as well, on
> the same usability grounds as banning tabs, except that putting an
> encoding dependency into this rule will not end well.)


Sound like we have some work to do, and that's not going to happen in 24 
hours.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
  2025-04-06 17:21                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
  2025-04-06 17:34                           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-04-06 17:51                             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
  2025-04-06 21:10                               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
@ 2025-04-10 18:28                                 ` Mahendra Singh Thalor <[email protected]>
  2025-04-11 14:47                                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Mahendra Singh Thalor @ 2025-04-10 18:28 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Tom Lane <[email protected]>; Álvaro Herrera <[email protected]>; Nathan Bossart <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On Mon, 7 Apr 2025 at 02:40, Andrew Dunstan <[email protected]> wrote:
>
>
> On 2025-04-06 Su 1:51 PM, Tom Lane wrote:
> > =?utf-8?Q?=C3=81lvaro?= Herrera <[email protected]> writes:
> >> On 2025-Apr-06, Tom Lane wrote:
> >>> If we can cite the SQL standard then it's an entirely defensible
> >>> restriction.
> >> We can.  It says (in 5.2 <token> and <separator>)
> >> <regular identifier> ::= <identifier body>
> >> <identifier body> ::= <identifier start> [ <identifier part>... ]
> >> <identifier part> ::= <identifier start> | <identifier extend>
> >> <identifier start> ::= !! See the Syntax Rules.
> >> <identifier extend> ::= !! See the Syntax Rules.
> > Hmm, but that's about non-quoted identifiers, so of course their
> > character set is pretty restricted.  What's of concern here is
> > what's allowed in double-quoted identifiers.  AFAICS there's
> > not much restriction: it can be any <nondoublequote character>,
> > and SR 7 says
> >
> >      7) A <nondoublequote character> is any character of the source
> >      language character set other than a <double quote>.
> >
> >          NOTE 115 — “source language character set” is defined in
> >          Subclause 4.10.1, “Host languages”, in ISO/IEC 9075-1.
> >
> > The referenced bit of 9075-1 is pretty vague too:
> >
> >      No matter what binding style is chosen, SQL-statements are written
> >      in an implementation-defined character set, known as the source
> >      language character set. The source language character set is not
> >      required to be the same as the character set of any character
> >      string appearing in SQL-data.
> >
> > So I'm not really seeing anything there that justifies forbidding any
> > characters.  However, I still think that if we're going to forbid CR
> > or LF, we might as well go the whole way and forbid all the ASCII
> > control characters; none of them are any saner to use in identifiers
> > than those two.  (I'd be for banning &nbsp; and similar as well, on
> > the same usability grounds as banning tabs, except that putting an
> > encoding dependency into this rule will not end well.)
>
>
> Sound like we have some work to do, and that's not going to happen in 24
> hours.
>
>
> cheers
>
>
> andrew
>
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
>

Hi,
As per above discussions, for v18, we will not do any change to server
side to fix the issue of \n\r in database names. But as a cleanup
patch, we can give an alert to the user by "pg_upgrade --check". As
per current code, pg_dump and pg_upgrade will fail with "shell
command" error but in the attached patch, we will give some extra info
to the user by "pg_upgrade --check" so that they can fix database
names before trying to upgrade.

Here, I am attaching a patch which will give a list of invalid
database names in "pg_upgrade --check". We can consider this as a
cleanup patch.

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


Attachments:

  [application/octet-stream] 0001-pg_upgrade-add-alert-for-invalid-database-names.patch (3.1K, ../../CAKYtNArodApzf7OkF49VS4eO-u_Qwy0Pup3FMTw_QoLi5xeczQ@mail.gmail.com/2-0001-pg_upgrade-add-alert-for-invalid-database-names.patch)
  download | inline diff:
From 14db9fc7157b0b7d935dfd025a37d9a0839aa9c7 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Thu, 10 Apr 2025 23:47:45 +0530
Subject: [PATCH] pg_upgrade : add alert for invalid database names

When any database has \n or \r in database name, then dump will report
shell command error. so better, we will give alert to user to fix it.
---
 src/bin/pg_upgrade/check.c | 67 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 18c2d652bb6..be5db5ddae5 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -31,6 +31,7 @@ static void check_new_cluster_logical_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_names_in_old_cluser(ClusterInfo *cluster);
 
 /*
  * DataTypesUsageChecks - definitions of data type checks for the old cluster
@@ -598,6 +599,9 @@ check_and_dump_old_cluster(void)
 	 */
 	check_for_connection_status(&old_cluster);
 
+	/* Validate database names from old cluser. */
+	check_database_names_in_old_cluser(&old_cluster);
+
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
@@ -2269,3 +2273,66 @@ check_old_cluster_subscription_state(void)
 	else
 		check_ok();
 }
+
+/*
+ * check_database_names_in_old_cluser()
+ *
+ * If any database name has newline or carriage return character in name,
+ * then this will report those as these special characters are not allowed
+ * for database names in dump. See appendShellStringNoError
+ */
+static void
+check_database_names_in_old_cluser(ClusterInfo *cluster)
+{
+	int			i;
+	PGconn		*conn_template1;
+	PGresult	*res;
+	int			ntups;
+	FILE		*script = NULL;
+	char		output_path[MAXPGPATH];
+	int			count = 0;
+
+	prep_status("Checking database names");
+
+	snprintf(output_path, sizeof(output_path), "%s/%s",
+			log_opts.basedir,
+			"invalid_db_names.txt");
+
+	conn_template1 = connectToServer(cluster, "template1");
+
+	/* Get database names from cluster. */
+	res = executeQueryOrDie(conn_template1,
+			"SELECT datname "
+			"FROM pg_catalog.pg_database");
+
+	ntups = PQntuples(res);
+	for (i = 0; i < ntups; i++)
+	{
+		char	*dbname = PQgetvalue(res, i, 0);
+
+		/* If name has \n or \r, then report it. */
+		if (strpbrk(dbname, "\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 : database name = \"%s\"\n", ++count, dbname);
+		}
+	}
+
+	PQclear(res);
+	PQfinish(conn_template1);
+
+	if (script)
+	{
+		fclose(script);
+		pg_log(PG_REPORT, "fatal");
+		pg_fatal("All the database names should have only valid characters. A newline or a carriage\n"
+				"return character is not allowed in these database names.  To fix this, please rename\n"
+				"these names with valid names. \n"
+				"To see all %d invalid database names, refer invalid_db_names.txt file. \n"
+				"    %s", count, output_path);
+	}
+	else
+		check_ok();
+}
-- 
2.39.3



^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
  2025-04-06 17:21                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
  2025-04-06 17:34                           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-04-06 17:51                             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
  2025-04-06 21:10                               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-04-10 18:28                                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
@ 2025-04-11 14:47                                   ` Nathan Bossart <[email protected]>
  2025-06-24 06:07                                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  0 siblings, 1 reply; 30+ messages in thread

From: Nathan Bossart @ 2025-04-11 14:47 UTC (permalink / raw)
  To: Mahendra Singh Thalor <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; Álvaro Herrera <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On Thu, Apr 10, 2025 at 11:58:41PM +0530, Mahendra Singh Thalor wrote:
> As per above discussions, for v18, we will not do any change to server
> side to fix the issue of \n\r in database names. But as a cleanup
> patch, we can give an alert to the user by "pg_upgrade --check". As
> per current code, pg_dump and pg_upgrade will fail with "shell
> command" error but in the attached patch, we will give some extra info
> to the user by "pg_upgrade --check" so that they can fix database
> names before trying to upgrade.
> 
> Here, I am attaching a patch which will give a list of invalid
> database names in "pg_upgrade --check". We can consider this as a
> cleanup patch.

Are you proposing this for v18?  I think this is all v19 material at this
point.  Perhaps we could argue this is a bug fix that should be
back-patched, but IMHO that's a bit of a stretch.  I don't sense a
tremendous amount of urgency, either.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
  2025-04-06 17:21                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
  2025-04-06 17:34                           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-04-06 17:51                             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
  2025-04-06 21:10                               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-04-10 18:28                                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-04-11 14:47                                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
@ 2025-06-24 06:07                                     ` Mahendra Singh Thalor <[email protected]>
  0 siblings, 0 replies; 30+ messages in thread

From: Mahendra Singh Thalor @ 2025-06-24 06:07 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; Álvaro Herrera <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On Fri, 11 Apr 2025 at 20:17, Nathan Bossart <[email protected]>
wrote:
>
> On Thu, Apr 10, 2025 at 11:58:41PM +0530, Mahendra Singh Thalor wrote:
> > As per above discussions, for v18, we will not do any change to server
> > side to fix the issue of \n\r in database names. But as a cleanup
> > patch, we can give an alert to the user by "pg_upgrade --check". As
> > per current code, pg_dump and pg_upgrade will fail with "shell
> > command" error but in the attached patch, we will give some extra info
> > to the user by "pg_upgrade --check" so that they can fix database
> > names before trying to upgrade.
> >
> > Here, I am attaching a patch which will give a list of invalid
> > database names in "pg_upgrade --check". We can consider this as a
> > cleanup patch.
>
> Are you proposing this for v18?  I think this is all v19 material at this
> point.  Perhaps we could argue this is a bug fix that should be
> back-patched, but IMHO that's a bit of a stretch.  I don't sense a
> tremendous amount of urgency, either.
>
> --
> nathan

Thanks Nathan for the review.

I want to re-start this thread for v19. I posted v06* patches in my
previous updates[1]. Please someone review it and let me know feedback.

v06 patches: v06_patches
<https://www.postgresql.org/message-id/CAKYtNAqC5pkjmh8UgvbNLtMyEVeKUtDF3_%2B9dvG9zb8YrhTJQQ%40mail.g...;

[1] :
https://www.postgresql.org/message-id/CAKYtNAqC5pkjmh8UgvbNLtMyEVeKUtDF3_%2B9dvG9zb8YrhTJQQ%40mail.g...

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


^ permalink  raw  reply  [nested|flat] 30+ messages in thread

* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
  2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
  2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
  2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
  2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
  2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
@ 2025-03-29 18:33                       ` Srinath Reddy <[email protected]>
  1 sibling, 0 replies; 30+ messages in thread

From: Srinath Reddy @ 2025-03-29 18:33 UTC (permalink / raw)
  To: Mahendra Singh Thalor <[email protected]>; +Cc: [email protected]; Andrew Dunstan <[email protected]>; [email protected]; [email protected]

./psql postgres

Hi,

On Fri, Mar 28, 2025 at 5:08 PM Mahendra Singh Thalor <[email protected]>
wrote:

>
> v04_001* has the changes for CREATE DATABASE/ROLE/USER and
> v04_002* has the changes into pg_upgrade to give ALERTS for invalid names.
>

i have reviewed these patches,in v04_002* i think it would be better if log
all the names like database ,roles/users names then throw error instead of
just logging database names into db_role_invalid_names and throwing
error,which helps the user to see at once all the invalid names and resolve
then again try pg_upgrade,so here i am attaching delta patch for the same
,except this the patches LGTM.


May the force be with you,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/

postgres=# \q


Attachments:

  [application/octet-stream] v05_002-Log-about-all-the-invalid-database-role-user-names-t.patch (1.9K, ../../CAFC+b6rKQEx3AmOJFJA5hgS2cwYDSWoc+wLh_SPoZJdz5hsF6A@mail.gmail.com/3-v05_002-Log-about-all-the-invalid-database-role-user-names-t.patch)
  download | inline diff:
From fb7bd4514a18c4fa74f229ae3a28899a2c764127 Mon Sep 17 00:00:00 2001
From: Srinath Reddy Sadipiralla <[email protected]>
Date: Sat, 29 Mar 2025 23:51:31 +0530
Subject: [PATCH 1/1] Log about all the invalid database/role/user names then
 throw error during pg_upgrade --check

---
 src/bin/pg_upgrade/check.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 39e74ad48bb..167b1853c8e 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -2136,17 +2136,6 @@ check_database_user_role_names_in_old_cluser(ClusterInfo *cluster)
 
 	PQclear(res);
 
-	if (script)
-	{
-		PQfinish(conn_template1);
-		fclose(script);
-		pg_log(PG_REPORT, "fatal");
-		pg_fatal("All the database names should have only valid characters.  A newline or \n"
-				"carriage return character is not allowed in database name.  To fix this, \n"
-				"please rename database names with valid names. \n"
-				"    %s", output_path);
-	}
-
 	/* Now check for users and roles. */
 	/* Can't use pg_authid because only superusers can view it. */
 	res = executeQueryOrDie(conn_template1,
@@ -2175,10 +2164,10 @@ check_database_user_role_names_in_old_cluser(ClusterInfo *cluster)
 	{
 		fclose(script);
 		pg_log(PG_REPORT, "fatal");
-		pg_fatal("All the role/user names should have only valid characters.  A newline or \n"
-				"carriage return characters is not allowed in role/user name.  To fix this, \n"
-				"please rename role/user names with valid names. \n"
-				"    %s", output_path);
+		pg_fatal("All the database or role/user names should have only valid characters.  A newline or \n"
+				"carriage return characters is not allowed in database or role/user names.  To fix this, \n"
+				"please rename database or role/user names with valid names. \n"
+				"See details in: %s", output_path);
 	}
 	else
 		check_ok();
-- 
2.43.0



^ permalink  raw  reply  [nested|flat] 30+ messages in thread


end of thread, other threads:[~2025-06-24 06:07 UTC | newest]

Thread overview: 30+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]>
2025-03-20 18:09 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
2025-03-24 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
2025-03-26 07:04   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
2025-03-26 12:25     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
2025-03-26 12:36       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
2025-03-26 12:52         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
2025-03-27 10:46           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
2025-03-27 11:33             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
2025-03-27 12:03               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
2025-03-27 11:57             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
2025-03-27 12:39               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
2025-03-27 12:55                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
2025-03-27 13:03                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
2025-03-27 20:07                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
2025-03-28 11:38                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
2025-03-28 14:43                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
2025-03-29 18:50                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>
2025-04-02 11:45                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
2025-04-05 16:48                           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
2025-04-06 14:50                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
2025-04-06 16:10                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
2025-04-06 17:21                         ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
2025-04-06 17:34                           ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]>
2025-04-06 17:51                             ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]>
2025-04-06 21:10                               ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]>
2025-04-10 18:28                                 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
2025-04-11 14:47                                   ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]>
2025-06-24 06:07                                     ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]>
2025-03-29 18:33                       ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Srinath Reddy <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox