Re: Fix md5_password_warnings for role/database settings

Chao Li <li.evan.chao@gmail.com>

From: Chao Li <li.evan.chao@gmail.com>
To: Fujii Masao <masao.fujii@gmail.com>
Cc: Japin Li <japinli@hotmail.com>, PostgreSQL-development <pgsql-hackers@lists.postgresql.org>, Nathan Bossart <nathandbossart@gmail.com>, Darold Gilles <gilles@darold.net>
Date: 2026-06-10T23:56:59Z
Lists: pgsql-hackers

Attachments


> On Jun 10, 2026, at 23:12, Fujii Masao <masao.fujii@gmail.com> wrote:
> 
> On Wed, Jun 10, 2026 at 8:58 PM Japin Li <japinli@hotmail.com> wrote:
>>> PSA v2 - addressed Japin’s first comment.
>>> 
>> Thanks for updating the patch, LGTM.
> 
> The patch basically looks good to me!

Thanks for your review.

> I just have three minor review comments.
> 
> 
> + my ($ret, $stdout, $stderr) = $node->psql(
> + 'postgres',
> + 'SELECT 1',
> + connstr => 'user=md5_role_no_warnings',
> + extra_params => ['-w'],
> + on_error_stop => 0);
> + is($ret, 0, 'md5 with warnings disabled');
> + unlike(
> + $stderr,
> + qr/authenticated with an MD5-encrypted password/,
> + 'md5 with warnings disabled: no MD5 authentication warning');
> 
> For this test, would it be better to use connect_ok() instead of
> psql(), like this? That would let us verify more behavior at once:
> the connection succeeds, stderr is empty (i.e., no MD5 warning is
> emitted), SHOW md5_password_warnings returns off, and the server
> log still shows method=md5.
> 
> $node->connect_ok(
> "user=md5_role_no_warnings",
> "md5 with warnings disabled",
> sql => "SHOW md5_password_warnings",
> expected_stdout => qr/^off$/,
> log_like =>
> [qr/connection authenticated: identity="md5_role_no_warnings" method=md5/]);
> 

Agreed.

> 
>  * NB: Caller should ensure the strings are allocated in a long-lived context
> - * like TopMemoryContext.
> + * like TopMemoryContext.  This function takes ownership of the strings, which
> + * will be freed in EmitConnectionWarnings().
> 
> Very minor comment: I'd suggest changing "allocated" to "palloc'd"
> and "freed" to "pfree'd", to avoid future callers passing some non-pfreeable
> memory unexpectedly.
> 

Okay, changed.

> 
> +typedef struct ConnectionWarning
> 
> Since this patch introduces a new typedef, ConnectionWarning,
> typedefs.list should probably be updated as well. Even if we forget
> to do that, it will eventually get updated later, but I think it's
> better to update it manually when possible so that pgindent works
> correctly before then.
> 

Ah, completely forgot that. Added.

PFA v3:
* Addressed all Fujii’s comments
* Changed a palloc to palloc_object

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/




Commits

  1. Fix md5_password_warnings for role and database settings