> 在 2026年6月10日，19:39，Chao Li <[email protected]> 写道： > > > >> On Jun 10, 2026, at 18:22, Japin Li <[email protected]> wrote: >> >> >> Hi, Chao >> >>> On Wed, 10 Jun 2026 at 14:26, Chao Li <[email protected]> wrote: >>> Hi, >>> >>> While testing “[bc60ee860] Warn upon successful MD5 password >>> authentication”, I found a small issue. >>> >>> This feature emits a warning based on the existing GUC >>> md5_password_warnings, but it queues the message in >>> md5_crypt_verify(), before GUC values are loaded by >>> process_startup_options() and process_settings(). As a result, >>> settings loaded later during connection startup, such as startup >>> options or ALTER ROLE/ALTER DATABASE settings, are not honored for >>> this warning. >>> >>> Here is a repro: >>> >>> 1. Edit pg_hba.conf, add this line: >>> ``` >>> local postgres md5_role md5 >>> ``` >>> >>> 2. Setup in session 1: >>> ``` >>> evantest=# set password_encryption='md5'; >>> SET >>> evantest=# create role md5_role login password 'pass'; >>> WARNING: setting an MD5-encrypted password >>> DETAIL: MD5 password support is deprecated and will be removed in a future >>> release of PostgreSQL. >>> HINT: Refer to the PostgreSQL documentation for details about migrating to >>> another password type. >>> CREATE ROLE >>> evantest=# >>> evantest=# alter role md5_role set md5_password_warnings =0; >>> ALTER ROLE >>> evantest=# select pg_reload_conf(); -- reload pg_hba.conf as I didn’t >>> restart the server >>> pg_reload_conf >>> ---------------- >>> t >>> (1 row) >>> ``` >>> >>> 3. Connect as md5_role: >>> ``` >>> % PGPASSWORD=pass psql -d postgres -U md5_role -X -qAt -c “show >>> md5_password_warnings" >>> WARNING: authenticated with an MD5-encrypted password >>> DETAIL: MD5 password support is deprecated and will be removed in a future >>> release of PostgreSQL. >>> off >>> ``` >>> >>> As we can see, although the role’s md5_password_warnings setting is off, >>> the warning message is still shown. >>> >>> This feature uses the connection warning infrastructure introduced by >>> 1d92e0c2cc, so fixing the problem requires enhancing that infrastructure. >>> >>> In the current implementation, there are two lists: >>> ConnectionWarningMessages and ConnectionWarningDetails. The attached >>> patch combines them into one list and adds a filter function to each >>> list member, so the filter can be applied in >>> EmitConnectionWarnings(). With this mechanism, the warning emitted >>> upon successful MD5 authentication is checked against the final value >>> of md5_password_warnings, while 1d92e0c2cc’s password expiration >>> warning logic remains unchanged. >>> >> >> I'm in favor of this idea. > > Thanks for your review. > >> >>> See the attached patch for details. >> >> A few comments: >> >> 1. >> EmitConnectionWarnings(void) >> { >> - ListCell *lc_msg; >> - ListCell *lc_detail; >> + ListCell *lc; >> >> if (ConnectionWarningsEmitted) >> elog(ERROR, "EmitConnectionWarnings() called more than once"); >> else >> ConnectionWarningsEmitted = true; >> >> - forboth(lc_msg, ConnectionWarningMessages, >> - lc_detail, ConnectionWarningDetails) >> + foreach(lc, ConnectionWarnings) >> { >> - ereport(WARNING, >> - (errmsg("%s", (char *) lfirst(lc_msg)), >> - errdetail("%s", (char *) lfirst(lc_detail)))); >> + ConnectionWarning *warning = lfirst(lc); >> + >> >> Perhaps we could use foreach_ptr(ConnectionWarning, warning, >> ConnectionWarnings) >> to simplify the code. > > Agreed. > >> >> 2. >> StoreConnectionWarning() states that the caller should ensure the strings are >> allocated in a long-lived context. Since the two existing calls already use >> TopMemoryContext, should the function always switch the memory context >> internally? >> > > I raised the same comment when I reviewed the original patch of 1d92e0c2cc, > and the comment was addressed by adding the header comment, see [1] commit 3. > So, I’d not touch this part.
I have missed that conversation — thanks for bringing it up. > PSA v2 - addressed Japin’s first comment. > Thanks for updating the patch, LGTM. > [1] https://postgr.es/m/[email protected] > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/ > > > > > <v2-0001-Fix-md5_password_warnings-for-role-and-database-s.patch>
