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