| From: | Mahendra Singh Thalor <mahi6run(at)gmail(dot)com> |
|---|---|
| To: | Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
| Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Srinath Reddy <srinath2133(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)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 |
| Date: | 2026-02-03 19:14:37 |
| Message-ID: | CAKYtNApHc3PMoU3ZkUYmEt_tqKAx+e+5n=Gzk6c=SwcYdqRsAw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Thanks Álvaro, Tom and Nathan for the review.
On Mon, 2 Feb 2026 at 16:46, Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
>
> > +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.
Fixed.
>
> 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
Fixed.
>
> # 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.
Fixed.
>
> --
> Á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)
Based on suggestions, I removed the new added function and changed the
error message and added a check for tablespace also.
Here, I am attaching updated patches.
--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com
| Attachment | Content-Type | Size |
|---|---|---|
| v09_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch | text/x-patch | 12.9 KB |
| v09_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch | text/x-patch | 3.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-02-03 19:16:09 | Re: Periodic authorization expiration checks using GoAway message |
| Previous Message | Heikki Linnakangas | 2026-02-03 19:08:21 | Re: [Patch] add new parameter to pg_replication_origin_session_setup |