Tightening up allowed custom GUC names

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Tightening up allowed custom GUC names
Date: 2021-02-09 22:34:37
Message-ID: 951335.1612910077@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Over in [1] it was noted that the system behaves rather oddly if
you try to do ALTER USER/DATABASE SET with a custom GUC name
containing "=" or "-". I think we should just disallow such cases.
Relaxing the restriction is harder than it might seem:

* The convention for entries in pg_db_role_setting is just
"name=value" with no quoting rule, so GUC names containing "="
can't work. We could imagine installing some kind of quoting rule,
but that would break client-side code that looks at this catalog;
pg_dump, for one, does so. On balance it seems clearly not worth
changing that.

* The problem with using "-" is that we parse pg_db_role_setting
entries with ParseLongOption(), which converts "-" to "_" because
that's what makes sense to do in the context of command-line switches
such as "-c work-mem=42MB". We could imagine adjusting the code to
not do that in the pg_db_role_setting case, but you'd still be left
with a GUC that cannot be set via PGOPTIONS="-c custom.my-guc=42".
To avoid that potential confusion, it seems best to ban "-" as well
as "=".

Now granting that the best answer is just to forbid these cases,
there are still a couple of decisions about how extensive the
prohibition ought to be:

* We could forbid these characters only when you try to actually
put such a GUC into pg_db_role_setting, and otherwise allow them.
That seems like a weird nonorthogonal choice though, so I'd
rather just forbid them period.

* A case could be made for tightening things up a lot more, and not
allowing anything that doesn't look like an identifier. I'm not
pushing for that, as it seems more likely to break existing
applications than the narrow restriction proposed here. But I could
live with it if people prefer that way.

Anyway, attached is a proposed patch that implements the restriction
as stated. I'm inclined to propose this for HEAD only and not
worry about the issue in the back branches.

Thoughts?

regards, tom lane

[1] https://www.postgresql.org/message-id/flat/20210209144059.GA21360%40depesz.com

Attachment Content-Type Size
restrict-custom-guc-names.patch text/x-diff 12.9 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2021-02-09 23:01:55 Re: Tightening up allowed custom GUC names
Previous Message Peter Geoghegan 2021-02-09 22:14:06 64-bit XIDs in deleted nbtree pages