Re: Fix PQport to never return NULL if the connection is valid - Mailing list pgsql-hackers

Laurenz Albe <laurenz.albe@cybertec.at> writes:
> On Thu, 2025-05-08 at 22:01 +0200, Daniele Varrazzo wrote:
>> Please find attached a new patch that doesn't change the behaviour and
>> just makes sure to not return NULL in case no info is available in
>> 'conn->connhost'.

> I think that it is important to fix that bug and to backpatch it.
> Without the patch, I can produce the following crash:

>   $ psql port=
>   psql (18beta1)
>   Type "help" for help.

>   test=> \conninfo
>   Segmentation fault (core dumped)

This crash is actually new in v18; earlier versions produce something
like

psql (17.5)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in "/tmp" at port "(null)".
postgres=#

because the code was basically

                    printf(_("You are connected to database \"%s\" as user \"%s\" via socket in \"%s\" at port
\"%s\".\n"),
                           db, PQuser(pset.db), host, PQport(pset.db));

and fortunately our implementation of printf is guaranteed not to
crash on a null string pointer.

Having said that, I agree with back-patching Daniele's fix, because it
is making the code agree with what happened before we introduced the
connhost[] data structure in v10, and also with the documentation:

       <xref linkend="libpq-PQport"/> returns <symbol>NULL</symbol> if the
       <parameter>conn</parameter> argument is <symbol>NULL</symbol>.
       Otherwise, if there is an error producing the port information (perhaps
       if the connection has not been fully established or there was an
       error), it returns an empty string.

However ... I wonder if we ought to take the opportunity to fix
\conninfo to ensure that it prints something useful rather than
just an empty string?  The upgrade to make it print in tabular
format is what broke this, but it also seems like an excuse to
improve the functionality as well as the cosmetics.

However, fixing this in \conninfo seems a bit tedious: we'd have
to grovel through the output of PQconndefaults.  (I don't think
it's okay for psql to assume that it can rely on having been
built with the same value of DEF_PGPORT_STR that the libpq it's
linked to is using.)  So it seems attractive to stick with the
original thought of making PQport substitute the correct value
for an empty string.  PQport *does* know the correct value of
DEF_PGPORT_STR, so it'd be pretty nearly a one-liner to make
this happen there.

I'd only propose doing this in v18/HEAD, and sticking to the
v2 patch in the stable branches.

            regards, tom lane