Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3776.700.51.11.1\))
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER
From: Daniel Gustafsson <daniel@yesql.se>
In-Reply-To: <CAOYmi+kQFrKGmC0=Um-QryU1fvCAOC2bpjcz-4rxgLrp69mSOg@mail.gmail.com>
Date: Tue, 21 Jan 2025 10:29:22 +0100
Cc: Peter Eisentraut <peter@eisentraut.org>,
 Antonin Houska <ah@cybertec.at>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <39A09EC8-2D72-4130-8C3E-8F3D1CC69A12@yesql.se>
References: <d1b467a78e0e36ed85a09adf979d04cf124a9d4b.camel@vmware.com>
 <CAOYmi+kHXMG2Z2KR7y36Cbsop_7t-YugrpfCnOXuXz8BpfaXaw@mail.gmail.com>
 <6bde5f56-9e7a-4148-b81c-eb6532cb3651@eisentraut.org>
 <CAOYmi+=ee8H=GaGNH_gZvBE+YPtmK9eqUdXqemk=zKDnO8YXRw@mail.gmail.com>
 <CAOYmi+kLTJ1wZ6gxRbgtR52E=EyiCpmp6J3mmSvtc1a6i7sZ3Q@mail.gmail.com>
 <CAOYmi+=ceXepXOrQXsxca-u57GxM+x_q34fpyNJ8PLmbUWPwEQ@mail.gmail.com>
 <9B417838-B872-453D-BBFB-99FA957EB723@yesql.se>
 <CAOYmi+=vJQ8EUiVf-iHYBwz4-tLHLrvp8rYDoNicB2yJrBKraw@mail.gmail.com>
 <CAOYmi+=FzVg+C-pQHCwjW0qU-POHmzZaD2z3CdsACj==14H8kQ@mail.gmail.com>
 <b09bc106-32f2-4e71-8c35-4db8608776a1@eisentraut.org>
 <CAOYmi+kjPLfT7iFqNMVV44sAUJ9m10TfrSMRPUYU2OgH5j0AYg@mail.gmail.com>
 <effc7572-c7ce-4ff2-9066-91d084401e1c@eisentraut.org>
 <CAOYmi+kn0_FioLpDqctn97a_W8n90XLt_OmU1xL6ZdK8DP8ufA@mail.gmail.com>
 <54c008ad-1c23-47d8-ba6c-bed98a39c31c@eisentraut.org>
 <CAOYmi+mEkXu9Jfg9v-5z9DmBCRCMS2dt43Z=TCbgVkbrqJXxRg@mail.gmail.com>
 <06102deb-00ef-431d-a3ea-65afb246700a@eisentraut.org>
 <CAOYmi+kZAka0sdxCOBxsQc2ozEZGZKHWU_9nrPXg3sG1NJ-zJw@mail.gmail.com>
 <CE4D545A-7583-4FA9-98DD-57388CCC006F@yesql.se>
 <CAOYmi+kQFrKGmC0=Um-QryU1fvCAOC2bpjcz-4rxgLrp69mSOg@mail.gmail.com>
To: Jacob Champion <jacob.champion@enterprisedb.com>
Archived-At: <https://www.postgresql.org/message-id/39A09EC8-2D72-4130-8C3E-8F3D1CC69A12%40yesql.se>
Precedence: bulk

> On 21 Jan 2025, at 01:40, Jacob Champion =
<jacob.champion@enterprisedb.com> wrote:
> On Mon, Jan 20, 2025 at 2:10=E2=80=AFPM Daniel Gustafsson =
<daniel@yesql.se> wrote:

>> +   /* Done. Tear down the async implementation. */
>> +   conn->cleanup_async_auth(conn);
>> +   conn->cleanup_async_auth =3D NULL;
>> +   Assert(conn->altsock =3D=3D PGINVALID_SOCKET);
>> In pqDropConnection() we set ->altsock to NULL
>=20
> (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?
>=20
> I'm not opposed; I just figured that the following code might be a bit
> confusing:
>=20
>    Assert(conn->altsock =3D=3D PGINVALID_SOCKET);
>    conn->altsock =3D PGINVALID_SOCKET;
>=20
> 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:
>=20

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

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

--
Daniel Gustafsson