Subject: [oauth] Stabilize the libpq-oauth ABI (and allow alternative implementations?)

From: Jacob Champion <[email-masked]>
Date: Tue, 9 Dec 2025 13:00:58 -0800

Hi everybody,

We introduced the libpq-oauth module late in the cycle for PG18, and
to put it mildly, its interface isn't great. The original
implementation depended on libpq internals, and we had to make sure
that we didn't start crashing during major or minor version upgrades.
So there were a bunch of compromises made to keep things safe,
including the restriction that the module name has to change every
major release.

Separately, but closely related: PG18's OAuth support allows you to
customize the client flow via a libpq hook function. Third-party
applications can make use of that, but our own utilities can still
only use the builtin device flow. That's annoying.

I've been working to replace the internal ABI with a stable one,
hopefully to solve both problems at the same time. A dlopen() is a
pretty clear seam for other people to use to modify and extend.
Unfortunately my first attempt (not pictured) ended up in a rabbit
hole, because I tried to tackle the third-party use case first. My
second attempt, attached, focuses on the ABI stabilization instead,
which I think is more likely to succeed.

(This took enough thinking that I'm really glad we didn't try this for
PG18. Thanks for letting me take on some technical debt for a bit.)

= Design =

Here's the train of thought behind the core changes (which are in patch 0004):

The builtin-flow code in fe-auth-oauth.c is similar to the custom-flow
code, but it's just ever-so-slightly different. I'd like to unify the
two, so I want libpq-oauth to make use of the public
PGoauthBearerRequest API, and that means that almost all of the
injections made in the PG18 ABI need to be replaced.

Most of those injections are simply subsumed by the hook API
(hooray!). A couple of others can be replaced by PQconninfo(). Four
are left over:
- pgthreadlock_t
- libpq_gettext
- conn->errorMessage
- conn->oauth_issuer_id

I think we should keep injecting libpq_gettext; no third-party
implementations would be able to use that. And application hooks are
presumably capable of figuring out top-level locking already, since
the application has to have called PQregisterThreadLock() if it needed
to coordinate with libpq.

That leaves error messages and the issuer identifier. I think both
would be useful for hooks to have, too, so I'd like to add them to
PQAUTHDATA_OAUTH_BEARER_TOKEN.

= PQAUTHDATA_OAUTH_BEARER_TOKEN, version 2 =

My original plan for authdata extensions was to add new members to the
end of the structs that libpq passes into the hook. Applications would
gate on a feature macro during compilation to see whether they could
use the new members. That should work fine for an application hook;
you're not allowed to downgrade libpq past the version that your
applications are compiled against, lest you lose symbols (or other
feature-flag functionality) you're relying on.

Plugins, unfortunately, can't rely on the feature macro. As we found
out during the libpq-oauth split [1], we have to handle a long-running
application with an old libpq that loads an upgraded libpq-oauth, even
if the OS package dependencies suggest otherwise. (A plugin
architecture flips the direction of the runtime dependency arrow.)

There are a couple ways we could handle this. For this draft, I've
implemented what I think is a middle ground between verbosity and
type-safety: introduce a new V2 struct that "inherits" the V1 struct
and can be down-cast in the callbacks, kinda similar to our Node
hierarchy. We could go even more verbose, and duplicate the entire
PGoauthBearerToken struct -- matching the callback parameter types for
maximum safety -- but I'm not convinced that this would be a good use
of maintenance effort. The ability to cast between the two means we
can share v1 and v2 implementations in our tests.

We could also just add the new members at the end, say that you're
only allowed to use them if the V2 hook type is in use, and scribble
on them in V1 hooks to try to get misbehaving implementations to crash
outright. This arguably has the same amount of type-safety as the
downcast, and the resulting code looks nicer. (The libcurl API we use
does something similar with curl_version_info().) But it is definitely
more "magic".

Also of note: this adds a PQExpBuffer to libpq-fe.h. Technically, that
type is "internal". But... is it really, though? It doesn't seem
possible for us to make incompatible changes there without crashing
earlier psqls, in which case I would like to make use of it too. Maybe
this deserves its own minithread.

Okay, on to the full patchset.

= Roadmap: Prep =

The first few patches are bugfixes I intend to backpatch to 18.

- 0001: I stomped on the SOCKTYPE name in libpq-fe.h, but that's not
in our namespace and it's conceivable that it might collide with
someone else. (It collided with my own test code during my work on
this.)
- 0002 fixes a copy-paste bug in meson.build, which luckily hadn't
caused problems yet.
- 0003 ports Tom's debug2 fix for Test::Cluster::connect_fails() over
to 002_client.pl. (Currently, log checks in this test aren't made
after connection failures, but I don't really want to chase that down
later after I've forgotten about it.)

= Roadmap: Implementation =

Next three patches are the core implementation, which stabilizes the
ABI for libpq-oauth. I feel fairly confident that this, or something
close to it, could land in PG19.

- 0004 introduces the new PQAUTHDATA_OAUTH_BEARER_TOKEN_V2 API.

As described above.

- 0005 makes use of the new API in libpq-oauth.

This removes more code than it introduces, which is exciting.

Now we can rename libpq-oauth-19.so to just libpq-oauth.so, since we
no longer depend on anything that could change between major versions.
(We still need lock and locale support from libpq, as mentioned above,
so they continue to be decoupled via injection at init time.)

Some of the code in this patch overlaps with the translation fixes
thread [2], which I need to get to first. I'm hoping some additional
simplifications can be made after those localization bugs are fixed.

I think I'd also like to get the threadlock into the module API (but
not the hook API). A third-party flow might need to perform its own
one-time initialization, and if it relies on an old version of Curl
(or worse, Kerberos [3]), it'll need to use the same lock that the
top-level application has registered for libpq. So I imagine I'll need
to break out an initialization function here. Alternatively, I could
introduce an API into libpq to retrieve the threadlock function in
use?

- 0006 removes a potential ABI-compatibility pitfall for future devs.

libpq-oauth needs to use the shared-library variant of libpgcommon,
but it can no longer assume that the encoding support exported by
libpq is compatible. So it must not accidentally link against those
functions (see [4]). I don't imagine anyone will try adding code that
does this in practice; we're pretty UTF8-centric in OAuth. But just to
be safe, define USE_PRIVATE_ENCODING_FUNCS so that anyone who tries
will fail the build.

= Roadmap: Plugins? =

So now we have a stable ABI, which technically means that any
enterprising developer who wants to play games with LD_LIBRARY_PATH
could implement their own version of an OAuth flow, and have our
utilities make use of it into the future.

That brings us to patch 0007, which experimentally promotes the stable
API to a public header, and introduces a really janky environment
variable so that people don't have to play games. It will be obvious
from the code that this is not well-baked yet.

I hope the ability to dlopen() a custom flow can make it for 19; I
just don't know that this envvar approach is any good. The ideal
situation, IMO, is for a flow to be selected in the connection string.
But we have to lock that down, similarly to how we protect
local_preload_libraries, to prevent horrible exploits. At which point
we'll have essentially designed a generic libpq plugin system. Not
necessarily a terrible thing, but I don't think I have time to take it
on for PG19.

WDYT?
--Jacob

[1] https://postgr.es/m/aAkJnDQq3mOUvmQV%40msg.df7cb.de
[2] https://postgr.es/m/TY4PR01MB1690746DB91991D1E9A47F57E94CDA%40TY4PR01MB16907.jpnprd01.prod.outlook.com
[3] https://postgr.es/m/aSSp03wmNMngi/Oe%40ubby
[4] https://postgr.es/c/b6c7cfac8

---

From: Chao Li <[email-masked]>
Date: Fri, 12 Dec 2025 15:42:25 +0800

> On Dec 10, 2025, at 05:00, Jacob Champion <[email-masked]> wrote:
> 
> Hi everybody,
> 
> <0001-libpq-fe.h-Don-t-claim-SOCKTYPE-in-the-global-namesp.patch><0002-libpq-oauth-use-correct-c_args-in-meson.build.patch><0003-oauth_validator-Avoid-races-in-log_check.patch><0004-libpq-Introduce-PQAUTHDATA_OAUTH_BEARER_TOKEN_V2.patch><0005-libpq-oauth-Use-the-PGoauthBearerRequestV2-API.patch><0006-libpq-oauth-Never-link-against-libpq-s-encoding-func.patch><0007-WIP-Introduce-third-party-OAuth-flow-plugins.patch>

Hi Jacob,

This is a solid patch set. Only a few small comments:

1 - 0001
```
 /* for PGoauthBearerRequest.async() */
 #ifdef _WIN32
-#define SOCKTYPE uintptr_t		/* avoids depending on winsock2.h for SOCKET */
+#define PQ_SOCKTYPE uintptr_t	/* avoids depending on winsock2.h for SOCKET */
 #else
-#define SOCKTYPE int
+#define PQ_SOCKTYPE int
 #endif
```

The commit message has explained why SOCKTYPE is temporary and the reason why adding prefix “PG_” is to avoid collisions. But I don’t think code readers will always read commit messages, given the macro is a local and temporary, why adding a prefix starting with a underscore, like “_PQ_SOCKTYPE”, which explicitly indicates the macro is kinda private.

===
0002 & 0003 Looks good.
===

2 - 0004
```
+ * Helper for handling user flow failures. If the implementation put anything
+ * into request->error, it's added to conn->errorMessage here.
```

Typo: put -> puts

3 - 0004
```
+# Make sure the v1 hook continues to work. */
+test(
```

“*/“ in the end of the comment line seems a typo.

4 - 0005
```
+	PQAUTHDATA_OAUTH_BEARER_TOKEN,	/* server requests an OAuth Bearer token
+									 * (prefer v2, below, instead) */
```
"(prefer v2, below, instead)" looks confusing to me, though I can understand what it means. Maybe make it clearer, like “(v2 is preferred; see below)"

===
0006 Looks good.
===

===
Not reviewing 0007 as it marks with WIP.
===

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

---

From: Jacob Champion <[email-masked]>
Date: Tue, 16 Dec 2025 09:40:44 -0800

> This is a solid patch set. Only a few small comments:

Thanks for the review!

> The commit message has explained why SOCKTYPE is temporary and the reason why adding prefix “PG_” is to avoid collisions. But I don’t think code readers will always read commit messages, given the…

_PQ_SOCKTYPE is reserved (starts with _P), but I could add more
explanatory comments if you think that'd be useful. See v2-0001, which
now includes an explanation of the signature in the documentation.

The hard part is that I don't want to require all Windows clients of
libpq-fe.h to have to depend on Winsock; that's the only reason for
this oddity. Otherwise I'd declare PGsocket as the public version of
our internal pgsocket and call it a day.

> + * into request->error, it's added to conn->errorMessage here.

Past tense was my intent, but I've reworded to avoid any garden paths:
"If anything was put into request->error, it's added to
conn->errorMessage here."

> “*/“ in the end of the comment line seems a typo.

Thanks, no idea why I did that.

> "(prefer v2, below, instead)" looks confusing to me, though I can understand what it means. Maybe make it clearer, like “(v2 is preferred; see below)"

Done.

--

v2 makes these changes and rebases over latest HEAD. I'll plan to get
0001-3 in this week; probably tomorrow.

Open questions remain:
1) 0004: Any objections to putting PQExpBuffer into libpq-fe.h?
2) 0004: Thoughts on the v2 inheritance struct style as opposed to
relying on implementations to double-check the struct length?
3) 0005: Should I add the thread lock to an init() API, or expose a
new PQgetThreadLock() that other code can use?
4) 0007: [all of it]

My personal thoughts on these:
1) it's fine
2) it's a coin flip for me; inheritance is ugly, length magic is scary
3) I like the idea of PQgetThreadLock() so that we don't have to
inject it everywhere it could possibly be needed

Thanks,
--Jacob
1:  3b27f54b4dc ! 1:  8312bf30726 libpq-fe.h: Don't claim SOCKTYPE in the global namespace
    @@ Commit message
         doesn't seem too far-fetched, given its proximity to existing POSIX
         macro names.
     
    -    Add a PQ_ prefix to avoid collisions, and backpatch.
    +    Add a PQ_ prefix to avoid collisions, update and improve the surrounding
    +    documentation, and backpatch.
     
    +    Reviewed-by: Chao Li <[email-masked]>
    +    Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
         Backpatch-through: 18
     
      ## doc/src/sgml/libpq.sgml ##
     @@ doc/src/sgml/libpq.sgml: typedef struct PGoauthBearerRequest
    -     /* Callback implementing a custom asynchronous OAuth flow. */
    + 
    +     /* Hook outputs */
    + 
    +-    /* Callback implementing a custom asynchronous OAuth flow. */
    ++    /*
    ++     * Callback implementing a custom asynchronous OAuth flow. The signature is
    ++     * platform-dependent: PQ_SOCKTYPE is SOCKET on Windows, and int everywhere
    ++     * else.
    ++     */
          PostgresPollingStatusType (*async) (PGconn *conn,
                                              struct PGoauthBearerRequest *request,
     -                                        SOCKTYPE *altsock);
    @@ doc/src/sgml/libpq.sgml: typedef struct PGoauthBearerRequest
      
          /* Callback to clean up custom allocations. */
          void        (*cleanup) (PGconn *conn, struct PGoauthBearerRequest *request);
    +@@ doc/src/sgml/libpq.sgml: typedef struct PGoauthBearerRequest
    +          hook. When the callback cannot make further progress without blocking,
    +          it should return either <symbol>PGRES_POLLING_READING</symbol> or
    +          <symbol>PGRES_POLLING_WRITING</symbol> after setting
    +-         <literal>*pgsocket</literal> to the file descriptor that will be marked
    ++         <literal>*altsock</literal> to the file descriptor that will be marked
    +          ready to read/write when progress can be made again. (This descriptor
    +          is then provided to the top-level polling loop via
    +          <function>PQsocket()</function>.) Return <symbol>PGRES_POLLING_OK</symbol>
     
      ## src/interfaces/libpq/libpq-fe.h ##
     @@ src/interfaces/libpq/libpq-fe.h: typedef struct _PGpromptOAuthDevice
    + 	int			expires_in;		/* seconds until user code expires */
    + } PGpromptOAuthDevice;
      
    - /* for PGoauthBearerRequest.async() */
    +-/* for PGoauthBearerRequest.async() */
    ++/*
    ++ * For PGoauthBearerRequest.async(). This macro just allows clients to avoid
    ++ * depending on libpq-int.h or Winsock for the "socket" type; it's undefined
    ++ * immediately below.
    ++ */
      #ifdef _WIN32
     -#define SOCKTYPE uintptr_t		/* avoids depending on winsock2.h for SOCKET */
     +#define PQ_SOCKTYPE uintptr_t	/* avoids depending on winsock2.h for SOCKET */
    @@ src/interfaces/libpq/libpq-fe.h: typedef struct _PGpromptOAuthDevice
      
      typedef struct PGoauthBearerRequest
     @@ src/interfaces/libpq/libpq-fe.h: typedef struct PGoauthBearerRequest
    + 	 * blocking during the original call to the PQAUTHDATA_OAUTH_BEARER_TOKEN
    + 	 * hook, it may be returned directly, but one of request->async or
    + 	 * request->token must be set by the hook.
    ++	 *
    ++	 * The (PQ_SOCKTYPE *) in the signature is a placeholder for the platform's
    ++	 * native socket type: (SOCKET *) on Windows, and (int *) everywhere else.
      	 */
      	PostgresPollingStatusType (*async) (PGconn *conn,
      										struct PGoauthBearerRequest *request,
2:  3b5690a49d7 ! 2:  899597d5777 libpq-oauth: use correct c_args in meson.build
    @@ Commit message
         libpq_so_c_args, rather than libpq_oauth_so_c_args. (At the moment, the
         two lists are identical, but that won't be true forever.)
     
    +    Reviewed-by: Chao Li <[email-masked]>
    +    Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
         Backpatch-through: 18
     
      ## src/interfaces/libpq-oauth/meson.build ##
3:  4933e9b53fb ! 3:  953c743f692 oauth_validator: Avoid races in log_check()
    @@ Commit message
     
         Commit e0f373ee4 fixed up races in Cluster::connect_fails when using
         log_like. t/002_client.pl didn't get the memo, though, because it
    -    doesn't use Test::Cluster to perform its custom hook tests. Introduce
    -    the fix, based on debug2 logging, to its use of log_check() as well, and
    -    move the logic into the test() helper so that any additions don't need
    -    to continually duplicate it.
    +    doesn't use Test::Cluster to perform its custom hook tests. (This is
    +    probably not an issue at the moment, since the log check is only done
    +    after authentication success and not failure, but there's no reason to
    +    wait for someone to hit it.)
     
    +    Introduce the fix, based on debug2 logging, to its use of log_check() as
    +    well, and move the logic into the test() helper so that any additions
    +    don't need to continually duplicate it.
    +
    +    Reviewed-by: Chao Li <[email-masked]>
    +    Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
         Backpatch-through: 18
     
      ## src/test/modules/oauth_validator/t/002_client.pl ##
4:  b2cece52ba6 ! 4:  3c047e6cf24 libpq: Introduce PQAUTHDATA_OAUTH_BEARER_TOKEN_V2
    @@ Commit message
         TODO: Could we just add to the end of PGoauthBearerRequest, and tell
               users not to use the additional fields if they have version 1?
     
    +    Reviewed-by: Chao Li <[email-masked]>
    +    Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
    +
      ## doc/src/sgml/libpq.sgml ##
     @@ doc/src/sgml/libpq.sgml: PQauthDataHook_type PQgetAuthDataHook(void);
              <indexterm><primary>PQAUTHDATA_PROMPT_OAUTH_DEVICE</primary></indexterm>
    @@ src/interfaces/libpq/libpq-fe.h: typedef enum
      									 * URL */
     -	PQAUTHDATA_OAUTH_BEARER_TOKEN,	/* server requests an OAuth Bearer token */
     +	PQAUTHDATA_OAUTH_BEARER_TOKEN,	/* server requests an OAuth Bearer token
    -+									 * (prefer v2, below, instead) */
    ++									 * (v2 is preferred; see below) */
     +	PQAUTHDATA_OAUTH_BEARER_TOKEN_V2,	/* newest API for OAuth Bearer tokens */
      } PGauthData;
      
    @@ src/interfaces/libpq/fe-auth-oauth.c: cleanup:
      }
      
     +/*
    -+ * Helper for handling user flow failures. If the implementation put anything
    -+ * into request->error, it's added to conn->errorMessage here.
    ++ * Helper for handling flow failures. If anything was put into request->error,
    ++ * it's added to conn->errorMessage here.
     + */
     +static void
     +report_user_flow_error(PGconn *conn, const PGoauthBearerRequestV2 *request)
    @@ src/test/modules/oauth_validator/t/002_client.pl: test(
     +	expect_success => 1,
      	log_like => [qr/oauth_validator: token="my-token", role="$user"/]);
      
    -+# Make sure the v1 hook continues to work. */
    ++# Make sure the v1 hook continues to work.
     +test(
     +	"v1 synchronous hook can provide a token",
     +	flags => [
5:  c012bc715e1 ! 5:  e655d4ee091 libpq-oauth: Use the PGoauthBearerRequestV2 API
    @@ Commit message
               to the private NLS initialization. Should we expose the lock to
               that function as well?
     
    +    Reviewed-by: Chao Li <[email-masked]>
    +    Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
    +
      ## src/interfaces/libpq-oauth/README ##
     @@ src/interfaces/libpq-oauth/README: results in a failed connection.
      
    @@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct asy
     +	 * also the documentation for struct async_ctx.
     +	 */
     +	if (actx->errctx)
    -+		appendPQExpBuffer(errbuf, "%s: ", libpq_gettext(actx->errctx));
    ++		appendPQExpBuffer(errbuf, "%s: ", actx->errctx);
     +
     +	if (PQExpBufferDataBroken(actx->errbuf))
     +		appendPQExpBufferStr(errbuf, libpq_gettext("out of memory"));
    @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow_impl(PGconn *conn)
     @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow_impl(PGconn *conn)
      		{
      			case OAUTH_STEP_INIT:
    - 				actx->errctx = "failed to fetch OpenID discovery document";
    + 				actx->errctx = libpq_gettext("failed to fetch OpenID discovery document");
     -				if (!start_discovery(actx, conn_oauth_discovery_uri(conn)))
     +				if (!start_discovery(actx, actx->discovery_uri))
      					goto error_return;
    @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow_impl(PGconn *conn)
     -	 * also the documentation for struct async_ctx.
     -	 */
     -	if (actx->errctx)
    --		appendPQExpBuffer(errbuf, "%s: ", libpq_gettext(actx->errctx));
    +-		appendPQExpBuffer(errbuf, "%s: ", actx->errctx);
     -
     -	if (PQExpBufferDataBroken(actx->errbuf))
     -		appendPQExpBufferStr(errbuf, libpq_gettext("out of memory"));
    @@ src/interfaces/libpq/fe-auth-oauth.c: oauth_init(PGconn *conn, const char *passw
      static void
      oauth_free(void *opaq)
     @@ src/interfaces/libpq/fe-auth-oauth.c: cleanup:
    - }
    - 
    - /*
    -- * Helper for handling user flow failures. If the implementation put anything
    -- * into request->error, it's added to conn->errorMessage here.
    -+ * Helper for handling flow failures. If the implementation put anything into
    -+ * request->error, it's added to conn->errorMessage here.
    +  * it's added to conn->errorMessage here.
       */
      static void
     -report_user_flow_error(PGconn *conn, const PGoauthBearerRequestV2 *request)
6:  dd491541a01 ! 6:  41132991bb4 libpq-oauth: Never link against libpq's encoding functions
    @@ Commit message
         dependency on the exported APIs will simply fail to link the shared
         module.
     
    +    Reviewed-by: Chao Li <[email-masked]>
    +    Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
    +
      ## src/interfaces/libpq-oauth/meson.build ##
     @@ src/interfaces/libpq-oauth/meson.build: libpq_oauth_sources = files(
      libpq_oauth_so_sources = files(
7:  eda0c6f9059 ! 7:  e20b555aba5 WIP: Introduce third-party OAuth flow plugins?
    @@ Commit message
               new vulnerabilities
         TODO: how hard would it be to support Windows here?
     
    +    Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
    +
      ## src/interfaces/libpq/meson.build ##
     @@ src/interfaces/libpq/meson.build: pkgconfig.generate(
      install_headers(
    @@ src/test/modules/oauth_validator/t/002_client.pl: sub test
      
      test(
     @@ src/test/modules/oauth_validator/t/002_client.pl: test(
    - # Make sure the v1 hook continues to work. */
    + # Make sure the v1 hook continues to work.
      test(
      	"v1 synchronous hook can provide a token",
     +	hook_only => 1,    # plugins don't support API v1

---

From: Jacob Champion <[email-masked]>
Date: Fri, 27 Feb 2026 11:42:52 -0800

On Thu, Feb 26, 2026 at 1:56 PM Jacob Champion
<[email-masked]> wrote:
> > And a few specific comments about the patches:
>
> Thank you for the review!

v4 should address that feedback. (The Valgrind work isn't part of it, yet.)

0001 should be ready to go. I meant to put 0002 up in its own thread
at the beginning of the week but got sidetracked, so I'll go do that
now. 0003 has to wait on Valgrind, and 0004-5 have to wait on 0002.

Thanks,
--Jacob
1:  0bc540fbb7e ! 1:  62c795ef489 oauth: Report cleanup errors as warnings on stderr
    @@ Commit message
         print WARNINGs when they find themselves in similar situations, so
         follow their lead.
     
    +    Reviewed-by: Zsolt Parragi <[email-masked]>
    +    Discussion: https://postgr.es/m/CAOYmi%2BmEU_q9sr1PMmE-4rLwFN%3DOjyndDwFZvpsMU3RNJLrM9g%40mail.gmail.com
    +
      ## src/interfaces/libpq-oauth/oauth-curl.c ##
    -@@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct async_ctx *actx)
    +@@ src/interfaces/libpq-oauth/oauth-curl.c: struct async_ctx
    +  * Tears down the Curl handles and frees the async_ctx.
    +  */
    + static void
    +-free_async_ctx(PGconn *conn, struct async_ctx *actx)
    ++free_async_ctx(struct async_ctx *actx)
    + {
    + 	/*
    + 	 * In general, none of the error cases below should ever happen if we have
      	 * no bugs above. But if we do hit them, surfacing those errors somehow
      	 * might be the only way to have a chance to debug them.
      	 *
    @@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct asy
      	}
      
      	free_provider(&actx->provider);
    +@@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_cleanup_oauth_flow(PGconn *conn)
    + 
    + 	if (state->async_ctx)
    + 	{
    +-		free_async_ctx(conn, state->async_ctx);
    ++		free_async_ctx(state->async_ctx);
    + 		state->async_ctx = NULL;
    + 	}
    + 
2:  074645e5b41 ! 2:  b31a3406af2 libpq: Add PQgetThreadLock() to mirror PQregisterThreadLock()
    @@ Commit message
         currently relies on direct injection of pg_g_threadlock, and should
         ideally not.
     
    +    Reviewed-by: Zsolt Parragi <[email-masked]>
    +    Discussion: https://postgr.es/m/CAOYmi%2BmEU_q9sr1PMmE-4rLwFN%3DOjyndDwFZvpsMU3RNJLrM9g%40mail.gmail.com
    +
      ## src/interfaces/libpq/exports.txt ##
     @@ src/interfaces/libpq/exports.txt: PQgetAuthDataHook         207
      PQdefaultAuthDataHook     208
    @@ src/interfaces/libpq/libpq-fe.h: extern "C"
      
     +/* Features added in PostgreSQL v19: */
     +/* Indicates presence of PQgetThreadLock */
    -+#define LIBPQ_HAS_GET_THREAD_LOCK
    ++#define LIBPQ_HAS_GET_THREAD_LOCK 1
     +
      /*
       * Option flags for PQcopyResult
3:  b431ee7a1e0 ! 3:  61eeb16c49d libpq: Introduce PQAUTHDATA_OAUTH_BEARER_TOKEN_V2
    @@ src/interfaces/libpq/libpq-fe.h
     @@ src/interfaces/libpq/libpq-fe.h: extern "C"
      /* Features added in PostgreSQL v19: */
      /* Indicates presence of PQgetThreadLock */
    - #define LIBPQ_HAS_GET_THREAD_LOCK
    + #define LIBPQ_HAS_GET_THREAD_LOCK 1
     +/* Indicates presence of the PQAUTHDATA_OAUTH_BEARER_TOKEN_V2 authdata hook */
     +#define LIBPQ_HAS_OAUTH_BEARER_TOKEN_V2 1
      
4:  59a5e8e27d6 ! 4:  3ec1933f6d4 libpq-oauth: Use the PGoauthBearerRequestV2 API
    @@ src/interfaces/libpq-oauth/exports.txt
     +pg_start_oauthbearer      2
     
      ## src/interfaces/libpq-oauth/meson.build ##
    -@@ src/interfaces/libpq-oauth/meson.build: libpq_oauth_st = static_library('libpq-oauth',
    +@@ src/interfaces/libpq-oauth/meson.build: endif
      
      # This is an internal module; we don't want an SONAME and therefore do not set
      # SO_MAJOR_VERSION.
     -libpq_oauth_name = 'libpq-oauth-@0@'.format(pg_version_major)
     -
    + if build_shared_lib
     -libpq_oauth_so = shared_module(libpq_oauth_name,
     +libpq_oauth_so = shared_module('libpq-oauth',
        libpq_oauth_sources + libpq_oauth_so_sources,
    @@ src/interfaces/libpq-oauth/oauth-curl.c: enum OAuthStep
      	enum OAuthStep step;		/* where are we in the flow? */
      
      	int			timerfd;		/* descriptor for signaling async timeouts */
    -@@ src/interfaces/libpq-oauth/oauth-curl.c: struct async_ctx
    -  * Tears down the Curl handles and frees the async_ctx.
    -  */
    - static void
    --free_async_ctx(PGconn *conn, struct async_ctx *actx)
    -+free_async_ctx(PGoauthBearerRequestV2 *req, struct async_ctx *actx)
    - {
    - 	/*
    - 	 * In general, none of the error cases below should ever happen if we have
    -@@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct async_ctx *actx)
    +@@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(struct async_ctx *actx)
      	if (actx->timerfd >= 0)
      		close(actx->timerfd);
      
    @@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct asy
     +
     +	/* request->cleanup is only set after actx has been allocated. */
     +	Assert(actx);
    -+
    -+	free_async_ctx((PGoauthBearerRequestV2 *) request, actx);
    + 
    +-	if (state->async_ctx)
    ++	free_async_ctx(actx);
     +	request->user = NULL;
     +
     +	/* libpq has made its own copy of the token; clear ours now. */
     +	if (request->token)
    -+	{
    + 	{
    +-		free_async_ctx(state->async_ctx);
    +-		state->async_ctx = NULL;
     +		explicit_bzero(request->token, strlen(request->token));
     +		free(request->token);
     +		request->token = NULL;
    + 	}
     +}
    -+}
    -+
    + 
    +-	set_conn_altsock(conn, PGINVALID_SOCKET);
     +/*
     + * Builds an error message from actx and stores it in req->error. The allocation
     + * is backed by actx->work_data (which will be reset first).
    @@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct asy
     +		appendPQExpBufferStr(errbuf, libpq_gettext("out of memory"));
     +	else
     +		appendPQExpBufferStr(errbuf, actx->errbuf.data);
    - 
    --	if (state->async_ctx)
    ++
     +	if (actx->curl_err[0])
    - 	{
    --		free_async_ctx(conn, state->async_ctx);
    --		state->async_ctx = NULL;
    ++	{
     +		appendPQExpBuffer(errbuf, " (libcurl: %s)", actx->curl_err);
     +
     +		/* Sometimes libcurl adds a newline to the error buffer. :( */
    @@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct asy
     +			errbuf->data[errbuf->len - 1] = '\0';
     +			errbuf->len--;
     +		}
    - 	}
    - 
    --	set_conn_altsock(conn, PGINVALID_SOCKET);
    ++	}
    ++
     +	req->error = errbuf->data;
      }
      
    @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn)
     +pg_start_oauthbearer(PGconn *conn, PGoauthBearerRequestV2 *request)
     +{
     +	struct async_ctx *actx;
    ++	PQconninfoOption *conninfo = NULL;
     +
     +	if (!initialize_curl(request))
     +		return -1;
    @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn)
     +	initPQExpBuffer(&actx->errbuf);
     +
     +	/* Pull relevant connection options. */
    -+	{
    -+		PQconninfoOption *conninfo = PQconninfo(conn);
    -+
    ++	conninfo = PQconninfo(conn);
     +	if (!conninfo)
     +		goto oom;
     +
    @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn)
     +	}
     +
     +	PQconninfoFree(conninfo);
    -+	}
    ++	conninfo = NULL;			/* keeps `goto oom` safe */
     +
     +	actx->discovery_uri = request->v1.openid_configuration;
     +	actx->issuer_id = request->issuer;
    @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn)
     +	return 0;
     +
     +oom:
    ++	if (conninfo)
    ++		PQconninfoFree(conninfo);
    ++
     +	request->error = libpq_gettext("out of memory");
     +	return -1;
     +}
5:  60c2a289390 = 5:  6cf4a1482ed libpq-oauth: Never link against libpq's encoding functions
6:  b6974d5c8e2 ! 6:  f4e787faccd WIP: Introduce third-party OAuth flow plugins?
    @@ Metadata
      ## Commit message ##
         WIP: Introduce third-party OAuth flow plugins?
     
    +    [Not intended for PG19.]
    +
         This experimental commit promotes the pg_start_oauthbearer API to a
         public header (libpq-oauth.h) and adds a PGOAUTHMODULE environment
         variable that overrides the load path for the plugin, allowing users to
    @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st
     +			fprintf(stderr, "failed dlsym for %s: %s\n", module_name, dlerror());
     +
     +		dlclose(state->flow_module);
    ++		state->flow_module = NULL;
     +
     +		if (state->builtin)
     +			return 0;
    @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st
     -		libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr);
     -		return 0;
     -	}
    -+			libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr);
    -+			return 0;
    -+		}
    ++			appendPQExpBuffer(&conn->errorMessage,
    ++							  "use_builtin_flow: failed to lock mutex (%d)\n",
    ++							  lockerr);
      
     -	if (!initialized)
     -	{
     -		init(
    ++			request->error = "";	/* satisfy report_flow_error() */
    ++			return -1;
    ++		}
    ++
     +		if (!initialized)
     +		{
     +			init(
    @@ src/test/modules/oauth_validator/oauth_flow.c (new)
     +			flag_count++;
     +	}
     +
    ++	/* Slice OAUTH_TEST_FLAGS into a fake argv array. */
    ++	env = strdup(env);
     +	argc = flag_count + 1;
     +	argv = malloc(sizeof(*argv) * (argc + 1));
    -+	if (!argv)
    ++
    ++	if (!env || !argv)
     +	{
     +		fprintf(stderr, "out of memory");
     +		exit(1);

---

From: Jacob Champion <[email-masked]>
Date: Tue, 3 Mar 2026 14:08:21 -0800

On Fri, Feb 27, 2026 at 11:42 AM Jacob Champion
<[email-masked]> wrote:
> now.

Done at [1].

v5 incorporates that version of the patch in -0002 and makes some
minor test updates for better coverage in -0003.

> 0003 has to wait on Valgrind, and 0004-5 have to wait on 0002.

v5-0006 is my experiment in Valgrind (and also AddressSanitizer, and
also generic pointer poisoning). I think it's interesting, and it
seems clean enough for some subset to be committable for 19. The
weak-symbol support for ASan is the most suspect; I have no idea if it
links correctly across all platforms. It's probably better for me to
propose generic ASan support in utils/memdebug.h at some point in the
future, and drop it from this patch.

I don't plan to block the other patches on -0006, since -0004 is
blocking PGOAUTHCAFILE.

--Jacob

[1] https://postgr.es/m/CAOYmi%2B%3DMHD%2BWKD4rsTn0v8220mYfyLGhEc5EfhmtqrAb7SmC5g%40mail.gmail.com
1:  caec9379055 = 1:  caec9379055 oauth: Report cleanup errors as warnings on stderr
2:  6a2eda46927 ! 2:  7228af114ca libpq: Add PQgetThreadLock() to mirror PQregisterThreadLock()
    @@ Commit message
         Reviewed-by: Zsolt Parragi <[email-masked]>
         Discussion: https://postgr.es/m/CAOYmi%2BmEU_q9sr1PMmE-4rLwFN%3DOjyndDwFZvpsMU3RNJLrM9g%40mail.gmail.com
     
    + ## doc/src/sgml/libpq.sgml ##
    +@@ doc/src/sgml/libpq.sgml: int PQisthreadsafe();
    +    Kerberos calls because Kerberos functions are not thread-safe.  See
    +    function <function>PQregisterThreadLock</function> in the
    +    <application>libpq</application> source code for a way to do cooperative
    +-   locking between <application>libpq</application> and your application.
    ++   locking between <application>libpq</application> and your application. (Note
    ++   that it is only safe to call <function>PQregisterThreadLock</function> when
    ++   there are no open connections.) Clients may retrieve the current locking
    ++   callback with <function>PQgetThreadLock</function>; if no custom callback
    ++   has been registered, a default is used.
    +   </para>
    + 
    +   <para>
    +
      ## src/interfaces/libpq/exports.txt ##
     @@ src/interfaces/libpq/exports.txt: PQgetAuthDataHook         207
      PQdefaultAuthDataHook     208
3:  12d4224bf61 ! 3:  751df22f99d libpq: Introduce PQAUTHDATA_OAUTH_BEARER_TOKEN_V2
    @@ src/test/modules/oauth_validator/t/002_client.pl: if ($ENV{with_libcurl} ne 'yes
      $common_connstr = "$common_connstr connect_timeout=1";
      test(
     @@ src/test/modules/oauth_validator/t/002_client.pl: foreach my $c (@cases)
    + 		"hook misbehavior: $c->{'flag'}",
    + 		flags => [ $c->{'flag'} ],
      		expected_stderr => $c->{'expected_error'});
    ++
    ++	test(
    ++		"hook misbehavior: $c->{'flag'} (v1)",
    ++		flags => [ '-v1', $c->{'flag'} ],
    ++		expected_stderr => $c->{'expected_error'});
      }
      
     +# v2 async flows should be able to set error messages, too.
4:  84d8dcd390e = 4:  fb9915437da libpq-oauth: Use the PGoauthBearerRequestV2 API
5:  e6b7735576b = 5:  ed6e7268ec2 libpq-oauth: Never link against libpq's encoding functions
-:  ----------- > 6:  e4b09853e3a WIP: test out poisoning
6:  e4e5c5164b8 ! 7:  3d6d0901f7e WIP: Introduce third-party OAuth flow plugins?
    @@ src/interfaces/libpq-oauth/oauth-curl.h (deleted)
     
      ## src/interfaces/libpq/fe-auth-oauth.h ##
     @@ src/interfaces/libpq/fe-auth-oauth.h: typedef struct
    - 	void	   *async_ctx;
      
    + 	bool		v1;
      	bool		builtin;
     -	void	   *builtin_flow;
     +	void	   *flow_module;
    @@ src/interfaces/libpq/fe-auth-oauth.c: extern int	pg_start_oauthbearer(PGconn *co
      }
      
     @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth_state *state)
    - 	if (res == 0)
    - 		res = PQauthDataHook(PQAUTHDATA_OAUTH_BEARER_TOKEN, conn, &request);
    + 		poison_req_v2(&request, false);
    + 	}
      	if (res == 0)
     -	{
     -		state->builtin = true;
    @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth
     -	return true;
      
      fail:
    - 	if (request.v1.cleanup)
    + 	do_cleanup(state, &request);
     
      ## src/test/modules/oauth_validator/meson.build ##
     @@ src/test/modules/oauth_validator/meson.build: test_install_libs += magic_validator
    @@ src/test/modules/oauth_validator/t/002_client.pl: if ($ENV{with_libcurl} ne 'yes
      		flags => ["--no-hook"],
      		expected_stderr =>
      		  qr/no OAuth flows are available \(try installing the libpq-oauth package\)/
    +@@ src/test/modules/oauth_validator/t/002_client.pl: foreach my $c (@cases)
    + 	test(
    + 		"hook misbehavior: $c->{'flag'} (v1)",
    + 		flags => [ '-v1', $c->{'flag'} ],
    ++		hook_only => 1,    # plugins can't use -v1
    + 		expected_stderr => $c->{'expected_error'});
    + }
    + 
     @@ src/test/modules/oauth_validator/t/002_client.pl: test(
      	expected_stderr =>
      	  qr/user-defined OAuth flow failed: async error message/);

---

From: Jacob Champion <[email-masked]>
Date: Thu, 5 Mar 2026 16:57:13 -0800

> I don't plan to block the other patches on -0006, since -0004 is

v5-0001 and -0002 are now committed. Attached is a v6 rebase with some
minor self-review fixups and the removal of ASan from the poisoning
experiment.

v6-0001 through -0003 are targeted for commit next. I'll leave -0004
for later; I plan to ask for some committer review, since it's pretty
unusual code for libpq, but it needs a real commit message and some
better docs first.

As for -0005: I think I'll cherry-pick only the ability for the
libpq_oauth_init entry to be optional, marking any flows that don't
provide it as user-defined, to make it possible for v19 developers to
help us iterate on v20 if they want.

Thanks,
--Jacob
1:  caec9379055 < -:  ----------- oauth: Report cleanup errors as warnings on stderr
2:  7228af114ca < -:  ----------- libpq: Add PQgetThreadLock() to mirror PQregisterThreadLock()
3:  751df22f99d ! 1:  90f05c5d963 libpq: Introduce PQAUTHDATA_OAUTH_BEARER_TOKEN_V2
    @@ doc/src/sgml/libpq.sgml: typedef struct PGoauthBearerRequest
     +         <replaceable>issuer</replaceable> contains the issuer identifier, as
     +         defined in <ulink url="https://datatracker.ietf.org/doc/html/rfc9207">RFC 9207</ulink>,
     +         that is in use for the current connection. This identifier is
    -+         determined by <xref linkend="libpq-connect-oauth-issuer"/>.
    ++         derived from <xref linkend="libpq-connect-oauth-issuer"/>.
     +         To avoid mix-up attacks, custom flows should ensure that any discovery
     +         metadata provided by the authorization server matches this issuer ID.
     +        </para>
    @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth

      ## src/test/modules/oauth_validator/oauth_hook_client.c ##
    -@@
    - 
    - #include "getopt_long.h"
    - #include "libpq-fe.h"
    -+#include "pqexpbuffer.h"
    - 
    - static int	handle_auth_data(PGauthData type, PGconn *conn, void *data);
    - static PostgresPollingStatusType async_cb(PGconn *conn,
     @@ src/test/modules/oauth_validator/oauth_hook_client.c: usage(char *argv[])
      
      	printf("recognized flags:\n");
4:  fb9915437da = 2:  d8fbfc8f7a0 libpq-oauth: Use the PGoauthBearerRequestV2 API
5:  ed6e7268ec2 = 3:  96bd5970668 libpq-oauth: Never link against libpq's encoding functions
6:  e4b09853e3a ! 4:  0e7fae1e152 WIP: test out poisoning
    @@ Metadata
      ## Commit message ##
         WIP: test out poisoning
     
    - ## src/interfaces/libpq/meson.build ##
    -@@ src/interfaces/libpq/meson.build: libpq_c_args = ['-DSO_MAJOR_VERSION=5']
    - # The OAuth implementation differs depending on the type of library being built.
    - libpq_so_c_args = ['-DUSE_DYNAMIC_OAUTH']
    - 
    -+libpq_link_args = []
    -+if host_system == 'darwin'
    -+  # TODO staticlib too...
    -+  libpq_link_args += [
    -+    '-Wl,-U,___asan_poison_memory_region',
    -+    '-Wl,-U,___asan_unpoison_memory_region',
    -+  ]
    -+endif
    -+
    - # Not using both_libraries() here as
    - # 1) resource files should only be in the shared library
    - # 2) we want the .pc file to include a dependency to {pgport,common}_static for
    -@@ src/interfaces/libpq/meson.build: libpq_so = shared_library('libpq',
    -   darwin_versions: ['5', '5.' + pg_version_major.to_string()],
    -   dependencies: [frontend_shlib_code, libpq_deps],
    -   link_depends: export_file,
    --  link_args: export_fmt.format(export_file.full_path()),
    -+  link_args: [
    -+    export_fmt.format(export_file.full_path()),
    -+    libpq_link_args,
    -+  ],
    -   kwargs: default_lib_args,
    - )
    - libpq_targets += libpq_so
    -
    - ## src/interfaces/libpq/Makefile ##
    -@@ src/interfaces/libpq/Makefile: ifneq ($(PORTNAME), win32)
    - override CFLAGS += $(PTHREAD_CFLAGS)
    - endif
    - 
    -+ifeq ($(PORTNAME),darwin)
    -+# TODO staticlib too...
    -+override LDFLAGS += -Wl,-U,___asan_poison_memory_region
    -+override LDFLAGS += -Wl,-U,___asan_unpoison_memory_region
    -+endif
    -+
    - OBJS = \
    - 	$(WIN32RES) \
    - 	fe-auth-scram.o \
    -
      ## src/interfaces/libpq/fe-auth-oauth.h ##
     @@ src/interfaces/libpq/fe-auth-oauth.h: typedef struct
      	PGconn	   *conn;
    @@ src/interfaces/libpq/fe-auth-oauth.c: oauth_unsafe_debugging_enabled(void)
     + * PGoauthBearerRequestV2. (Such clients may crash or worse when speaking to
     + * libpq 18.)
     + *
    -+ * This attempts to use Valgrind and AddressSanitizer hooks, if present, to mark
    -+ * the extra members as inaccessible. For uninstrumented builds, it also munges
    -+ * request->issuer to try to crash clients that perform string operations, and
    -+ * it aborts if request->error is set.
    ++ * This attempts to use Valgrind hooks, if present, to mark the extra members as
    ++ * inaccessible. For uninstrumented builds, it also munges request->issuer to
    ++ * try to crash clients that perform string operations, and it aborts if
    ++ * request->error is set.
     + */
     +
    -+#if defined(__has_attribute) && __has_attribute(weak)
    -+void		__asan_poison_memory_region(void const volatile *addr, size_t size) __attribute__((weak));
    -+void		__asan_unpoison_memory_region(void const volatile *addr, size_t size) __attribute__((weak));
    -+#else
    -+static void (*const __asan_poison_memory_region) (void const volatile *addr, size_t size) = NULL;
    -+static void (*const __asan_unpoison_memory_region) (void const volatile *addr, size_t size) = NULL;
    -+#endif
    -+
     +#define MASK_BITS ((uintptr_t) 0x55aa55aa55aa55aa)
     +#define POISON_MASK(ptr) ((void *) (((uintptr_t) ptr) ^ MASK_BITS))
     +
    @@ src/interfaces/libpq/fe-auth-oauth.c: oauth_unsafe_debugging_enabled(void)
     +		request->issuer = POISON_MASK(request->issuer);
     +
     +		VALGRIND_MAKE_MEM_NOACCESS(base, len);
    -+		if (__asan_poison_memory_region)
    -+			__asan_poison_memory_region(base, len);
     +	}
     +	else
     +	{
    @@ src/interfaces/libpq/fe-auth-oauth.c: oauth_unsafe_debugging_enabled(void)
     +		 * original state of the memory.
     +		 */
     +		VALGRIND_MAKE_MEM_DEFINED(base, len);
    -+		if (__asan_unpoison_memory_region)
    -+			__asan_unpoison_memory_region(base, len);
     +
     +		/* Undo our mask. */
     +		request->issuer = POISON_MASK(request->issuer);
7:  3d6d0901f7e ! 5:  fbb89bcd980 WIP: Introduce third-party OAuth flow plugins?
    @@ src/test/modules/oauth_validator/meson.build: oauth_hook_client = executable('oa
        'name': 'oauth_validator',
        'sd': meson.current_source_dir(),
     @@ src/test/modules/oauth_validator/meson.build: tests += {
    -       'PYTHON': python.full_path(),
            'with_libcurl': oauth_flow_supported ? 'yes' : 'no',
            'with_python': 'yes',
    +       'cert_dir': meson.project_source_root() / 'src/test/ssl/ssl',
     +      'flow_module_path': oauth_flow.full_path(),
          },
          'deps': [oauth_hook_client],
    @@ src/test/modules/oauth_validator/Makefile: top_builddir = ../../../..
      export PYTHON
      export with_libcurl
      export with_python
    +-export cert_dir=$(top_srcdir)/src/test/ssl/ssl
    ++export cert_dir := $(top_srcdir)/src/test/ssl/ssl
     +export flow_module_path := $(abs_top_builddir)/$(subdir)/oauth_flow$(DLSUFFIX)
      
      endif
    @@ src/test/modules/oauth_validator/oauth_hook_client.c
      
     -#include "getopt_long.h"
      #include "libpq-fe.h"
    --#include "pqexpbuffer.h"
      
     -static int	handle_auth_data(PGauthData type, PGconn *conn, void *data);
     -static PostgresPollingStatusType async_cb(PGconn *conn,
    @@ src/test/modules/oauth_validator/oauth_test_common.c (new)
     +
     +#include "getopt_long.h"
     +#include "libpq-fe.h"
    -+#include "pqexpbuffer.h"
     +
     +#include "oauth_test_common.h"
     +

---

From: Zsolt Parragi <[email-masked]>
Date: Fri, 6 Mar 2026 22:44:43 +0000

For the first commits I only have a few more questions/comments about
the error messages, otherwise looks good.

> > + libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr);
> > + return 0;
> > + }
> > Shouldn't this path return -1,
>
> We could. I chose zero to try to retain the PG18 behavior, but I could
> expand this error message and set request->error instead. If that'd be
> less confusing to you as a reader, it's probably worth the change.

If this returns 0, we print out

failed to lock mutex
no OAuth flows are available (try installing the libpq-oauth package)

Which isn't ideal, as the library is there, so installing the package
wouldn't help.

+ if ((start_flow = dlsym(state->flow_module, "pg_start_oauthbearer")) == NULL)

And this path has the same issue, the library is there, so suggesting
to install libpq-oauth isn't helpful.
The more detailed message is only printed out with unsafe debugging,
without that it just returns 0.

+ appendPQExpBuffer(&conn->errorMessage,
+   "use_builtin_flow: failed to lock mutex (%d)\n",
+   lockerr);

This is after an assert, so maybe it is okay as is, but this bypasses
gettext. (or shouldn't it use "internal error:" similarly to the other
untranslated error message? and another 2 internal errors are
translated)

> (try installing the libpq-oauth package)

This isn't changed in these patches, but Is it okay to assume a
package name here? This is not a package that universally exists
everywhere, we can't even be sure that pg was installed with a package
manager. On RHEL it is called postgresql18-libs-oauth, on suse it's
part of the main libpq package. In both cases if for some internal
error it can't find/load the library, we suggest installing a package
that doesn't exist on that system.

---

From: Jacob Champion <[email-masked]>
Date: Fri, 6 Mar 2026 16:24:38 -0800

On Thu, Mar 5, 2026 at 4:57 PM Jacob Champion
<[email-masked]> wrote:
> v6-0001 through -0003 are targeted for commit next.

v6-0001 is pushed, after making another docs/comments/commit message
pass, and adding one last test to pin the correct meaning of
PGoauthBearerRequestV2.issuer for future maintainers. I'm working on
some of Zsolt's feedback before posting a new set.

--Jacob

---

From: Jacob Champion <[email-masked]>
Date: Fri, 6 Mar 2026 16:27:12 -0800

> to install libpq-oauth isn't helpful.

I'll cherry-pick some of the -1 handling backwards in the patchset to
handle this.

> gettext.

Correct. For PG18, I got the feedback that can't-happen errors in
OAuth should really remain untranslated, unless it's clear that the
user can act on them. Otherwise we're consuming translators' time for
no practical benefit.

> package name here?

No, not really, but see [1]. Any "vanilla" version of that error
message will contain the string "libpq-oauth" regardless; that's the
module's name. So package maintainers need to either patch the line if
it's not useful, or else let us know how they'd prefer to override
this -- Makefile? Configure? (Meson?) -- to improve the situation.
Christoph gave the most feedback here, so Debian has the most-greased
wheel at the moment. :D

Thanks,
--Jacob

[1] https://postgr.es/m/aAOREVWMFTuWvJ1l%40msg.df7cb.de

---

From: Jacob Champion <[email-masked]>
Date: Tue, 10 Mar 2026 09:32:30 -0700

> I'll cherry-pick some of the -1 handling backwards in the patchset to

Done in v7-0001. Some of the improvements in the WIP patch were also
cherry-picked, and I fixed a stray comment bug. -0001 and -0002 are
next up for commit.

-0003 fills out the commit message and should be generally reviewable
now. -0004 adds the ability to LD_LIBRARY_PATH your way into flow
plugin development for the v20 cycle. -0005 becomes pretty much
useless other than for testing (and it should possible to adapt those
tests to -0004's implementation at some point).

Thanks,
--Jacob
1:  90f05c5d963 < -:  ----------- libpq: Introduce PQAUTHDATA_OAUTH_BEARER_TOKEN_V2
2:  d8fbfc8f7a0 ! 1:  f5b861f7efd libpq-oauth: Use the PGoauthBearerRequestV2 API
    @@ src/interfaces/libpq/fe-auth-oauth.c: run_user_oauth_flow(PGconn *conn)
      /*
     - * Cleanup callback for the async user flow. Delegates most of its job to
     + * Cleanup callback for the async flow. Delegates most of its job to
    -  * PGoauthBearerRequestV2.cleanup(), then disconnects the altsock and frees the
    +  * PGoauthBearerRequest.cleanup(), then disconnects the altsock and frees the
       * request itself.
     + *
     + * This is called either at the end of a successful authentication, or during
    @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st
     +		|| (start_flow = dlsym(state->builtin_flow, "pg_start_oauthbearer")) == NULL)
      	{
      		/*
    - 		 * This is more of an error condition than the one above, but due to
    -@@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_state *state)
    +-		 * This is more of an error condition than the one above, but due to
    +-		 * the dlerror() threadsafety issue, lock it behind PGOAUTHDEBUG too.
    ++		 * This is more of an error condition than the one above, but the
    ++		 * cause is still locked behind PGOAUTHDEBUG due to the dlerror()
    ++		 * threadsafety issue.
    + 		 */
    + 		if (oauth_unsafe_debugging_enabled())
      			fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror());
      
      		dlclose(state->builtin_flow);
     -		return false;
    -+		return 0;
    ++		state->builtin_flow = NULL;
    ++
    ++		request->error = libpq_gettext("could not find entry point for libpq-oauth");
    ++		return -1;
      	}
      
      	/*
     @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_state *state)
    + 		/* Should not happen... but don't continue if it does. */
      		Assert(false);
      
    +-		libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr);
     -		return false;
      
      	if (!initialized)
    @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth
     +		conn->async_auth = run_oauth_flow;
     +		conn->cleanup_async_auth = cleanup_oauth_flow;
      		state->async_ctx = request_copy;
    - 	}
    - 	else if (res < 0)
    - 	{
    +-	}
    +-	else if (res < 0)
    +-	{
     -		report_user_flow_error(conn, &request);
    -+		report_flow_error(conn, &request);
    - 		goto fail;
    - 	}
    +-		goto fail;
    +-	}
     -	else if (!use_builtin_flow(conn, state))
    +-	{
    +-		libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)");
    +-		goto fail;
    ++
    ++		return true;
    + 	}
    + 
    +-	return true;
    ++	/*
    ++	 * Failure cases: either we tried to set up a flow and failed, or there
    ++	 * was no flow to try.
    ++	 */
    ++	if (res < 0)
    ++		report_flow_error(conn, &request);
     +	else
    - 	{
    - 		libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)");
    - 		goto fail;
    ++		libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)");
    + 
    + fail:
    + 	if (request.v1.cleanup)
3:  96bd5970668 = 2:  f59d6431b04 libpq-oauth: Never link against libpq's encoding functions
4:  0e7fae1e152 ! 3:  75ac7ddc120 WIP: test out poisoning
    @@ Metadata
     Author: Jacob Champion <[email-masked]>
     
      ## Commit message ##
    -    WIP: test out poisoning
    +    libpq: Poison the v2 part of a v1 Bearer request
    +
    +    The new PGoauthBearerRequestV2 API (which has similarities to the
    +    "subclass" pointer architecture in use by the backend, for Nodes)
    +    carries the risk of a developer ignoring the type of hook in use and
    +    just casting directly to the V2 struct. This will appear to work fine in
    +    19, but crash (or worse) when speaking to libpq 18.
    +
    +    However, we're in a unique position to catch this problem, because we
    +    have tight control over the struct. Add poisoning code to the v1 path
    +    which does the following:
    +
    +    - masks the v2 request->issuer pointer, to hopefully point at nonsense
    +      memory
    +    - abort()s if the v2 request->error is assigned by the hook
    +    - attempts to cover both with VALGRIND_MAKE_MEM_NOACCESS for the
    +      duration of the callback (a potential AddressSanitizer implementation
    +      is left for future work)
    +
    +    The struct is unpoisoned after the call, so we can switch back to the v2
    +    internal implementation when necessary.
    +
    +    Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com

     @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth_state *state)
    - 	return true;
    + 		libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)");
      
      fail:
     -	if (request.v1.cleanup)
-:  ----------- > 4:  7a43398c90a libpq: Allow developers to reimplement libpq-oauth
5:  fbb89bcd980 ! 5:  b3daf3140f8 WIP: Introduce third-party OAuth flow plugins?
    @@ Commit message
         This experimental commit promotes the pg_start_oauthbearer API to a
         public header (libpq-oauth.h) and adds a PGOAUTHMODULE environment
         variable that overrides the load path for the plugin, allowing users to
    -    provide their own. The libpq_oauth_init function is now optional, and
    -    will remain undocumented. (Modules that don't provide it are marked as
    -    user-defined.)
    +    provide their own. It's probably not a good solution; it provides almost
    +    nothing over LD_LIBRARY_PATH (other than not being LD_LIBRARY_PATH).
     
         This is a relatively small amount of implementation change, but
         unfortunately the tests have a large amount of code motion to be able to
    @@ src/interfaces/libpq-oauth/oauth-curl.h (deleted)
     -
     -#endif							/* OAUTH_CURL_H */
     
    - ## src/interfaces/libpq/fe-auth-oauth.h ##
    -@@ src/interfaces/libpq/fe-auth-oauth.h: typedef struct
    - 
    - 	bool		v1;
    - 	bool		builtin;
    --	void	   *builtin_flow;
    -+	void	   *flow_module;
    - } fe_oauth_state;
    - 
    - extern void pqClearOAuthToken(PGconn *conn);
    -
      ## src/interfaces/libpq/libpq-oauth.h (new) ##
     @@
     +/*-------------------------------------------------------------------------
    @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st
      		"libpq-oauth" DLSUFFIX;
      #endif
      
    --	state->builtin_flow = dlopen(module_name, RTLD_NOW | RTLD_LOCAL);
    --	if (!state->builtin_flow)
     +	/*-
     +	 * Additionally, the user may override the module path explicitly to be
     +	 * able to provide their own module, via PGOAUTHMODULE.
    @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st
     +	else
     +		state->builtin = true;
     +
    -+	state->flow_module = dlopen(module_name, RTLD_NOW | RTLD_LOCAL);
    -+	if (!state->flow_module)
    + 	state->flow_module = dlopen(module_name, RTLD_NOW | RTLD_LOCAL);
    + 	if (!state->flow_module)
      	{
      		/*
      		 * For end users, this probably isn't an error condition, it just
    @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st
     +		return -1;
      	}
      
    --	if ((init = dlsym(state->builtin_flow, "libpq_oauth_init")) == NULL
    --		|| (start_flow = dlsym(state->builtin_flow, "pg_start_oauthbearer")) == NULL)
    -+	if ((start_flow = dlsym(state->flow_module, "pg_start_oauthbearer")) == NULL)
    - 	{
    - 		/*
    - 		 * This is more of an error condition than the one above, but due to
    - 		 * the dlerror() threadsafety issue, lock it behind PGOAUTHDEBUG too.
    - 		 */
    - 		if (oauth_unsafe_debugging_enabled())
    --			fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror());
    -+			fprintf(stderr, "failed dlsym for %s: %s\n", module_name, dlerror());
    -+
    -+		dlclose(state->flow_module);
    -+		state->flow_module = NULL;
    -+
    -+		if (state->builtin)
    -+			return 0;
    - 
    --		dlclose(state->builtin_flow);
    --		return 0;
    -+		request->error = libpq_gettext("plugin entry point could not be located");
    -+		return -1;
    - 	}
    - 
    - 	/*
    -@@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re
    - 	 */
    - 
      	/*
    --	 * We need to inject necessary function pointers into the module. This
    --	 * only needs to be done once -- even if the pointers are constant,
    --	 * assigning them while another thread is executing the flows feels like
    --	 * tempting fate.
    -+	 * Our libpq-oauth.so provides a special initialization function for libpq
    + 	 * Our libpq-oauth.so provides a special initialization function for libpq
    +-	 * integration. If we don't find this, assume that a custom module is in
    +-	 * use instead.
     +	 * integration. It's not a problem if we don't find this; it just means
     +	 * that a user-defined module is being used.
      	 */
    --	if ((lockerr = pthread_mutex_lock(&init_mutex)) != 0)
    -+	init = dlsym(state->flow_module, "libpq_oauth_init");
    -+
    -+	if (!init)
    -+		state->builtin = false; /* adjust our error messages */
    -+	else
    - 	{
    --		/* Should not happen... but don't continue if it does. */
    --		Assert(false);
    -+		/*
    -+		 * We need to inject necessary function pointers into the module. This
    -+		 * only needs to be done once -- even if the pointers are constant,
    -+		 * assigning them while another thread is executing the flows feels
    -+		 * like tempting fate.
    -+		 */
    -+		if ((lockerr = pthread_mutex_lock(&init_mutex)) != 0)
    -+		{
    -+			/* Should not happen... but don't continue if it does. */
    -+			Assert(false);
    - 
    --		libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr);
    --		return 0;
    --	}
    -+			appendPQExpBuffer(&conn->errorMessage,
    -+							  "use_builtin_flow: failed to lock mutex (%d)\n",
    -+							  lockerr);
    - 
    --	if (!initialized)
    --	{
    --		init(
    -+			request->error = "";	/* satisfy report_flow_error() */
    -+			return -1;
    -+		}
    -+
    -+		if (!initialized)
    -+		{
    -+			init(
    - #ifdef ENABLE_NLS
    --			 libpq_gettext
    -+				 libpq_gettext
    - #else
    --			 NULL
    -+				 NULL
    - #endif
    --			);
    -+				);
    + 	init = dlsym(state->flow_module, "libpq_oauth_init");
    + 	if (!init)
    +@@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re
    + 		 * threadsafety issue.
    + 		 */
    + 		if (oauth_unsafe_debugging_enabled())
    +-			fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror());
    ++			fprintf(stderr, "failed dlsym for %s: %s\n", module_name, dlerror());
      
    --		initialized = true;
    --	}
    -+			initialized = true;
    -+		}
    + 		dlclose(state->flow_module);
    + 		state->flow_module = NULL;
      
    --	pthread_mutex_unlock(&init_mutex);
    -+		pthread_mutex_unlock(&init_mutex);
    -+	}
    +-		request->error = libpq_gettext("could not find entry point for libpq-oauth");
    ++		request->error = state->builtin ?
    ++			libpq_gettext("could not find entry point for libpq-oauth") :
    ++			libpq_gettext("could not find entry point for custom plugin");
    + 		return -1;
    + 	}
      
    - 	return (start_flow(conn, request) == 0) ? 1 : -1;
    - }
     @@ src/interfaces/libpq/fe-auth-oauth.c: extern int	pg_start_oauthbearer(PGconn *conn, PGoauthBearerRequestV2 *request);
      static int
      use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *request)
    @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth
      
      	if (res > 0)
      	{
    -@@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth_state *state)
    - 		conn->async_auth = run_oauth_flow;
    - 		conn->cleanup_async_auth = cleanup_oauth_flow;
    - 		state->async_ctx = request_copy;
    -+
    -+		return true;
    - 	}
    --	else if (res < 0)
    --	{
    -+
    -+	/*
    -+	 * Failure cases: either we tried to set up a flow and failed, or there
    -+	 * was no flow to try.
    -+	 */
    -+	if (res < 0)
    - 		report_flow_error(conn, &request);
    --		goto fail;
    --	}
    - 	else
    --	{
    - 		libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)");
    --		goto fail;
    --	}
    --
    --	return true;
    - 
    - fail:
    - 	do_cleanup(state, &request);

      test(
    -@@ src/test/modules/oauth_validator/t/002_client.pl: test(
    +@@ src/test/modules/oauth_validator/t/002_client.pl: $common_connstr = "$base_connstr oauth_issuer=$issuer oauth_client_id=myID";
      # Make sure the v1 hook continues to work.
      test(
      	"v1 synchronous hook can provide a token",

---

From: Zsolt Parragi <[email-masked]>
Date: Tue, 10 Mar 2026 22:10:29 +0000

1-4 looks good to me, with one question:

 Won't this cause a warning in 32 bit builds? (0x55aa55aa definitely
should work in both?)

---

From: Jacob Champion <[email-masked]>
Date: Tue, 10 Mar 2026 16:27:24 -0700

>  Won't this cause a warning in 32 bit builds? (0x55aa55aa definitely

Hopefully not -- it's an explicit rather than implicit truncation, and
we've required 64-bit integer support from our compilers for a long
time now. But I'm bad at keeping the rules in my head; did you have a
particular compiler in mind?

(The 32-bit build in Cirrus doesn't seem to mind this, though it does
mind the unused variables that I forgot to wrap in USE_VALGRIND...
will fix.)

--Jacob

---

From: Zsolt Parragi <[email-masked]>
Date: Wed, 11 Mar 2026 07:28:47 +0000

> did you have a

It worked on x86 gcc/clang where I tested it, but I was worried that
postgres supports many different platforms and could cause issues
somewhere.
But it's not an issue: after looking at it for a while I completely
forgot/ignored that the masking expression has several explicit casts,
so it is well defined in C11 and shouldn't cause warnings anywhere.

---

From: "Jonathan Gonzalez V." <[email-masked]>
Date: Fri, 13 Mar 2026 08:18:57 +0100

Hello!

> cherry-picked, and I fixed a stray comment bug. -0001 and -0002 are

While rebasing this patch[1] I notice that the test where wailing, that
was due to the following missing dependency in the test, small patch
here:

```
diff --git a/src/test/modules/oauth_validator/meson.build
b/src/test/modules/oauth_validator/meson.build
index 915603cbd29..7e52aef7829 100644
--- a/src/test/modules/oauth_validator/meson.build
+++ b/src/test/modules/oauth_validator/meson.build
@@ -97,6 +97,6 @@ tests += {
       'cert_dir': meson.project_source_root() / 'src/test/ssl/ssl',
       'flow_module_path': oauth_flow.full_path(),
     },
-    'deps': [oauth_hook_client],
+    'deps': [oauth_hook_client,oauth_flow],
   },
 }
```

Regards!
[1]
https://www.postgresql.org/message-id/flat/[email-masked]
-- 
Jonathan Gonzalez V. 
EDB: https://www.enterprisedb.com

---

From: Jacob Champion <[email-masked]>
Date: Fri, 13 Mar 2026 10:59:23 -0700

On Fri, Mar 13, 2026 at 12:24 AM Jonathan Gonzalez V.
> was due to the following missing dependency in the test, small patch

Fixed in v8, thanks.

v7-0001 and -0002 are committed, plus the removal of some additional
typedefs, plus a fix for a silly bug in the Makefile that indri
caught.

--Jacob
1:  f5b861f7efd < -:  ----------- libpq-oauth: Use the PGoauthBearerRequestV2 API
2:  f59d6431b04 < -:  ----------- libpq-oauth: Never link against libpq's encoding functions
3:  75ac7ddc120 ! 1:  16fa033f184 libpq: Poison the v2 part of a v1 Bearer request
    @@ Commit message
         The struct is unpoisoned after the call, so we can switch back to the v2
         internal implementation when necessary.
     
    +    Reviewed-by: Zsolt Parragi <[email-masked]>
         Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
     
      ## src/interfaces/libpq/fe-auth-oauth.h ##
    @@ src/interfaces/libpq/fe-auth-oauth.c: oauth_unsafe_debugging_enabled(void)
     +static void
     +poison_req_v2(PGoauthBearerRequestV2 *request, bool poison)
     +{
    ++#ifdef USE_VALGRIND
     +	void	   *const base = (char *) request + sizeof(request->v1);
     +	const size_t len = sizeof(*request) - sizeof(request->v1);
    ++#endif
     +
     +	if (poison)
     +	{
4:  7a43398c90a ! 2:  6f886914c83 libpq: Allow developers to reimplement libpq-oauth
    @@ Commit message
         provide feedback for PG20, by telling the runtime linker to find a
         different libpq-oauth. It remains undocumented for end users.
     
    +    Reviewed-by: Zsolt Parragi <[email-masked]>
         Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
     
      ## src/interfaces/libpq/fe-auth-oauth.h ##
5:  b3daf3140f8 ! 3:  651d9bdcbc3 WIP: Introduce third-party OAuth flow plugins?
    @@ Commit message
         TODO: how hard would it be to support Windows here?
     
         Reviewed-by: Zsolt Parragi <[email-masked]>
    +    Reviewed-by: Jonathan Gonzalez V. <[email-masked]>
         Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
     
      ## src/interfaces/libpq/meson.build ##
    @@ src/test/modules/oauth_validator/meson.build: tests += {
            'cert_dir': meson.project_source_root() / 'src/test/ssl/ssl',
     +      'flow_module_path': oauth_flow.full_path(),
          },
    -     'deps': [oauth_hook_client],
    +-    'deps': [oauth_hook_client],
    ++    'deps': [oauth_hook_client, oauth_flow],
        },
    + }
     
      ## src/test/modules/oauth_validator/Makefile ##
     @@ src/test/modules/oauth_validator/Makefile: PGFILEDESC = "validator - test OAuth validator module"

---

From: Chao Li <[email-masked]>
Date: Mon, 16 Mar 2026 15:33:10 +0800

> On Mar 14, 2026, at 01:59, Jacob Champion <[email-masked]> wrote:
> 
> <since-v7.diff.txt><v8-0001-libpq-Poison-the-v2-part-of-a-v1-Bearer-request.patch><v8-0002-libpq-Allow-developers-to-reimplement-libpq-oauth.patch><v8-0003-WIP-Introduce-third-party-OAuth-flow-plugins.patch>

Hi Jacob, a few comments on v8.

1 - 0001
```
+		/*
+		 * For uninstrumented builds, make sure request->error wasn't touched.
+		 */
+		if (request->error)
+		{
+			fprintf(stderr,
+					"abort! out-of-bounds write to PGoauthBearerRequest by PQAUTHDATA_OAUTH_BEARER_TOKEN hook\n");
+			abort();
+		}
```

I think this check relies on that when poisoning, request->error should be NULL. So, does it make sense to Assert(request->error==NULL) in the poison branch?

2 - 0002 Overall LGTM. A small comment is that, now use_builtin_flow() becomes a bit misleading because it may turn to non-builtin when libpq_oauth_init is not provided by the lib. So, maybe rename the function to something like try_builtin_flow()?

Best regards,

---

From: Zsolt Parragi <[email-masked]>
Date: Tue, 17 Mar 2026 06:33:15 +0000

+/*
+ * Wrapper around PGoauthBearerRequest.async() which applies poison during the
+ * callback when necessary.
+ */
+static int
+do_async(fe_oauth_state *state, PGoauthBearerRequestV2 *request)
+{
+ PGconn    *conn = state->conn;
+ int ret;

Shouldn't this return PostgresPollingStatusType, and declare ret using it?

---

From: Jacob Champion <[email-masked]>
Date: Tue, 31 Mar 2026 11:50:02 -0700

> I think this check relies on that when poisoning, request->error should be NULL. So, does it make sense to Assert(request->error==NULL) in the poison branch?

Yes, that's a good idea. Done in the committed version.

> 2 - 0002 Overall LGTM. A small comment is that, now use_builtin_flow() becomes a bit misleading because it may turn to non-builtin when libpq_oauth_init is not provided by the lib. So, maybe rename…

I think you're correct that the naming here has not caught up to where
we are, but try_builtin_flow() doesn't get us much closer in my
opinion, and I wasn't able to find something I really liked. This will
need a refactor when plugins arrive, so I'm deferring for now.

Thanks!
--Jacob

---

From: Jacob Champion <[email-masked]>
Date: Tue, 31 Mar 2026 11:51:14 -0700

> Shouldn't this return PostgresPollingStatusType, and declare ret using it?

Yes, thanks. Done in 0af4d402c.

--Jacob