public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Jelte Fennema <[email protected]>
Cc: Jelte Fennema <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Subject: Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
Date: Mon, 16 Jan 2023 14:22:27 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAGECzQQ_h_-kUg_xy=mm8b6wQ=TSMV=-5h37T32=qODxi+GOMA@mail.gmail.com>
References: <DBBPR83MB0507FEC2E8965012990A80D0F7FC9@DBBPR83MB0507.EURPRD83.prod.outlook.com>
<[email protected]>
<CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com>
<[email protected]>
<CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com>
<[email protected]>
<CAGECzQQ_h_-kUg_xy=mm8b6wQ=TSMV=-5h37T32=qODxi+GOMA@mail.gmail.com>
On Fri, Jan 13, 2023 at 09:19:10AM +0100, Jelte Fennema wrote:
>> Even if folks applying quotes
>> would not be able anymore to replace the pattern, the risk seems a bit
>> remote?
>
> Yeah I agree the risk is remote. To be clear, the main pattern I'm
> worried about breaking is simply "\1". Where people had put
> quotes around \1 for no reason. All in all, I'm fine if 0003 gets
> merged, but I'd also be fine with it if it doesn't. Both the risk
> and the advantage seem fairly small.
Still, I am having a few second thoughts about 0003 after thinking
about it over the weekend. Except if I am missing something, there
are no issues with 0004 if we keep the current behavior of always
replacing \1 even if pg-user is quoted? I would certainly add a new
test case either way.
>> I don't see how much that's different from the recent discussion with
>> regexps added for databases and users to pg_hba.conf. And consistency
>> sounds pretty good to me here.
>
> It's not much different, except that here also all and + change their meaning
> (for pg_hba.conf those special cases already existed). Mainly I called it out
> because I realised this discussion was called out in that commit too.
>
>> Regexps can have commas
>
> That's a really good reason to allow quoted regexes indeed. Even for pg_ident
> entries, commas in unquoted regexes would cause the AuthToken parsing to fail.
>
> Is there anything you still want to see changed about any of the patches?
+ /*
+ * Mark the token as quoted, so it will only be compared literally
+ * and not for special meanings like, such as "all" and membership
+ * checks using the + prefix.
+ */
+ expanded_pg_user_token = make_auth_token(expanded_pg_user, true);
It is critical to quote this AuthToken after the replacement, indeed.
Or we are in big trouble.
- /* no substitution, so copy the match */
- expanded_pg_user = pstrdup(identLine->pg_user->string);
+ expanded_pg_user_token = identLine->pg_user;
Perhaps it would be simpler to use copy_auth_token() in this code path
and always free the resulting token?
In the code path where system-user is a regexp, could it be better
to skip the replacement of \1 in the new AuthToken if pg-user is
itself a regexp? The compiled regexp would be the same, but it could
be considered as a bit confusing, as it can be thought that the
compiled regexp of pg-user happened after the replacement?
No issues with 0002 after a second look, so applied to move on.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (13+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox