v30-0004-Review-comments.patch

From 0b522f3117d4dc827a6cf949687113a7a019951c Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Thu, 28 Mar 2024 21:59:02 +0100
Subject: [PATCH v30 4/5] Review comments

Fixes and tidy-ups following a review of v21, a few items
are (listed in no specific order):

* Implement a version check for libcurl in autoconf, the equivalent
  check for Meson is still a TODO. [ed: moved to an earlier commit]
* Address a few TODOs in the code
* libpq JSON support memory management fixups [ed: moved to an earlier
  commit]
---
 src/interfaces/libpq/fe-auth-oauth-curl.c | 36 ++++++++++++-----------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/src/interfaces/libpq/fe-auth-oauth-curl.c b/src/interfaces/libpq/fe-auth-oauth-curl.c
index 8370d82660..35c9a30ef4 100644
--- a/src/interfaces/libpq/fe-auth-oauth-curl.c
+++ b/src/interfaces/libpq/fe-auth-oauth-curl.c
@@ -1852,32 +1852,34 @@ finish_token_request(struct async_ctx *actx, struct token *tok)
 	CHECK_GETINFO(actx, CURLINFO_RESPONSE_CODE, &response_code, return false);
 
 	/*
-	 * Per RFC 6749, Section 5, a successful response uses 200 OK. An error
-	 * response uses either 400 Bad Request or 401 Unauthorized.
-	 *
-	 * TODO: there are references online to 403 appearing in the wild...
+	 * Per RFC 6749, Section 5, a successful response uses 200 OK.
 	 */
-	if (response_code != 200
-		&& response_code != 400
-		 /* && response_code != 401 TODO */ )
+	if (response_code == 200)
 	{
-		actx_error(actx, "unexpected response code %ld", response_code);
-		return false;
+		actx->errctx = "failed to parse access token response";
+		if (!parse_access_token(actx, tok))
+			return false;		/* error message already set */
+
+		return true;
 	}
 
 	/*
-	 * Pull the fields we care about from the document.
+	 * An error response uses either 400 Bad Request or 401 Unauthorized.
+	 * There are references online to implementations using 403 for error
+	 * return which would violate the specification. For now we stick to the
+	 * specification but we might have to revisit this.
 	 */
-	if (response_code == 200)
+	if (response_code == 400 || response_code == 401)
 	{
-		actx->errctx = "failed to parse access token response";
-		if (!parse_access_token(actx, tok))
-			return false;		/* error message already set */
+		if (!parse_token_error(actx, &tok->err))
+			return false;
+
+		return true;
 	}
-	else if (!parse_token_error(actx, &tok->err))
-		return false;
 
-	return true;
+	/* Any other response codes are considered invalid */
+	actx_error(actx, "unexpected response code %ld", response_code);
+	return false;
 }
 
 /*
-- 
2.34.1