> On 21 Jan 2025, at 17:46, Jacob Champion <jacob.champ...@enterprisedb.com>
> wrote:
> Done that way in v43.
I've spent some time staring at, and testing, 0001, 0002 and 0003 with the
intent of getting them in to pave the way for the end goal of getting 0004 in.
In general I would say they are ready, I only have a small nitpick on 0002:
+ conn->allowed_sasl_mechs[0] = &pg_scram_mech;
I'm not a huge fan of this hardcoding in fill_allowed_sasl_mechs(). It's true
that we only have one as of this patch, but we might as well plan a little for
the future maintainability. I took a quick stab in the attached.
On top of that I just re-arranged a comment to, IMHO, better match the style in
the rest of the file.
Unless there are objections I aim at committing these patches reasonably soon
to lower the barrier for getting OAuth support committed.
--
Daniel Gustafsson
commit 73f6e943711f9c158a0f1b32fcc78f210d767083
Author: Daniel Gustafsson <dan...@yesql.se>
Date: Mon Jan 27 15:13:58 2025 +0100
nitpickerying
diff --git a/src/interfaces/libpq/fe-connect.c
b/src/interfaces/libpq/fe-connect.c
index 85ebf9f6d87..e390e428284 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -396,6 +396,25 @@ static const PQEnvironmentOption EnvironmentOptions[] =
}
};
+typedef struct SupportedSASLMech
+{
+ const char *name;
+ const pg_fe_sasl_mech *mech;
+} SupportedSASLMech;
+
+#define SASL_MECHANISM_COUNT 1
+
+static SupportedSASLMech supported_sasl_mech[] =
+{
+ {
+ "SCRAM", &pg_scram_mech
+ },
+ {
+ NULL, NULL
+ }
+};
+
+
/* The connection URI must start with either of the following designators: */
static const char uri_designator[] = "postgresql://";
static const char short_uri_designator[] = "postgres://";
@@ -512,8 +531,8 @@ pqDropConnection(PGconn *conn, bool flushInput)
conn->cleanup_async_auth = NULL;
}
conn->async_auth = NULL;
- conn->altsock = PGINVALID_SOCKET; /* cleanup_async_auth() should
have
-
* done this, but make sure. */
+ /* cleanup_async_auth() should have done this, but make sure */
+ conn->altsock = PGINVALID_SOCKET;
#ifdef ENABLE_GSS
{
OM_uint32 min_s;
@@ -1144,14 +1163,14 @@ fill_allowed_sasl_mechs(PGconn *conn)
*
* To add a new mechanism to require_auth,
* - update the length of conn->allowed_sasl_mechs,
- * - add the new pg_fe_sasl_mech pointer to this function, and
* - handle the new mechanism name in the require_auth portion of
* pqConnectOptions2(), below.
*/
- StaticAssertDecl(lengthof(conn->allowed_sasl_mechs) == 1,
- "fill_allowed_sasl_mechs() must be
updated when resizing conn->allowed_sasl_mechs[]");
+ StaticAssertDecl(lengthof(conn->allowed_sasl_mechs) ==
SASL_MECHANISM_COUNT,
+ "conn->allowed_sasl_mechs[] is not
sufficiently large for holding all supported SASL mechanisms");
- conn->allowed_sasl_mechs[0] = &pg_scram_mech;
+ for (int i = 0; i < SASL_MECHANISM_COUNT; i++)
+ conn->allowed_sasl_mechs[i] = supported_sasl_mech[i].mech;
}
/*
@@ -1506,8 +1525,7 @@ pqConnectOptions2(PGconn *conn)
* Next group: SASL mechanisms. All of these use the
same request
* codes, so the list of allowed mechanisms is tracked
separately.
*
- * fill_allowed_sasl_mechs() must be updated when
adding a new
- * mechanism here!
+ * supported_sasl_mech must contain all mechanism
handled here.
*/
else if (strcmp(method, "scram-sha-256") == 0)
{
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index a2644a2e653..89d738efdc1 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2801,6 +2801,7 @@ Subscription
SubscriptionInfo
SubscriptionRelState
SummarizerReadLocalXLogPrivate
+SupportedSASLMech
SupportRequestCost
SupportRequestIndexCondition
SupportRequestOptimizeWindowClause
