Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-05 Thread Tom Lane
Jianghua Yang writes: > I wanted to ask if this patch can be merged first, as it seems to be a > separate issue different from the aclitem parsing problem Tom discussed in > https://www.postgresql.org/message-id/3792884.1751492172%40sss.pgh.pa.us Merging your patch will make it more difficult to

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-05 Thread Jianghua Yang
Hello Daniel, Tom lane I wanted to ask if this patch can be merged first, as it seems to be a separate issue different from the aclitem parsing problem Tom discussed in https://www.postgresql.org/message-id/3792884.1751492172%40sss.pgh.pa.us

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-02 Thread Tom Lane
Peter Eisentraut writes: > ... 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. Yeah, that is definitely broken, and also it occurs to me that this coding is not robust in the face

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-02 Thread Jianghua Yang
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 th

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-02 Thread Peter Eisentraut
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 PDT [14888] FATAL:role """ does

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-02 Thread Jianghua Yang
Hi hackers, Based on the suggestion that we should explicitly reject empty usernames instead of silently falling back, I’ve updated the patch accordingly. ### Changes in v2: - `initdb` now errors out immediately if the `-U` or `--username` argument is an empty string. - The error message is:

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-02 Thread Jianghua Yang
Thanks for the feedback! I've updated the test to use `command_fails_like()` instead of `command_fails()`, so it now asserts that the error message matches the expected stderr output. I also changed the test invocation to use the `-U => ''` syntax for consistency, as seen in the adjacent `--userna

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-02 Thread Daniel Gustafsson
> On 2 Jul 2025, at 15:52, Jianghua Yang wrote: > > Hi hackers, > > Based on the suggestion that we should explicitly reject empty usernames > instead of silently falling back, I’ve updated the patch accordingly. > > ### Changes in v2: > > - `initdb` now errors out immediately if the `-U` or

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-02 Thread Dagfinn Ilmari Mannsåker
Jianghua Yang writes: > - A regression test is added to `src/bin/initdb/t/001_initdb.pl` to verify > that the case `initdb -U ''` fails as expected. [ ... ] > diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl > index 15dd10ce40a..67eb53064f6 100644 > --- a/src/bin/initd

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-02 Thread Daniel Gustafsson
> On 2 Jul 2025, at 06:31, Robert Treat wrote: > FWIW, I tend to agree with David; I feel like if a user passes in -U, > there was probably a reason, and a good error message would be more > useful in clarifying things rather than blindly pushing forward with > potentially the wrong thing. Agree

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-01 Thread Robert Treat
On Wed, Jul 2, 2025 at 12:01 AM David G. Johnston wrote: > On Tue, Jul 1, 2025 at 8:31 PM Jianghua Yang wrote: >> >> git show 8e673801262c66af4a54837f63ff596407835c20 >> >> >> effective_user = get_id(); >> >> - if (strlen(username) == 0) >> >> + if (!username) >> >>

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-01 Thread David G. Johnston
We try to stick with plain text and inline/bottom replies here. On Tue, Jul 1, 2025 at 8:31 PM Jianghua Yang wrote: > git show 8e673801262c66af4a54837f63ff596407835c20 > > > effective_user = get_id(); > > - if (strlen(username) == 0) > > + if (!username) > > u

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-01 Thread Jianghua Yang
git show 8e673801262c66af4a54837f63ff596407835c20 effective_user = get_id(); - if (strlen(username) == 0) + if (!username) username = effective_user; The previous code already intended to treat a missing username as falling back to the system user. The chec

Re: [PATCH] initdb: Treat empty -U argument as unset username

2025-07-01 Thread David G. Johnston
On Tue, Jul 1, 2025 at 7:56 PM Jianghua Yang wrote: > Let me know if this approach seems reasonable or if you’d prefer we > explicitly reject empty usernames with an error instead. > > I'd rather we reject the ambiguous input. David J.