public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] initdb: Treat empty -U argument as unset username
16+ messages / 7 participants
[nested] [flat]
* [PATCH] initdb: Treat empty -U argument as unset username
@ 2025-07-02 02:55 Jianghua Yang <[email protected]>
2025-07-02 03:11 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 14:39 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Peter Eisentraut <[email protected]>
0 siblings, 2 replies; 16+ messages in thread
From: Jianghua Yang @ 2025-07-02 02:55 UTC (permalink / raw)
To: [email protected]
Hi hackers,
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 not exist at character 72
2025-07-01 19:48:42.006 PDT [14888] STATEMENT:
UPDATE pg_class SET relacl = (SELECT array_agg(a.acl) FROM (SELECT
E'=r/""' as acl UNION SELECT unnest(pg_catalog.acldefault( CASE WHEN
relkind = '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 propose a small patch that treats an empty string passed to `-U` as if
the option was not provided at all — falling back to the current system
user, which is the documented and expected behavior when `-U` is omitted.
This change improves robustness and avoids confusing failure messages due
to user input that is technically invalid but easy to produce (e.g., via
scripting or argument quoting issues).
### Patch summary:
- Checks if the passed `username` is non-null but empty (`'\0'`)
- Replaces it with the effective system user in that case
- Keeps the logic consistent with the existing behavior when `-U` is omitted
Let me know if this approach seems reasonable or if you’d prefer we
explicitly reject empty usernames with an error instead.
Patch attached.
Best regards,
Jianghua Yang
Attachments:
[application/octet-stream] 0001-initdb-Treat-empty-U-argument-as-unset-username.patch (728B, 3-0001-initdb-Treat-empty-U-argument-as-unset-username.patch)
download | inline diff:
From 38c34daaad05825d41027b8f5962658b3eee507a Mon Sep 17 00:00:00 2001
From: Jianghua Yang <[email protected]>
Date: Tue, 1 Jul 2025 19:52:52 -0700
Subject: [PATCH] initdb: Treat empty -U argument as unset username
---
src/bin/initdb/initdb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 62bbd08d9f6..2df15996271 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -3472,7 +3472,7 @@ main(int argc, char *argv[])
setup_bin_paths(argv[0]);
effective_user = get_id();
- if (!username)
+ if (!username || username[0] == '\0')
username = effective_user;
if (strncmp(username, "pg_", 3) == 0)
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
@ 2025-07-02 03:11 ` David G. Johnston <[email protected]>
2025-07-02 03:30 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
1 sibling, 1 reply; 16+ messages in thread
From: David G. Johnston @ 2025-07-02 03:11 UTC (permalink / raw)
To: Jianghua Yang <[email protected]>; +Cc: [email protected]
On Tue, Jul 1, 2025 at 7:56 PM Jianghua Yang <[email protected]> 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.
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 03:11 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
@ 2025-07-02 03:30 ` Jianghua Yang <[email protected]>
2025-07-02 04:00 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Jianghua Yang @ 2025-07-02 03:30 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: [email protected]
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 check was changed from strlen(username) == 0 to !username, but this
inadvertently stopped handling the empty-string case. This patch restores
the original intent and makes the behavior consistent.
David G. Johnston <[email protected]> 于2025年7月1日周二 20:12写道：
> On Tue, Jul 1, 2025 at 7:56 PM Jianghua Yang <[email protected]> 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.
>
>
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 03:11 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 03:30 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
@ 2025-07-02 04:00 ` David G. Johnston <[email protected]>
2025-07-02 04:31 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Robert Treat <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: David G. Johnston @ 2025-07-02 04:00 UTC (permalink / raw)
To: Jianghua Yang <[email protected]>; +Cc: [email protected]
We try to stick with plain text and inline/bottom replies here.
On Tue, Jul 1, 2025 at 8:31 PM Jianghua Yang <[email protected]> wrote:
> 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 check was changed from strlen(username) == 0 to !username, but this
> inadvertently stopped handling the empty-string case. This patch restores
> the original intent and makes the behavior consistent.
>
>>
>>
At this point I'd rather take advantage of this behaveing in the "doesn't
work" category for the past 8 years, and thus all supported releases, and
not change existing behavior (just improve the error message) rather than
accept original intent. Also, the amount of things it has to be consistent
with is quite small and I'm pytr sure that some of those are also broken -
encoding/pgdata/textsearch all exhibit the same pattern (xlog is the
reverse so maybe ok...)
David J.
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 03:11 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 03:30 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 04:00 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
@ 2025-07-02 04:31 ` Robert Treat <[email protected]>
2025-07-02 07:16 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Robert Treat @ 2025-07-02 04:31 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: Jianghua Yang <[email protected]>; [email protected]
On Wed, Jul 2, 2025 at 12:01 AM David G. Johnston
<[email protected]> wrote:
> On Tue, Jul 1, 2025 at 8:31 PM Jianghua Yang <[email protected]> wrote:
>>
>> 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 check was changed from strlen(username) == 0 to !username, but this inadvertently stopped handling the empty-string case. This patch restores the original intent and makes the behavior consistent.
>>>
>>>
>
> At this point I'd rather take advantage of this behaveing in the "doesn't work" category for the past 8 years, and thus all supported releases, and not change existing behavior (just improve the error message) rather than accept original intent. Also, the amount of things it has to be consistent with is quite small and I'm pytr sure that some of those are also broken - encoding/pgdata/textsearch all exhibit the same pattern (xlog is the reverse so maybe ok...)
>
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.
Robert Treat
https://xzilla.net
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 03:11 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 03:30 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 04:00 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 04:31 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Robert Treat <[email protected]>
@ 2025-07-02 07:16 ` Daniel Gustafsson <[email protected]>
2025-07-02 13:52 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Daniel Gustafsson @ 2025-07-02 07:16 UTC (permalink / raw)
To: Robert Treat <[email protected]>; +Cc: David G. Johnston <[email protected]>; Jianghua Yang <[email protected]>; [email protected]
> On 2 Jul 2025, at 06:31, Robert Treat <[email protected]> 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.
Agreed, and it's not even clear that the previous code was intentionally trying
to allow an empty -U. An improved error message would be a good patch though.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 03:11 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 03:30 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 04:00 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 04:31 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Robert Treat <[email protected]>
2025-07-02 07:16 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Daniel Gustafsson <[email protected]>
@ 2025-07-02 13:52 ` Jianghua Yang <[email protected]>
2025-07-02 14:01 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Daniel Gustafsson <[email protected]>
2025-07-02 14:09 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Dagfinn Ilmari Mannsåker <[email protected]>
0 siblings, 2 replies; 16+ messages in thread
From: Jianghua Yang @ 2025-07-02 13:52 UTC (permalink / raw)
To: [email protected]; +Cc: Robert Treat <[email protected]>; David G. Johnston <[email protected]>; [email protected]
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:
superuser name must not be empty
- A regression test is added to `src/bin/initdb/t/001_initdb.pl` to verify
that the case `initdb -U ''` fails as expected.
This approach avoids any ambiguity about whether an empty username is
valid, and fails early with a clear message. It also brings consistency
with existing checks, such as the one disallowing superuser names starting
with `pg_`.
Let me know if this looks acceptable or if further refinement is needed.
Patch attached.
Best regards,
Jianghua Yang
Daniel Gustafsson <[email protected]> 于2025年7月2日周三 00:16写道：
> > On 2 Jul 2025, at 06:31, Robert Treat <[email protected]> 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.
>
> Agreed, and it's not even clear that the previous code was intentionally
> trying
> to allow an empty -U. An improved error message would be a good patch
> though.
>
> --
> Daniel Gustafsson
>
>
Attachments:
[application/octet-stream] 0001-initdb-Reject-empty-string-for-U-username-option.patch (1.7K, 3-0001-initdb-Reject-empty-string-for-U-username-option.patch)
download | inline diff:
From 77326a030fd2ffa4ae012aae28540b3d8f5bd4ef Mon Sep 17 00:00:00 2001
From: Jianghua Yang <[email protected]>
Date: Wed, 2 Jul 2025 06:48:48 -0700
Subject: [PATCH] initdb: Reject empty string for -U/--username option
Previously, passing an empty string to the -U or --username option
(e.g., `initdb -U ''`) would cause confusing errors during bootstrap,
as initdb attempted to create a role with an empty name.
This patch adds an explicit check for empty usernames and exits
immediately with a clear error message.
A test case is added to verify that initdb fails when -U is given an
empty string.
---
src/bin/initdb/initdb.c | 5 +++++
src/bin/initdb/t/001_initdb.pl | 4 ++++
2 files changed, 9 insertions(+)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 62bbd08d9f6..0fd67ad496f 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -3291,6 +3291,11 @@ main(int argc, char *argv[])
pwprompt = true;
break;
case 'U':
+ if (optarg[0] == '\0')
+ {
+ pg_log_error("superuser name must not be empty");
+ exit(1);
+ }
username = pg_strdup(optarg);
break;
case 'd':
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/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -37,6 +37,10 @@ command_fails(
command_fails([ 'initdb', '--username' => 'pg_test', $datadir ],
'role names cannot begin with "pg_"');
+command_fails(
+ [ 'initdb', '-U', '', $datadir ],
+ 'empty username not allowed');
+
mkdir $datadir;
# make sure we run one successful test without a TZ setting so we test
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 03:11 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 03:30 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 04:00 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 04:31 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Robert Treat <[email protected]>
2025-07-02 07:16 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Daniel Gustafsson <[email protected]>
2025-07-02 13:52 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
@ 2025-07-02 14:01 ` Daniel Gustafsson <[email protected]>
1 sibling, 0 replies; 16+ messages in thread
From: Daniel Gustafsson @ 2025-07-02 14:01 UTC (permalink / raw)
To: Jianghua Yang <[email protected]>; +Cc: Robert Treat <[email protected]>; David G. Johnston <[email protected]>; [email protected]
> On 2 Jul 2025, at 15:52, Jianghua Yang <[email protected]> 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 `--username` argument is an empty string.
> - The error message is:
>
> superuser name must not be empty
>
> - A regression test is added to `src/bin/initdb/t/001_initdb.pl` to verify that the case `initdb -U ''` fails as expected.
>
> This approach avoids any ambiguity about whether an empty username is valid, and fails early with a clear message. It also brings consistency with existing checks, such as the one disallowing superuser names starting with `pg_`.
>
> Let me know if this looks acceptable or if further refinement is needed.
+ pg_log_error("superuser name must not be empty");
+ exit(1);
I would prefer pg_fatal() for this.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 03:11 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 03:30 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 04:00 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 04:31 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Robert Treat <[email protected]>
2025-07-02 07:16 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Daniel Gustafsson <[email protected]>
2025-07-02 13:52 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
@ 2025-07-02 14:09 ` Dagfinn Ilmari Mannsåker <[email protected]>
2025-07-02 14:17 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
1 sibling, 1 reply; 16+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2025-07-02 14:09 UTC (permalink / raw)
To: Jianghua Yang <[email protected]>; +Cc: [email protected]; Robert Treat <[email protected]>; David G. Johnston <[email protected]>; [email protected]
Jianghua Yang <[email protected]> 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/initdb/t/001_initdb.pl
> +++ b/src/bin/initdb/t/001_initdb.pl
> @@ -37,6 +37,10 @@ command_fails(
> command_fails([ 'initdb', '--username' => 'pg_test', $datadir ],
> 'role names cannot begin with "pg_"');
>
> +command_fails(
> + [ 'initdb', '-U', '', $datadir ],
> + 'empty username not allowed');
> +
This only tests that it fails, not that it fails as expected. It should
use command_fails_like() to check that stderr contains the expected
error. Also, it shoud use => between the -U option and its argument, as
seen in the above test with --username.
- ilmari
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 03:11 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 03:30 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 04:00 ` Re: [PATCH] initdb: Treat empty -U argument as unset username David G. Johnston <[email protected]>
2025-07-02 04:31 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Robert Treat <[email protected]>
2025-07-02 07:16 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Daniel Gustafsson <[email protected]>
2025-07-02 13:52 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 14:09 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Dagfinn Ilmari Mannsåker <[email protected]>
@ 2025-07-02 14:17 ` Jianghua Yang <[email protected]>
0 siblings, 0 replies; 16+ messages in thread
From: Jianghua Yang @ 2025-07-02 14:17 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <[email protected]>; +Cc: [email protected]; Robert Treat <[email protected]>; David G. Johnston <[email protected]>; [email protected]
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 `--username` test.
Let me know if any further adjustments are needed.
Best regards,
Jianghua Yang
Dagfinn Ilmari Mannsåker <[email protected]> 于2025年7月2日周三 07:09写道：
> Jianghua Yang <[email protected]> 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/initdb/t/001_initdb.pl
> > +++ b/src/bin/initdb/t/001_initdb.pl
> > @@ -37,6 +37,10 @@ command_fails(
> > command_fails([ 'initdb', '--username' => 'pg_test', $datadir ],
> > 'role names cannot begin with "pg_"');
> >
> > +command_fails(
> > + [ 'initdb', '-U', '', $datadir ],
> > + 'empty username not allowed');
> > +
>
> This only tests that it fails, not that it fails as expected. It should
> use command_fails_like() to check that stderr contains the expected
> error. Also, it shoud use => between the -U option and its argument, as
> seen in the above test with --username.
>
> - ilmari
>
Attachments:
[application/octet-stream] 0001-initdb-Reject-empty-string-for-U-username-option-v3.patch (1.7K, 3-0001-initdb-Reject-empty-string-for-U-username-option-v3.patch)
download | inline diff:
From 58cbbf1812022ead2bdab4a313b230624f97e7b0 Mon Sep 17 00:00:00 2001
From: Jianghua Yang <[email protected]>
Date: Wed, 2 Jul 2025 06:48:48 -0700
Subject: [PATCH] initdb: Reject empty string for -U/--username option v3
Previously, passing an empty string to the -U or --username option
(e.g., `initdb -U ''`) would cause confusing errors during bootstrap,
as initdb attempted to create a role with an empty name.
This patch adds an explicit check for empty usernames and exits
immediately with a clear error message.
A test case is added to verify that initdb fails when -U is given an
empty string.
---
src/bin/initdb/initdb.c | 2 ++
src/bin/initdb/t/001_initdb.pl | 7 +++++++
2 files changed, 9 insertions(+)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 62bbd08d9f6..ad5675fd120 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -3291,6 +3291,8 @@ main(int argc, char *argv[])
pwprompt = true;
break;
case 'U':
+ if (optarg[0] == '\0')
+ pg_fatal("superuser name must not be empty.");
username = pg_strdup(optarg);
break;
case 'd':
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index b7ef7ed8d06..0109925af85 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -37,6 +37,13 @@ command_fails(
command_fails([ 'initdb', '--username' => 'pg_test', $datadir ],
'role names cannot begin with "pg_"');
+command_fails_like(
+ [ 'initdb', '--username' => '', $datadir ],
+ qr/superuser name must not be empty./,
+ 'empty username not allowed');
+
+
+
mkdir $datadir;
# make sure we run one successful test without a TZ setting so we test
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
@ 2025-07-02 14:39 ` Peter Eisentraut <[email protected]>
2025-07-02 15:03 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 21:48 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Tom Lane <[email protected]>
1 sibling, 2 replies; 16+ messages in thread
From: Peter Eisentraut @ 2025-07-02 14:39 UTC (permalink / raw)
To: Jianghua Yang <[email protected]>; [email protected]
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 not exist at character 72
>
> 2025-07-01 19:48:42.006 PDT [14888] STATEMENT:
>
> UPDATE pg_class SET relacl = (SELECT array_agg(a.acl) FROM(SELECT
> E'=r/""' as acl UNION SELECT unnest(pg_catalog.acldefault(CASE WHEN
> relkind = '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'=r/""'
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.
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 14:39 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Peter Eisentraut <[email protected]>
@ 2025-07-02 15:03 ` Jianghua Yang <[email protected]>
1 sibling, 0 replies; 16+ messages in thread
From: Jianghua Yang @ 2025-07-02 15:03 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: [email protected]
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 <[email protected]> 于2025年7月2日周三 07:39写道：
> 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 not exist at character 72
> >
> > 2025-07-01 19:48:42.006 PDT [14888] STATEMENT:
> >
> > UPDATE pg_class SET relacl = (SELECT array_agg(a.acl) FROM(SELECT
> > E'=r/""' as acl UNION SELECT unnest(pg_catalog.acldefault(CASE WHEN
> > relkind = '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'=r/""'
>
> 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.
>
>
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 14:39 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Peter Eisentraut <[email protected]>
@ 2025-07-02 21:48 ` Tom Lane <[email protected]>
2025-07-05 16:58 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
1 sibling, 1 reply; 16+ messages in thread
From: Tom Lane @ 2025-07-02 21:48 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Jianghua Yang <[email protected]>; [email protected]
Peter Eisentraut <[email protected]> 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 of non-ASCII identifiers (since
the behavior of isalnum is unportable in that case). I've posted
a draft patch for that in a separate thread [1].
> Maybe it's worth fixing that, and making putid() also print empty user
> names correspondingly.
putid() prints an empty name as an empty string, which seems fine
at least at this level.
> Alternatively, it's the fault of initdb that it constructs aclitem
> values that don't follow the aclitem-specific quoting rules.
With the patch at [1], initdb -U '' still fails at the same place,
but now with
FATAL: a name must follow the "/" sign at character 72
which is a consequence of getid()'s output not distinguishing
"I found nothing" from "I found an empty string". Now if we
cared to support empty identifiers, we could make some more
revisions here so that those were consistently represented as
"" rather than nothing. But I fail to see that that's a useful
expenditure of time: we're never going to allow empty names.
> Another thought is, if we don't allow zero-length names, shouldn't
> namein() reject empty input strings?
I'm inclined to think not. It's introducing too much of a gotcha
for too little return. As a perhaps-not-quite-exact analogy,
NULL::name also doesn't correspond to any identifier you can spell
in SQL; but we're not going to try to forbid that value. So there
is at least one value of type "name" that isn't valid in SQL, and
I don't see why ''::name can't be another.
regards, tom lane
[1] https://www.postgresql.org/message-id/3792884.1751492172%40sss.pgh.pa.us
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 14:39 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Peter Eisentraut <[email protected]>
2025-07-02 21:48 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Tom Lane <[email protected]>
@ 2025-07-05 16:58 ` Jianghua Yang <[email protected]>
2025-07-05 17:34 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Tom Lane <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Jianghua Yang @ 2025-07-05 16:58 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; [email protected]
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
<https://www.google.com/url?source=gmail&sa=D&sa=E&q=https://www.postgresql.org/message-i...;
.
Best regards,
Jianghua
Tom Lane <[email protected]> 于2025年7月2日周三 14:48写道：
> Peter Eisentraut <[email protected]> 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 of non-ASCII identifiers (since
> the behavior of isalnum is unportable in that case). I've posted
> a draft patch for that in a separate thread [1].
>
> > Maybe it's worth fixing that, and making putid() also print empty user
> > names correspondingly.
>
> putid() prints an empty name as an empty string, which seems fine
> at least at this level.
>
> > Alternatively, it's the fault of initdb that it constructs aclitem
> > values that don't follow the aclitem-specific quoting rules.
>
> With the patch at [1], initdb -U '' still fails at the same place,
> but now with
>
> FATAL: a name must follow the "/" sign at character 72
>
> which is a consequence of getid()'s output not distinguishing
> "I found nothing" from "I found an empty string". Now if we
> cared to support empty identifiers, we could make some more
> revisions here so that those were consistently represented as
> "" rather than nothing. But I fail to see that that's a useful
> expenditure of time: we're never going to allow empty names.
>
> > Another thought is, if we don't allow zero-length names, shouldn't
> > namein() reject empty input strings?
>
> I'm inclined to think not. It's introducing too much of a gotcha
> for too little return. As a perhaps-not-quite-exact analogy,
> NULL::name also doesn't correspond to any identifier you can spell
> in SQL; but we're not going to try to forbid that value. So there
> is at least one value of type "name" that isn't valid in SQL, and
> I don't see why ''::name can't be another.
>
> regards, tom lane
>
> [1]
> https://www.postgresql.org/message-id/3792884.1751492172%40sss.pgh.pa.us
>
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 14:39 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Peter Eisentraut <[email protected]>
2025-07-02 21:48 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Tom Lane <[email protected]>
2025-07-05 16:58 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
@ 2025-07-05 17:34 ` Tom Lane <[email protected]>
2026-03-20 13:32 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Tom Lane @ 2025-07-05 17:34 UTC (permalink / raw)
To: Jianghua Yang <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; [email protected]
Jianghua Yang <[email protected]> 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 poke at these
underlying issues, so I'm not in a hurry to do that. It's not
like this is an urgent issue: initdb has behaved like that for
years if not decades, and nobody's complained before.
regards, tom lane
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: [PATCH] initdb: Treat empty -U argument as unset username
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 14:39 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Peter Eisentraut <[email protected]>
2025-07-02 21:48 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Tom Lane <[email protected]>
2025-07-05 16:58 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-05 17:34 ` Re: [PATCH] initdb: Treat empty -U argument as unset username Tom Lane <[email protected]>
@ 2026-03-20 13:32 ` Jianghua Yang <[email protected]>
0 siblings, 0 replies; 16+ messages in thread
From: Jianghua Yang @ 2026-03-20 13:32 UTC (permalink / raw)
To: Tom Lane <[email protected]>; [email protected]
Hi,
The underlying aclitem quoting issue has been fixed in commit 64840e46243
("Fix inconsistent quoting of role names in ACLs"), so the concern about
this patch interfering with that investigation no longer applies.
Here's a rebased version of the patch. The approach is unchanged from v3:
reject an empty string passed to -U/--username early with a clear error
message, rather than letting it fall through to a confusing FATAL during
bootstrap SQL execution.
Changes:
- Add an empty-string check before the existing pg_ prefix check
in initdb's main(), using pg_fatal().
- Add a regression test using command_fails_like() to verify the
error message.
Jianghua Yang <[email protected]> 于2025年7月5日周六 10:45写道：
> Hi Tom,
>
> Thanks for the clarification.
>
> Just to provide a bit more context — we encountered this issue in a
> production environment where initdb was being run inside a Docker
> container. In such environments, the $USER environment variable may be
> unset or empty, which causes initdb -U $USER to fail with a confusing
> error.
>
> While it's true that this behavior has existed for a long time, it's
> become more relevant as Docker and containerized deployments have become
> common. This issue can be hard to diagnose for users unfamiliar with the
> internals of initdb, especially since the failure message isn't very
> informative in this case.
>
> I understand your point about not wanting to merge the patch prematurely
> if it could obscure underlying problems. That said, this particular fix
> seems orthogonal to the ACL parsing issue and addresses a real-world
> usability problem with minimal risk.
>
> Would you be open to reconsidering a standalone merge of this patch, or
> perhaps applying a minimal workaround for cases where $USER is unset?
>
> Thanks again for your time and consideration.
>
> Best regards,
> Jianghua Yang
>
> Tom Lane <[email protected]> 于2025年7月5日周六 10:34写道：
>
>> Jianghua Yang <[email protected]> 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 poke at these
>> underlying issues, so I'm not in a hurry to do that. It's not
>> like this is an urgent issue: initdb has behaved like that for
>> years if not decades, and nobody's complained before.
>>
>> regards, tom lane
>>
>
Attachments:
[application/octet-stream] 0001-initdb-reject-empty-string-for-U-username-option.patch (1.8K, 3-0001-initdb-reject-empty-string-for-U-username-option.patch)
download | inline diff:
From e062dc164da02619f0716fa78ff0f9e422490c65 Mon Sep 17 00:00:00 2001
From: Jianghua Yang <[email protected]>
Date: Fri, 20 Mar 2026 06:30:03 -0700
Subject: [PATCH] initdb: reject empty string for -U/--username option
Previously, passing an empty string to initdb's -U option (e.g.,
initdb -U '') was accepted without complaint, but later caused a
confusing FATAL error during bootstrap SQL execution.
Add an explicit check for an empty superuser name before the existing
pg_ prefix validation, so that the user gets a clear error message
upfront.
Discussion: https://postgr.es/m/[email protected]
---
src/bin/initdb/initdb.c | 3 +++
src/bin/initdb/t/001_initdb.pl | 3 +++
2 files changed, 6 insertions(+)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 509f1114ef6..25cd398cc6c 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -3500,6 +3500,9 @@ main(int argc, char *argv[])
if (!username)
username = effective_user;
+ if (username[0] == '\0')
+ pg_fatal("superuser name must not be empty");
+
if (strncmp(username, "pg_", 3) == 0)
pg_fatal("superuser name \"%s\" is disallowed; role names cannot begin with \"pg_\"", username);
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index 081535a22e4..8c54461fe98 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -34,6 +34,9 @@ command_fails(
[ 'initdb', '--waldir' => 'pgxlog', $datadir ],
'relative xlog directory not allowed');
+command_fails_like([ 'initdb', '--username' => '', $datadir ],
+ qr/superuser name must not be empty/,
+ 'empty username not allowed');
command_fails([ 'initdb', '--username' => 'pg_test', $datadir ],
'role names cannot begin with "pg_"');
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 16+ messages in thread
end of thread, other threads:[~2026-03-20 13:32 UTC | newest]
Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-07-02 02:55 [PATCH] initdb: Treat empty -U argument as unset username Jianghua Yang <[email protected]>
2025-07-02 03:11 ` David G. Johnston <[email protected]>
2025-07-02 03:30 ` Jianghua Yang <[email protected]>
2025-07-02 04:00 ` David G. Johnston <[email protected]>
2025-07-02 04:31 ` Robert Treat <[email protected]>
2025-07-02 07:16 ` Daniel Gustafsson <[email protected]>
2025-07-02 13:52 ` Jianghua Yang <[email protected]>
2025-07-02 14:01 ` Daniel Gustafsson <[email protected]>
2025-07-02 14:09 ` Dagfinn Ilmari Mannsåker <[email protected]>
2025-07-02 14:17 ` Jianghua Yang <[email protected]>
2025-07-02 14:39 ` Peter Eisentraut <[email protected]>
2025-07-02 15:03 ` Jianghua Yang <[email protected]>
2025-07-02 21:48 ` Tom Lane <[email protected]>
2025-07-05 16:58 ` Jianghua Yang <[email protected]>
2025-07-05 17:34 ` Tom Lane <[email protected]>
2026-03-20 13:32 ` Jianghua Yang <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox