Re: [PoC] Federated Authn/z with OAUTHBEARER

> On 21 Jan 2025, at 01:40, Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> wrote:
> On Mon, Jan 20, 2025 at 2:10 PM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:

>> + /* Done. Tear down the async implementation. */
>> + conn->cleanup_async_auth(conn);
>> + conn->cleanup_async_auth = NULL;
>> + Assert(conn->altsock == PGINVALID_SOCKET);
>> In pqDropConnection() we set ->altsock to NULL
>
> (I assume you mean PGINVALID_SOCKET?)

Doh, yes.

>> just to be sure rather than
>> assert that cleanup has done so. Shouldn't we be consistent in the
>> expectation and set to NULL here as well?
>
> I'm not opposed; I just figured that the following code might be a bit
> confusing:
>
> Assert(conn->altsock == PGINVALID_SOCKET);
> conn->altsock = PGINVALID_SOCKET;
>
> But I can add a comment to the assignment to try to explain. I don't
> know what the likelihood of landing code that trips that assertion is,
> but an explicit assignment would at least stop problems from
> cascading.

It is weird, but stopping the escalation of a problem seems important.

> On 21 Jan 2025, at 01:43, Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> wrote:
>

> On second thought, I can just fail the connection if this happens.

Yeah, I think that's the best option here.

--
Daniel Gustafsson