Re: Fix PQport to never return NULL if the connection is valid

From: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
To: Daniele Varrazzo <daniele(dot)varrazzo(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix PQport to never return NULL if the connection is valid
Date: 2025-07-16 15:32:59
Message-ID: f8b27cdf8ef2a1e36d24d4dcc2e18e169fad9a3f.camel@cybertec.at
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 2025-05-08 at 22:01 +0200, Daniele Varrazzo wrote:
> I looked a bit more into the meaning of the port="" setting. The docs
> for the port parameter / PGPORT env var
> <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-PORT>
> say:
>
> An empty string, or an empty item in a comma-separated list,
> specifies the default port number established when
> PostgreSQL was built
>
> Thus I understand that the return value of PQport wants to reflect
> this behaviour, therefore the value "" is legitimate and it's up to
> the client to figure out how the libpq was built (PQconndefaults may
> tell that).
>
> 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)

> diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
> --- a/src/interfaces/libpq/fe-connect.c
> +++ b/src/interfaces/libpq/fe-connect.c
> @@ -7574,7 +7574,9 @@ PQport(const PGconn *conn)
> if (!conn)
> return NULL;
>
> - if (conn->connhost != NULL)
> + if (conn->connhost != NULL &&
> + conn->connhost[conn->whichhost].port != NULL &&
> + conn->connhost[conn->whichhost].port[0] != '\0')
> return conn->connhost[conn->whichhost].port;
>
> return "";

You should omit the third test. If the first character of the port
string is 0, the string is empty, and you might as well return it
instead of a static empty string.

The patch applies (with considerable offset), builds well and passes
the regression tests. The latter is unsurprising, since nothing tests
for that. I wondered if it was worth adding a test, but could not
find a convenient place. Adding a TAP test seems a bit out of
proportion for a small fix like that.

Status set to "waiting on author".

Yours,
Laurenz Albe

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2025-07-16 15:44:16 Re: Changing shared_buffers without restart
Previous Message Peter Geoghegan 2025-07-16 15:29:40 Re: index prefetching