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
