Feedback-ID: ia2694551:Fastmail
Date: Mon, 2 Feb 2026 12:16:20 +0100
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@alvh.no-ip.org>
To: Mahendra Singh Thalor <mahi6run@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Andrew Dunstan <andrew@dunslane.net>, 
	Nathan Bossart <nathandbossart@gmail.com>, Srinath Reddy <srinath2133@gmail.com>, 
	pgsql-hackers@lists.postgresql.org
Subject: Re: getting "shell command argument contains a newline or carriage
 return:" error with pg_dumpall when db name have new line in double quote
Message-ID: <202602021101.lguk6kclmciv@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAKYtNAr0TcS4NWgduwdqTcApTs_RBaiRY+C+WAay3-xXjkDC_w@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/202602021101.lguk6kclmciv%40alvherre.pgsql>
Precedence: bulk


> +void
> +reject_newline_in_name(const char *objname, const char *objtype)
> +{
> +	/* Report error if name has \n or \r character. */
> +	if (strpbrk(objname, "\n\r"))
> +		ereport(ERROR,
> +				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
> +				errmsg("\"%s\" name \"%s\" contains a newline or carriage return character",objtype, objname));
> +}

I think this error message doesn't work very well.  Having the
"database" word be an untranslatable piece of the message makes no sense
to me.  I would rather have the ereport() in each place where this is
needed so that we can have a complete phrase to translate, and avoid
this wrinkle.  Alternatively you could pass the error message from the
caller (to abstract away the strpbrk() call) but I'm not sure that's
really all that useful.

BTW, looking at generate_db in src/bin/pg_upgrade/t/002_pg_upgrade.pl I
wonder why don't we reject BEL here also.  (Or actually, maybe not, but
I think commit 322becb6085c was wrong to lose the part of the comment
that explained the reason.)

> --- a/src/bin/scripts/t/020_createdb.pl
> +++ b/src/bin/scripts/t/020_createdb.pl
> @@ -241,6 +241,18 @@ $node->command_fails(
>  	],
>  	'fails for invalid locale provider');
>  
> +$node->command_fails_like(
> +    [ 'createdb', "invalid \n dbname" ],
> +    qr(contains a newline or carriage return character),
> +    'fails if database name containing newline character in name'
> +);
> +
> +$node->command_fails_like(
> +    [ 'createdb', "invalid \r dbname" ],
> +    qr(contains a newline or carriage return character),,
> +    'fails if database name containing carriage return character in name'
> +);

Note there are two commas the the qr() line in the second stanza.  Seems
to be innocuous, because the test log shows

# Running: createdb invalid 
 dbname
[11:57:00.942](0.012s) ok 34 - fails if database name containing newline character in name: exit code not 0
[11:57:00.942](0.000s) ok 35 - fails if database name containing newline character in name: matches
# Running: createdb invalid ^M dbname
[11:57:00.953](0.011s) ok 36 - fails if database name containing carriage return character in name: exit code not 0
[11:57:00.954](0.000s) ok 37 - fails if database name containing carriage return character in name: matches

but it'd look nicer without those commas.  Also, the "fails if ...
containing" test names sound ungrammatical to me.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Always assume the user will do much worse than the stupidest thing
you can imagine."                                (Julien PUYDT)