Thread: correction suggestion for https://www.postgresql.org/docs/17/auth-username-maps.html
correction suggestion for https://www.postgresql.org/docs/17/auth-username-maps.html
From
PG Doc comments form
Date:
The following documentation comment has been logged on the website: Page: https://www.postgresql.org/docs/17/auth-username-maps.html Description: Dear all, Pls. let me suggest the correction for the https://www.postgresql.org/docs/17/auth-username-maps.html page. It has the following sentence: ' If the database-username field starts with a slash (/), the remainder of the field is treated as a regular expression (see Section 9.7.3.1 for details of PostgreSQL's regular expression syntax). It is not possible to use \1 to use a capture from regular expression on system-username for a regular expression on database-username. ' It looks like 'to use a capture' has to be replaced by 'to capture'. best regards Alexey Shishkin
Re: correction suggestion for https://www.postgresql.org/docs/17/auth-username-maps.html
From
"David G. Johnston"
Date:
On Wed, Jul 9, 2025 at 7:56 AM PG Doc comments form <noreply@postgresql.org> wrote:
The following documentation comment has been logged on the website:
Page: https://www.postgresql.org/docs/17/auth-username-maps.html
Description:
Dear all,
Pls. let me suggest the correction for the
https://www.postgresql.org/docs/17/auth-username-maps.html page.
It has the following sentence:
'
If the database-username field starts with a slash (/), the remainder of the
field is treated as a regular expression (see Section 9.7.3.1 for details of
PostgreSQL's regular expression syntax). It is not possible to use \1 to use
a capture from regular expression on system-username for a regular
expression on database-username.
'
It looks like 'to use a capture' has to be replaced by 'to capture'.
best regards
What is written is factually correct. Your suggestion makes it incorrect; and wouldn't be good English even if it was.
"to capture" involves (...) while using said capture involves \#
David J.
Re: correction suggestion for https://www.postgresql.org/docs/17/auth-username-maps.html
From
Tom Lane
Date:
"David G. Johnston" <david.g.johnston@gmail.com> writes: > On Wed, Jul 9, 2025 at 7:56 AM PG Doc comments form <noreply@postgresql.org> > wrote: >> Pls. let me suggest the correction for the >> https://www.postgresql.org/docs/17/auth-username-maps.html page. >> It has the following sentence: >> ' >> If the database-username field starts with a slash (/), the remainder of >> the >> field is treated as a regular expression (see Section 9.7.3.1 for details >> of >> PostgreSQL's regular expression syntax). It is not possible to use \1 to >> use >> a capture from regular expression on system-username for a regular >> expression on database-username. >> ' >> It looks like 'to use a capture' has to be replaced by 'to capture'. >> best regards > What is written is factually correct. Your suggestion makes it > incorrect; and wouldn't be good English even if it was. The existing sentence is pretty mangled English, though. I think it would be clearer as When the database-username field is a regular expression, it is not possible to use \1 within it to refer to a capture from the system-username field. Thoughts? regards, tom lane
Re: correction suggestion for https://www.postgresql.org/docs/17/auth-username-maps.html
From
"David G. Johnston"
Date:
On Wed, Jul 9, 2025 at 9:22 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Wed, Jul 9, 2025 at 7:56 AM PG Doc comments form <noreply@postgresql.org>
> wrote:
>> Pls. let me suggest the correction for the
>> https://www.postgresql.org/docs/17/auth-username-maps.html page.
>> It has the following sentence:
The existing sentence is pretty mangled English, though.
Agreed.
When the database-username field is a regular expression, it is
not possible to use \1 within it to refer to a capture from
the system-username field.
Thoughts?
Its good as far a narrow fix goes.
But how about the attached? More invasive but covers the salient points better, IMO, and less repetitive than having the two fields have their own basically copy-pasted paragraphs.
I didn't add an example but felt the point "be referenced a single time within" to be needed since, usefulness not withstanding, writing \1\1 for database-username works but only the first instance of \1 is replaced.
Also, should we attempt to align this documentation and pg_ident.conf.sample as pertains to pg-username vs. database-username?
David J.
Attachment
Re: correction suggestion for https://www.postgresql.org/docs/17/auth-username-maps.html
From
Tom Lane
Date:
"David G. Johnston" <david.g.johnston@gmail.com> writes: > Its good as far a narrow fix goes. > But how about the attached? More invasive but covers the salient points > better, IMO, and less repetitive than having the two fields have their own > basically copy-pasted paragraphs. Meh... I initially thought that merging the two paras sounded like a good idea, but I'm not finding that this formulation reads any better. Notably, as things stand we have parallel constructions "If the <field> starts with an <x> character" in the preceding para as well as these two, and I think it's good to keep that parallelism. I do agree that it's overly repetitive, but we could improve that by dropping the second instance of the parenthetical link to posix-syntax-details. > I didn't add an example but felt the point "be referenced a single time > within" to be needed since, usefulness not withstanding, writing \1\1 for > database-username works but only the first instance of \1 is replaced. Hmm, I wonder if that isn't a bug we should fix. It's hard to believe anyone is relying on the second \1 *not* getting replaced, and perhaps there are use-cases for multiple replacements. > Also, should we attempt to align this documentation and > pg_ident.conf.sample as pertains to pg-username vs. database-username? Agreed that making pg_ident.conf.sample match would be an improvement. regards, tom lane
Re: correction suggestion for https://www.postgresql.org/docs/17/auth-username-maps.html
From
Tom Lane
Date:
I wrote: > "David G. Johnston" <david.g.johnston@gmail.com> writes: >> I didn't add an example but felt the point "be referenced a single time >> within" to be needed since, usefulness not withstanding, writing \1\1 for >> database-username works but only the first instance of \1 is replaced. > Hmm, I wonder if that isn't a bug we should fix. It's hard to believe > anyone is relying on the second \1 *not* getting replaced, and perhaps > there are use-cases for multiple replacements. Here's a quick patch for that. I hacked up 003_peer.pl enough to prove that multiple replacement works, but that test change is not committable as-is because it assumes that the "system user" name is "postgres". I don't like the existing test much either, since it only tests the case of the substituted string being empty, which means the substitution code could be quite broken and it wouldn't notice. But I don't offhand see a way to improve that without making assumptions about the incoming name... regards, tom lane diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 332fad27835..fecee8224d0 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -2873,8 +2873,11 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name, !token_has_regexp(identLine->pg_user) && (ofs = strstr(identLine->pg_user->string, "\\1")) != NULL) { + const char *repl_str; + size_t repl_len; + char *old_pg_user; char *expanded_pg_user; - int offset; + size_t offset; /* substitution of the first argument requested */ if (matches[1].rm_so < 0) @@ -2886,18 +2889,33 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name, *error_p = true; return; } + repl_str = system_user + matches[1].rm_so; + repl_len = matches[1].rm_eo - matches[1].rm_so; /* - * length: original length minus length of \1 plus length of match - * plus null terminator + * It's allowed to have more than one \1 in the string, and we'll + * replace them all. But that's pretty unusual so we optimize on + * the assumption of only one occurrence, which motivates doing + * repeated replacements instead of making two passes over the + * string to determine the final length right away. */ - expanded_pg_user = palloc0(strlen(identLine->pg_user->string) - 2 + (matches[1].rm_eo - matches[1].rm_so) +1); - offset = ofs - identLine->pg_user->string; - memcpy(expanded_pg_user, identLine->pg_user->string, offset); - memcpy(expanded_pg_user + offset, - system_user + matches[1].rm_so, - matches[1].rm_eo - matches[1].rm_so); - strcat(expanded_pg_user, ofs + 2); + old_pg_user = identLine->pg_user->string; + do + { + /* + * length: current length minus length of \1 plus length of + * replacement plus null terminator + */ + expanded_pg_user = palloc(strlen(old_pg_user) - 2 + repl_len + 1); + /* ofs points into the old_pg_user string at this point */ + offset = ofs - old_pg_user; + memcpy(expanded_pg_user, old_pg_user, offset); + memcpy(expanded_pg_user + offset, repl_str, repl_len); + strcpy(expanded_pg_user + offset + repl_len, ofs + 2); + if (old_pg_user != identLine->pg_user->string) + pfree(old_pg_user); + old_pg_user = expanded_pg_user; + } while ((ofs = strstr(old_pg_user + offset + repl_len, "\\1")) != NULL); /* * Mark the token as quoted, so it will only be compared literally diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl index f2320b62c87..8a9431e5594 100644 --- a/src/test/authentication/t/003_peer.pl +++ b/src/test/authentication/t/003_peer.pl @@ -93,6 +93,8 @@ if ($node->log_contains( $node->safe_psql('postgres', qq{CREATE ROLE testmapuser LOGIN}); $node->safe_psql('postgres', "CREATE ROLE testmapgroup NOLOGIN"); $node->safe_psql('postgres', "GRANT testmapgroup TO testmapuser"); +# This role is for testing \1 substitution. +$node->safe_psql('postgres', qq{CREATE ROLE testgresgresmapuser LOGIN}); # Note the double quotes here. $node->safe_psql('postgres', 'CREATE ROLE "testmapgroupliteral\\1" LOGIN'); $node->safe_psql('postgres', 'GRANT "testmapgroupliteral\\1" TO testmapuser'); @@ -212,10 +214,10 @@ test_role( # Success as the regular expression matches and \1 is replaced in the given # subexpression. -reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$}, 'test\1mapuser'); +reset_pg_ident($node, 'mypeermap', qq{/^post(.*)\$}, 'test\1\1mapuser'); test_role( $node, - qq{testmapuser}, + qq{testgresgresmapuser}, 'peer', 0, 'with regular expression in user name map with \1 replaced',