Horton

Thread

Commits

  1. libpq: Improve error handling in passwordFromFile()

  1. [PATCH] Add error message for out-of-memory in passwordFromFile()

    Joshua Shanks <jjshanks@gmail.com> — 2025-11-01T23:49:02Z

    Hi,

I noticed a XXX comment in fe-connect.c at line 8064 indicating that
an error message would be nice when strdup() fails while reading the
password file. Currently, passwordFromFile() silently returns NULL on
allocation failure, leaving users without feedback on what went wrong.

This patch adds proper error reporting by:

1. Adding a PGconn *conn parameter to passwordFromFile() to enable
error reporting via libpq_append_conn_error()
2. Reporting "out of memory" when strdup() fails, following the
pattern used by other OOM handlers in fe-connect.c

The conn parameter is placed first, consistent with the object-oriented
convention used throughout libpq (see connectOptions1,
fillPGconn, store_conn_addrinfo, etc.).

Cheers,
Joshua

  2. Re: [PATCH] Add error message for out-of-memory in passwordFromFile()

    Michael Paquier <michael@paquier.xyz> — 2025-11-02T00:09:55Z

    On Sat, Nov 01, 2025 at 04:49:02PM -0700, Joshua Shanks wrote:
> The conn parameter is placed first, consistent with the object-oriented
> convention used throughout libpq (see connectOptions1,
> fillPGconn, store_conn_addrinfo, etc.).

Sounds OK to me.  Nice catch.  It's probably not worth bothering in
the stable branches as the error is unlikely going to show up.
--
Michael

  3. Re: [PATCH] Add error message for out-of-memory in passwordFromFile()

    Michael Paquier <michael@paquier.xyz> — 2025-11-03T10:35:40Z

    On Sun, Nov 02, 2025 at 09:09:55AM +0900, Michael Paquier wrote:
> Sounds OK to me.  Nice catch.  It's probably not worth bothering in
> the stable branches as the error is unlikely going to show up.

And, while checking the surroundings, I have noticed that your patch
is incomplete: passwordFromFile() is not designed currently in such a
way in fe-connect.c so as it is possible for its callers to make a
difference between one of the valid cases (like a dbname or user name
equal to NULL, service file that cannot be opened, etc.) and an error.

The point would be to have pqConnectOptions2() take its oom_error path
in the case you are pointing at.  It seems to me that the correct
answer would be to extend passwordFromFile() with an extra "const char
**errmsg", where an error is saved, assign an error string in the
routine, check if the error pointer is set when exiting
passwordFromFile() in pqConnectOptions2(), and consume the error
properly before failing in pqConnectOptions2() with a CONNECTION_BAD.
--
Michael

  4. Re: [PATCH] Add error message for out-of-memory in passwordFromFile()

    Joshua Shanks <jjshanks@gmail.com> — 2025-11-04T05:15:25Z

    On Mon, Nov 3, 2025 at 2:35 AM Michael Paquier <michael@paquier.xyz> wrote:

> The point would be to have pqConnectOptions2() take its oom_error path
> in the case you are pointing at.  It seems to me that the correct
> answer would be to extend passwordFromFile() with an extra "const char
> **errmsg", where an error is saved, assign an error string in the
> routine, check if the error pointer is set when exiting
> passwordFromFile() in pqConnectOptions2(), and consume the error
> properly before failing in pqConnectOptions2() with a CONNECTION_BAD.
>

Thank you for the detailed feedback! This is my first C project
contribution, so I really appreciate you taking the time to explain the
proper error handling pattern.

I've updated the patch to address your concerns:

Changes in v2:
- Extended passwordFromFile() with const char **errmsg parameter to signal
actual errors
- Set *errmsg = "out of memory" for OOM cases (enlargePQExpBuffer and
strdup failures)
- Updated pqConnectOptions2() to check errmsg and fail with CONNECTION_BAD
when set
- Removed the PGconn *conn parameter since error reporting is now handled
by the caller
- Updated function comment to document the error signaling convention

Cheers,
Joshua

  5. Re: [PATCH] Add error message for out-of-memory in passwordFromFile()

    Michael Paquier <michael@paquier.xyz> — 2025-11-04T11:17:02Z

    On Mon, Nov 03, 2025 at 09:15:25PM -0800, Joshua Shanks wrote:
> Thank you for the detailed feedback! This is my first C project
> contribution, so I really appreciate you taking the time to explain the
> proper error handling pattern.

No problem.  Applied on HEAD after a few tweaks:
* two libpq_gettext() added,
* *errmsg set to NULL when entering passwordFromFile().
* Some rewords.
--
Michael