Subject: Re: Refactor SASL exchange in preparation for OAuth Bearer

From: Daniel Gustafsson <[email-masked]>
Date: Wed, 28 Feb 2024 23:54:03 +0100

> On 26 Feb 2024, at 19:56, Jacob Champion <[email-masked]> wrote:

>> + * SASL_FAILED: The exchance has failed and the connection should be
> 
> s/exchance/exchange/

I rank that as one of my better typos actually. Fixed though.

>> - if (final && !done)
>> + if (final && !(status == SASL_FAILED || status == SASL_COMPLETE))
> 
> Since there's not yet a SASL_ASYNC, I wonder if this would be more
> readable if it were changed to
>    if (final && status == SASL_CONTINUE)
> to match the if condition shortly after it.

Fair point, that's more readable in this commit.

> In 0002, at the beginning of pg_SASL_init, the `password` variable now
> has an uninitialized code path. The OAuth patchset initializes it to
> NULL:

Nice catch, fixed.

--
Daniel Gustafsson

---

From: Jacob Champion <[email-masked]>
Date: Thu, 29 Feb 2024 11:58:46 -0800

> I rank that as one of my better typos actually. Fixed though.

LGTM!

Thanks,
--Jacob

---

From: Daniel Gustafsson <[email-masked]>
Date: Wed, 20 Mar 2024 15:28:10 +0100

> On 29 Feb 2024, at 20:58, Jacob Champion <[email-masked]> wrote:
> 

Thanks for review, and since Heikki marked it ready for committer I assume that
counting as a +1 as well.  Attached is a rebase on top of HEAD to get a fresh
run from the CFBot before applying this.

--
Daniel Gustafsson