Use HostsFileName everywhere
Hello!
I noticed that 3 error messages hardcode pg_hosts.conf instead of
using the user-configured filename.
Attached patch fixes these occurrences.
Attachments:
0001-Use-HostsFileName-everywhere.patchapplication/octet-stream; name=0001-Use-HostsFileName-everywhere.patchDownload+3-4
Hi Zsolt,
Thanks for reporting the bug and the patch. If an operator changes the
hosts_file GUC, without the patch those three messages don't get updated
and it makes debugging a lot harder. The fix is clearly right.
A couple of observations:
1. Small typo in the commit message "harcoded" → "hardcoded".
2. While reviewing I noticed a related NULL-safety question. This isn't
caused by your patch, I was just digging to see whether it could turn into
a crash or segfault after the change.
guc_parameters.dat declares hosts_file with boot_val => 'NULL', so
HostsFileName can be NULL if the GUC is never assigned. After the patch,
the three messages pass HostsFileName straight to errmsg("... \"%s\" ...",
HostsFileName), and passing NULL to %s is technically undefined behaviour.
I traced the code path and ran a small reproducer, if HostsFileName is NULL
and ssl_sni is on, execution flows into load_hosts() ->
open_auth_file(NULL, ...) -> fopen(NULL, "r"), which on macOS returns NULL
with errno=EFAULT (glibc behaves the same way).
The subsequent errmsg(..., "%s", NULL) prints (null) rather than crashing.
So the observable difference from this patch in the NULL corner case is:
before the patch we would see: 'could not load "pg_hosts.conf": ...'
after the patch we would see: 'could not load "(null)": ...'
Both are misleading but neither is fatal. Also the corner case itself is
hard to reach with configuration alone as SelectConfigFiles() in guc.c
fills in configdir/pg_hosts.conf if hosts_file is unset.
Regards,
Surya Poondla
On Mon, Jul 06, 2026 at 04:43:53PM -0700, surya poondla wrote:
Hi Zsolt,
Thanks for reporting the bug and the patch. If an operator changes the
hosts_file GUC, without the patch those three messages don't get updated
and it makes debugging a lot harder. The fix is clearly right.
Looks like an oversight of 4f433025f666. This is not critical, still
a nice life improvement if setting a custom file value for these error
messages.
A couple of observations:
1. Small typo in the commit message "harcoded" → "hardcoded".
2. While reviewing I noticed a related NULL-safety question. This isn't
caused by your patch, I was just digging to see whether it could turn into
a crash or segfault after the change.
SetConfigOption() is called for hosts_file at an early startup stage,
as of SelectConfigFiles(). init_host_context() and be_tls_init()
require the GUCs to be loaded, meaning that a NULL value would not be
an issue because it will be either the default of pg_hosts.conf or the
custom value set in postgresql.conf. HBA and ident files work the
same way: we need them loaded before any authentication would kick in,
including direct SSL requests.
In short, I think that the patch should be OK. Purely cosmetic, still
OK. Daniel?
--
Michael
On 7 Jul 2026, at 08:36, Michael Paquier <michael@paquier.xyz> wrote:
In short, I think that the patch should be OK. Purely cosmetic, still
OK. Daniel?
Agreed, I had this thread shortlisted for when back from vacation so I will
take care of it shortly.
--
Daniel Gustafsson