Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: [oauth] Stabilize the libpq-oauth ABI (and allow alternative
 implementations?)
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CAOYmi+=cd0SdEm4Pw3RE4eD=rbXex9Rkc3DsNiaDAqQti9taQQ@mail.gmail.com>
Date: Mon, 16 Mar 2026 15:33:10 +0800
Cc: "Jonathan Gonzalez V." <jonathan.abdiel@gmail.com>,
 Zsolt Parragi <zsolt.parragi@percona.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <821576A8-5958-401D-B8D1-7E4E30F5A40F@gmail.com>
References: 
 <CAOYmi+mrGg+n_X2MOLgeWcj3v_M00gR8uz_D7mM8z=dX1JYVbg@mail.gmail.com>
 <3720B2E1-0B96-4063-8D63-B5AE6AFEA159@gmail.com>
 <CAOYmi+mwzS4xfgomL1Lte+W1hRk+zahhZLnRMDJ8nYxoHUWt_w@mail.gmail.com>
 <CAOYmi+mEU_q9sr1PMmE-4rLwFN=OjyndDwFZvpsMU3RNJLrM9g@mail.gmail.com>
 <CAN4CZFPwdH_GMrappThju=t4m5n95Dse6ecTNwN=h4GpZLOMng@mail.gmail.com>
 <CAOYmi+m-hphbx=CRfGSdcrHi-jL3OrZGJMTAPOYp=OK253hBTA@mail.gmail.com>
 <CAOYmi+nnYr_Oe=KH2ZW6Ld88eZWv5P-s5ZtWOEh2KsPtMqOzbg@mail.gmail.com>
 <CAOYmi+nCg5upBVOo_UCSjMfO=YMkZXcSEsgaADKXqerr5wahZQ@mail.gmail.com>
 <CAOYmi+nRmt+K+ZVNztKFv=LuasDCCSsXLihZB6BEkVeroW8EvA@mail.gmail.com>
 <CAN4CZFOMv=0Y1PZ8uw3xgJmEqt4D1dr1-yOq9jjZYTARZp7H9Q@mail.gmail.com>
 <CAOYmi+mLaohk3FLbH9fKGmzN7yFGHs2Zefdyp31wbL1130puiA@mail.gmail.com>
 <CAOYmi+=Pr7AAdkcKXyLw3ycxcrjGKsOV2CTYVV2PKYQw9ecG0Q@mail.gmail.com>
 <be219c8a903f9c8a190c503cd5a6d084a56fd9a5.camel@gmail.com>
 <CAOYmi+=cd0SdEm4Pw3RE4eD=rbXex9Rkc3DsNiaDAqQti9taQQ@mail.gmail.com>
To: Jacob Champion <jacob.champion@enterprisedb.com>
Archived-At: 
 <https://www.postgresql.org/message-id/821576A8-5958-401D-B8D1-7E4E30F5A40F%40gmail.com>
Precedence: bulk


> On Mar 14, 2026, at 01:59, Jacob Champion =
<jacob.champion@enterprisedb.com> wrote:
>=20
> On Fri, Mar 13, 2026 at 12:24=E2=80=AFAM Jonathan Gonzalez V.
> <jonathan.abdiel@gmail.com> wrote:
>> 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:
>=20
> Fixed in v8, thanks.
>=20
> 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.
>=20
> --Jacob
> =
<since-v7.diff.txt><v8-0001-libpq-Poison-the-v2-part-of-a-v1-Bearer-reques=
t.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=3D=3DNULL) 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,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/