On Wed, Sep 7, 2022 at 6:17 PM Michael Paquier <michael@paquier.xyz> wrote:
> >> + /* Initialize SystemUser now that MyClientConnectionInfo is restored. */
> >> + InitializeSystemUser(MyClientConnectionInfo.authn_id,
> >> + hba_authname(MyClientConnectionInfo.auth_method));
> >
> > It makes me a little nervous to call hba_authname(auth_method) without
> > checking to see that auth_method is actually valid (which is only true
> > if authn_id is not NULL).
>
> You have mentioned that a couple of months ago if I recall correctly,
> and we pass down an enum value.
Ah, sorry. Do you remember which thread?
I am probably misinterpreting you, but I don't see why auth_method's
being an enum helps. uaReject (and the "reject" string) is not a sane
value to be using in SYSTEM_USER, and the more call stacks away we get
from MyClientConnectionInfo, the easier it is to forget that that
value is junk. As long as the code doesn't get more complicated, I
suppose there's no real harm being done, but it'd be cleaner not to
access auth_method at all if authn_id is NULL. I won't die on that
hill, though.
> There is actually a second and much deeper issue
> here, in the shape of a collation problem.
Oh, none of that sounds fun. :/
--Jacob
