v48-0002-fixup-Add-OAUTHBEARER-SASL-mechanism.patch

From 9171989a75e364b3977e42c33e37bc983c2d000c Mon Sep 17 00:00:00 2001
From: Jacob Champion <jacob.champion@enterprisedb.com>
Date: Fri, 7 Feb 2025 14:23:40 -0800
Subject: [PATCH v48 2/8] fixup! Add OAUTHBEARER SASL mechanism

---
 doc/src/sgml/libpq.sgml                   |  6 +++---
 src/backend/libpq/auth-oauth.c            |  5 ++---
 src/interfaces/libpq/fe-auth-oauth-curl.c | 26 +++++++++++++++--------
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index ddfc2a27c50..3ee0a31e6b7 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -10255,9 +10255,9 @@ typedef struct _PGpromptOAuthDevice
          example, by displaying a QR code). The URL and user code should still
          be displayed to the end user in this case, because the code will be
          manually confirmed by the provider, and the URL lets users continue
-         even if they can't use the non-textual method. Review the RFC's
-         <ulink url="https://datatracker.ietf.org/doc/html/rfc8628#section-3.3.1">notes
-         on non-textual verification</ulink>.
+         even if they can't use the non-textual method. For more information,
+         see section 3.3.1 in
+         <ulink url="https://datatracker.ietf.org/doc/html/rfc8628#section-3.3.1">RFC 8628</ulink>.
         </para>
        </listitem>
       </varlistentry>
diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index d910cbcb161..aa16977c643 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -490,9 +490,8 @@ generate_error_response(struct oauth_ctx *ctx, char **output, int *outputlen)
 	/*
 	 * The admin needs to set an issuer and scope for OAuth to work. There's
 	 * not really a way to hide this from the user, either, because we can't
-	 * choose a "default" issuer, so be honest in the failure message.
-	 *
-	 * TODO: see if there's a better place to fail, earlier than this.
+	 * choose a "default" issuer, so be honest in the failure message. (In
+	 * practice such configurations are rejected during HBA parsing.)
 	 */
 	if (!ctx->issuer || !ctx->scope)
 		ereport(FATAL,
diff --git a/src/interfaces/libpq/fe-auth-oauth-curl.c b/src/interfaces/libpq/fe-auth-oauth-curl.c
index 96c5096e4ca..2179bb89800 100644
--- a/src/interfaces/libpq/fe-auth-oauth-curl.c
+++ b/src/interfaces/libpq/fe-auth-oauth-curl.c
@@ -225,13 +225,14 @@ static void
 free_async_ctx(PGconn *conn, struct async_ctx *actx)
 {
 	/*
-	 * TODO: in general, none of the error cases below should ever happen if
-	 * we have no bugs above. But if we do hit them, surfacing those errors
-	 * somehow might be the only way to have a chance to debug them. What's
-	 * the best way to do that? Assertions? Spraying messages on stderr?
-	 * Bubbling an error code to the top? Appending to the connection's error
-	 * message only helps if the bug caused a connection failure; otherwise
-	 * it'll be buried...
+	 * In general, none of the error cases below should ever happen if we have
+	 * no bugs above. But if we do hit them, surfacing those errors somehow
+	 * might be the only way to have a chance to debug them.
+	 *
+	 * TODO: At some point it'd be nice to have a standard way to warn about
+	 * teardown failures. Appending to the connection's error message only
+	 * helps if the bug caused a connection failure; otherwise it'll be
+	 * buried...
 	 */
 
 	if (actx->curlm && actx->curl)
@@ -876,7 +877,7 @@ parse_json_number(const char *s)
 		 * Either the lexer screwed up or our assumption above isn't true, and
 		 * either way a developer needs to take a look.
 		 */
-		Assert(cnt == 1);
+		Assert(false);
 		return 0;
 	}
 
@@ -1121,6 +1122,7 @@ setup_multiplexer(struct async_ctx *actx)
 	actx->mux = kqueue();
 	if (actx->mux < 0)
 	{
+		/*- translator: the term "kqueue" (kernel queue) should not be translated */
 		actx_error(actx, "failed to create kqueue: %m");
 		return false;
 	}
@@ -1486,7 +1488,7 @@ debug_callback(CURL *handle, curl_infotype type, char *data, size_t size,
 
 		if (!printed_prefix)
 		{
-			appendPQExpBuffer(&buf, "%s ", prefix);
+			appendPQExpBuffer(&buf, "[libcurl] %s ", prefix);
 			printed_prefix = true;
 		}
 
@@ -1570,6 +1572,12 @@ setup_curl_handles(struct async_ctx *actx)
 	 * NB: If libcurl is not built against a friendly DNS resolver (c-ares or
 	 * threaded), setting this option prevents DNS lookups from timing out
 	 * correctly. We warn about this situation at configure time.
+	 *
+	 * TODO: Perhaps there's a clever way to warn the user about synchronous
+	 * DNS at runtime too? It's not immediately clear how to do that in a
+	 * helpful way: for many standard single-threaded use cases, the user
+	 * might not care at all, so spraying warnings to stderr would probably do
+	 * more harm than good.
 	 */
 	CHECK_SETOPT(actx, CURLOPT_NOSIGNAL, 1L, return false);
 
-- 
2.34.1