> 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/






Reply via email to