MIME-Version: 1.0
References: 
 <CAAZLFmRK+XFp=mqCeruyNVkqGq5mH45CP+e-8oNttPRtLuB5eQ@mail.gmail.com>
 <ee96443a-72f3-4a12-8ba7-326069fd1c14@eisentraut.org>
In-Reply-To: <ee96443a-72f3-4a12-8ba7-326069fd1c14@eisentraut.org>
From: Jianghua Yang <yjhjstz@gmail.com>
Date: Wed, 2 Jul 2025 08:03:05 -0700
Message-ID: 
 <CAAZLFmQ5Ckdb1WSp0wkVczk2rde1j-2PdrmwPNywG-XO5ph+aQ@mail.gmail.com>
Subject: Re: [PATCH] initdb: Treat empty -U argument as unset username
To: Peter Eisentraut <peter@eisentraut.org>
Cc: pgsql-hackers@lists.postgresql.org
Content-Type: multipart/alternative; boundary="000000000000081cb20638f3913b"
Archived-At: 
 <https://www.postgresql.org/message-id/CAAZLFmQ5Ckdb1WSp0wkVczk2rde1j-2PdrmwPNywG-XO5ph%2BaQ%40mail.gmail.com>
Precedence: bulk

--000000000000081cb20638f3913b
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Peter,

Thanks for your detailed analysis. I appreciate you digging deeper into the
root cause.

For this patch, I'd like to keep the changes to `initdb` minimal and
focused on rejecting empty usernames, as that seems to be the consensus
from the previous discussion.

I'll be happy to discuss the `getid()` and `aclitem` parsing behavior in a
separate thread.

Best regards,
Jianghua Yang


Peter Eisentraut <peter@eisentraut.org> =E4=BA=8E2025=E5=B9=B47=E6=9C=882=
=E6=97=A5=E5=91=A8=E4=B8=89 07:39=E5=86=99=E9=81=93=EF=BC=9A

> On 02.07.25 04:55, Jianghua Yang wrote:
> > While working with `initdb`, I noticed that passing an empty string to
> > the `-U` option (e.g., `initdb -U ''`) causes it to fail with a
> > misleading error:
> >
> > performing post-bootstrap initialization ... 2025-07-01 19:48:42.006 PD=
T
> > [14888] FATAL:role """ does not exist at character 72
> >
> > 2025-07-01 19:48:42.006 PDT [14888] STATEMENT:
> >
> > UPDATE pg_class SET relacl =3D (SELECT array_agg(a.acl) FROM(SELECT
> > E'=3Dr/""' as acl UNION SELECT unnest(pg_catalog.acldefault(CASE WHEN
> > relkind =3D 'S' THEN 's'ELSE 'r' END::"char",10::oid)) ) as a) WHERE
> > relkind IN ('r', 'v', 'm', 'S')AND relacl IS NULL;
> >
> > This happens because `initdb` accepts the empty string as a valid role
> > name and attempts to use it as the database superuser, which is not
> > intended and fails during bootstrap SQL.
>
> I'll start by saying, of course an empty user name isn't going to work,
> so we should reject it.
>
> But let's dig a little deeper into why it fails.  Observe the error:
>
>      FATAL:role """ does not exist at character 72
>
> It thinks that the role name is `"` (a sole double-quote, not empty!).
> Why is that?
>
> This error comes from the literal
>
>     E'=3Dr/""'
>
> interpreted as an aclitem value.  The aclitem parsing ends up in getid()
> in src/backend/utils/adt/acl.c, which thinks that an input string
> consisting entirely of "" is an escaped double quote.
>
> Maybe it's worth fixing that, and making putid() also print empty user
> names correspondingly.
>
> Alternatively, it's the fault of initdb that it constructs aclitem
> values that don't follow the aclitem-specific quoting rules.
>
> Another thought is, if we don't allow zero-length names, shouldn't
> namein() reject empty input strings?  Then this whole thing would fail
> as postgres.bki is being loaded.  (This is more hypothetical, since this
> appears to break a number of other things.)
>
> All of this is to say, it's worth looking at the actual cause and think
> about if there are related problems, maybe other name patterns that we
> don't handle well, instead of just papering over it at the top level.
>
>

--000000000000081cb20638f3913b
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div class=3D"Znr6db">Hi Peter,<br><br>Thanks for your det=
ailed analysis. I appreciate you digging deeper into the root cause.<br><br=
>For this patch, I&#39;d like to keep the changes to `initdb`=C2=A0minimal =
and focused on rejecting empty usernames, as that seems to be the consensus=
 from the previous discussion.<br><br>I&#39;ll be happy to discuss the `get=
id()`=C2=A0and `aclitem`=C2=A0parsing behavior in a separate thread.<br><br=
>Best regards,<br>Jianghua Yang</div><br></div><br><div class=3D"gmail_quot=
e gmail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">Peter Eisent=
raut &lt;<a href=3D"mailto:peter@eisentraut.org">peter@eisentraut.org</a>&g=
t; =E4=BA=8E2025=E5=B9=B47=E6=9C=882=E6=97=A5=E5=91=A8=E4=B8=89 07:39=E5=86=
=99=E9=81=93=EF=BC=9A<br></div><blockquote class=3D"gmail_quote" style=3D"m=
argin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left=
:1ex">On 02.07.25 04:55, Jianghua Yang wrote:<br>
&gt; While working with `initdb`, I noticed that passing an empty string to=
 <br>
&gt; the `-U` option (e.g., `initdb -U &#39;&#39;`) causes it to fail with =
a <br>
&gt; misleading error:<br>
&gt; <br>
&gt; performing post-bootstrap initialization ... 2025-07-01 19:48:42.006 P=
DT <br>
&gt; [14888] FATAL:role &quot;&quot;&quot; does not exist at character 72<b=
r>
&gt; <br>
&gt; 2025-07-01 19:48:42.006 PDT [14888] STATEMENT:<br>
&gt; <br>
&gt; UPDATE pg_class SET relacl =3D (SELECT array_agg(a.acl) FROM(SELECT <b=
r>
&gt; E&#39;=3Dr/&quot;&quot;&#39; as acl UNION SELECT unnest(pg_catalog.acl=
default(CASE WHEN <br>
&gt; relkind =3D &#39;S&#39; THEN &#39;s&#39;ELSE &#39;r&#39; END::&quot;ch=
ar&quot;,10::oid)) ) as a) WHERE <br>
&gt; relkind IN (&#39;r&#39;, &#39;v&#39;, &#39;m&#39;, &#39;S&#39;)AND rel=
acl IS NULL;<br>
&gt; <br>
&gt; This happens because `initdb` accepts the empty string as a valid role=
 <br>
&gt; name and attempts to use it as the database superuser, which is not <b=
r>
&gt; intended and fails during bootstrap SQL.<br>
<br>
I&#39;ll start by saying, of course an empty user name isn&#39;t going to w=
ork, <br>
so we should reject it.<br>
<br>
But let&#39;s dig a little deeper into why it fails.=C2=A0 Observe the erro=
r:<br>
<br>
=C2=A0 =C2=A0 =C2=A0FATAL:role &quot;&quot;&quot; does not exist at charact=
er 72<br>
<br>
It thinks that the role name is `&quot;` (a sole double-quote, not empty!).=
 <br>
Why is that?<br>
<br>
This error comes from the literal<br>
<br>
=C2=A0 =C2=A0 E&#39;=3Dr/&quot;&quot;&#39;<br>
<br>
interpreted as an aclitem value.=C2=A0 The aclitem parsing ends up in getid=
() <br>
in src/backend/utils/adt/acl.c, which thinks that an input string <br>
consisting entirely of &quot;&quot; is an escaped double quote.<br>
<br>
Maybe it&#39;s worth fixing that, and making putid() also print empty user =
<br>
names correspondingly.<br>
<br>
Alternatively, it&#39;s the fault of initdb that it constructs aclitem <br>
values that don&#39;t follow the aclitem-specific quoting rules.<br>
<br>
Another thought is, if we don&#39;t allow zero-length names, shouldn&#39;t =
<br>
namein() reject empty input strings?=C2=A0 Then this whole thing would fail=
 <br>
as postgres.bki is being loaded.=C2=A0 (This is more hypothetical, since th=
is <br>
appears to break a number of other things.)<br>
<br>
All of this is to say, it&#39;s worth looking at the actual cause and think=
 <br>
about if there are related problems, maybe other name patterns that we <br>
don&#39;t handle well, instead of just papering over it at the top level.<b=
r>
<br>
</blockquote></div>

--000000000000081cb20638f3913b--