Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
> On 4/20/24 22:40, Tom Lane wrote:
>> Seems reasonable. The alternative could be to remove createdb.c's use
>> of fmtId() here, but I don't think that's actually better.
> Why? It seems to me this is quite close to e.g. LOCALE_PROVIDER, and we
> don't do fmtId() for that. So why should we do that for STRATEGY?
Hah, nothing like being randomly inconsistent with adjacent code.
Every other argument handled by createdb gets wrapped by either
fmtId or appendStringLiteralConn.
I think the argument for this is it ensures that the switch value as
accepted by createdb is the argument that CREATE DATABASE will see.
Compare
$ createdb --strategy="foo bar" mydb
createdb: error: database creation failed: ERROR: invalid create database strategy "foo bar"
HINT: Valid strategies are "wal_log", and "file_copy".
$ createdb --locale-provider="foo bar" mydb
createdb: error: database creation failed: ERROR: syntax error at or near ";"
LINE 1: CREATE DATABASE mydb LOCALE_PROVIDER foo bar;
^
I'm not suggesting that this is an interesting security vulnerability,
because if you can control the arguments to createdb it's probably
game over long since. But wrapping the arguments is good for
delivering on-point error messages. So I'd add a fmtId() call to
LOCALE_PROVIDER too.
BTW, someone's taken the Oxford comma too far in that HINT.
Nobody writes a comma when there are only two alternatives.
regards, tom lane
