On 8/19/22 01:12, Drouvot, Bertrand wrote: > + wstr = palloc((strlen(tok->string + 1) + 1) * sizeof(pg_wchar)); > > + wlen = pg_mb2wchar_with_len(tok->string + 1, > > + wstr, strlen(tok->string + 1));
The (tok->string + 1) construction comes up often enough that I think it should be put in a `regex` variable or similar. That would help my eyes with the (strlen(tok->string + 1) + 1) construction, especially. I noticed that for pg_ident, we precompile the regexes per-line and reuse those in child processes. Whereas here we're compiling, using, and then discarding the regex for each check. I think the example set by the pg_ident code is probably the one to follow, unless you have a good reason not to. > +# Testing with regular expression for username > > +reset_pg_hba($node, '/^.*md.*$', 'password'); > > +test_role($node, 'md5_role', 'password from pgpass and regular expression > for username', 0); > + IMO the coverage for this patch needs to be filled out. Negative test cases are more important than positive ones for security-related code. Other than that, and Tom's note on potentially expanding this to other areas, I think this is a pretty straightforward patch. Thanks, --Jacob
