Re: [PoC] Federated Authn/z with OAUTHBEARER - Mailing list pgsql-hackers

> On 21 Jan 2025, at 01:40, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
> On Mon, Jan 20, 2025 at 2:10 PM Daniel Gustafsson <daniel@yesql.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.champion@enterprisedb.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