public inbox for [email protected]  

help / color / mirror / Atom feed
From: Chao Li <[email protected]>
To: Jacob Champion <[email protected]>
Cc: Jonathan Gonzalez V. <[email protected]>
Cc: Zsolt Parragi <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [oauth] Stabilize the libpq-oauth ABI (and allow alternative implementations?)
Date: Mon, 16 Mar 2026 15:33:10 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAOYmi+=cd0SdEm4Pw3RE4eD=rbXex9Rkc3DsNiaDAqQti9taQQ@mail.gmail.com>
References: <CAOYmi+mrGg+n_X2MOLgeWcj3v_M00gR8uz_D7mM8z=dX1JYVbg@mail.gmail.com>
	<[email protected]>
	<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>
	<[email protected]>
	<CAOYmi+=cd0SdEm4Pw3RE4eD=rbXex9Rkc3DsNiaDAqQti9taQQ@mail.gmail.com>



> On Mar 14, 2026, at 01:59, Jacob Champion <[email protected]> wrote:
> 
> On Fri, Mar 13, 2026 at 12:24 AM Jonathan Gonzalez V.
> <[email protected]> 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:
> 
> 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
> <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,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/









view thread (19+ messages)  latest in thread

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [oauth] Stabilize the libpq-oauth ABI (and allow alternative implementations?)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox