From df7ae7991a7daf042729a09092b741b0cf7d14c3 Mon Sep 17 00:00:00 2001
From: Jacob Champion <jacob.champion@enterprisedb.com>
Date: Tue, 12 Mar 2024 09:44:53 -0700
Subject: [PATCH] common/jsonapi: support json_errdetail in FRONTEND

json_errdetail() now allocates its error message inside memory owned by
the JsonLexContext, so clients don't need to worry about freeing it. We
can now partially revert b44669b2ca.

Extracted from the OAuth patchset; the new destroyStringInfo() API is by
Daniel Gustafsson.
---
 src/bin/pg_verifybackup/t/005_bad_manifest.pl |   3 +-
 src/common/jsonapi.c                          | 132 +++++++++++-------
 src/common/parse_manifest.c                   |   2 +-
 src/common/stringinfo.c                       |  16 +++
 src/include/common/jsonapi.h                  |   1 +
 src/include/lib/stringinfo.h                  |  11 +-
 6 files changed, 109 insertions(+), 56 deletions(-)

diff --git a/src/bin/pg_verifybackup/t/005_bad_manifest.pl b/src/bin/pg_verifybackup/t/005_bad_manifest.pl
index e278ccea5a..e2a297930e 100644
--- a/src/bin/pg_verifybackup/t/005_bad_manifest.pl
+++ b/src/bin/pg_verifybackup/t/005_bad_manifest.pl
@@ -13,7 +13,8 @@ use Test::More;
 my $tempdir = PostgreSQL::Test::Utils::tempdir;
 
 test_bad_manifest('input string ended unexpectedly',
-	qr/could not parse backup manifest: parsing failed/, <<EOM);
+	qr/could not parse backup manifest: The input string ended unexpectedly/,
+	<<EOM);
 {
 EOM
 
diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c
index 32931ded82..d70f4267da 100644
--- a/src/common/jsonapi.c
+++ b/src/common/jsonapi.c
@@ -170,23 +170,27 @@ makeJsonLexContextCstringLen(JsonLexContext *lex, char *json,
 		lex->strval = makeStringInfo();
 		lex->flags |= JSONLEX_FREE_STRVAL;
 	}
+	lex->errormsg = NULL;
 
 	return lex;
 }
 
 /*
- * Free memory in a JsonLexContext.  There's no need for this if a *lex
- * pointer was given when the object was made and need_escapes was false,
- * or (in backend environment) a memory context delete/reset is imminent.
+ * Free memory in a JsonLexContext.
+ *
+ * There's no need for this if a *lex pointer was given when the object was
+ * made, need_escapes was false, and json_errdetail() was not called; or if (in
+ * backend environment) a memory context delete/reset is imminent.
  */
 void
 freeJsonLexContext(JsonLexContext *lex)
 {
 	if (lex->flags & JSONLEX_FREE_STRVAL)
-	{
-		pfree(lex->strval->data);
-		pfree(lex->strval);
-	}
+		destroyStringInfo(lex->strval);
+
+	if (lex->errormsg)
+		destroyStringInfo(lex->errormsg);
+
 	if (lex->flags & JSONLEX_FREE_STRUCT)
 		pfree(lex);
 }
@@ -1145,72 +1149,84 @@ report_parse_error(JsonParseContext ctx, JsonLexContext *lex)
 	return JSON_SUCCESS;		/* silence stupider compilers */
 }
 
-
-#ifndef FRONTEND
-/*
- * Extract the current token from a lexing context, for error reporting.
- */
-static char *
-extract_token(JsonLexContext *lex)
-{
-	int			toklen = lex->token_terminator - lex->token_start;
-	char	   *token = palloc(toklen + 1);
-
-	memcpy(token, lex->token_start, toklen);
-	token[toklen] = '\0';
-	return token;
-}
-
 /*
  * Construct an (already translated) detail message for a JSON error.
  *
- * Note that the error message generated by this routine may not be
- * palloc'd, making it unsafe for frontend code as there is no way to
- * know if this can be safely pfree'd or not.
+ * The returned allocation is either static or owned by the JsonLexContext and
+ * should not be freed.
  */
 char *
 json_errdetail(JsonParseErrorType error, JsonLexContext *lex)
 {
+	int			toklen = lex->token_terminator - lex->token_start;
+
+	if (lex->errormsg)
+		resetStringInfo(lex->errormsg);
+	else
+		lex->errormsg = makeStringInfo();
+
 	switch (error)
 	{
 		case JSON_SUCCESS:
 			/* fall through to the error code after switch */
 			break;
 		case JSON_ESCAPING_INVALID:
-			return psprintf(_("Escape sequence \"\\%s\" is invalid."),
-							extract_token(lex));
+			appendStringInfo(lex->errormsg,
+							 _("Escape sequence \"\\%.*s\" is invalid."),
+							 toklen, lex->token_start);
+			break;
 		case JSON_ESCAPING_REQUIRED:
-			return psprintf(_("Character with value 0x%02x must be escaped."),
-							(unsigned char) *(lex->token_terminator));
+			appendStringInfo(lex->errormsg,
+							 _("Character with value 0x%02x must be escaped."),
+							 (unsigned char) *(lex->token_terminator));
+			break;
 		case JSON_EXPECTED_END:
-			return psprintf(_("Expected end of input, but found \"%s\"."),
-							extract_token(lex));
+			appendStringInfo(lex->errormsg,
+							 _("Expected end of input, but found \"%.*s\"."),
+							 toklen, lex->token_start);
+			break;
 		case JSON_EXPECTED_ARRAY_FIRST:
-			return psprintf(_("Expected array element or \"]\", but found \"%s\"."),
-							extract_token(lex));
+			appendStringInfo(lex->errormsg,
+							 _("Expected array element or \"]\", but found \"%.*s\"."),
+							 toklen, lex->token_start);
+			break;
 		case JSON_EXPECTED_ARRAY_NEXT:
-			return psprintf(_("Expected \",\" or \"]\", but found \"%s\"."),
-							extract_token(lex));
+			appendStringInfo(lex->errormsg,
+							 _("Expected \",\" or \"]\", but found \"%.*s\"."),
+							 toklen, lex->token_start);
+			break;
 		case JSON_EXPECTED_COLON:
-			return psprintf(_("Expected \":\", but found \"%s\"."),
-							extract_token(lex));
+			appendStringInfo(lex->errormsg,
+							 _("Expected \":\", but found \"%.*s\"."),
+							 toklen, lex->token_start);
+			break;
 		case JSON_EXPECTED_JSON:
-			return psprintf(_("Expected JSON value, but found \"%s\"."),
-							extract_token(lex));
+			appendStringInfo(lex->errormsg,
+							 _("Expected JSON value, but found \"%.*s\"."),
+							 toklen, lex->token_start);
+			break;
 		case JSON_EXPECTED_MORE:
 			return _("The input string ended unexpectedly.");
 		case JSON_EXPECTED_OBJECT_FIRST:
-			return psprintf(_("Expected string or \"}\", but found \"%s\"."),
-							extract_token(lex));
+			appendStringInfo(lex->errormsg,
+							 _("Expected string or \"}\", but found \"%.*s\"."),
+							 toklen, lex->token_start);
+			break;
 		case JSON_EXPECTED_OBJECT_NEXT:
-			return psprintf(_("Expected \",\" or \"}\", but found \"%s\"."),
-							extract_token(lex));
+			appendStringInfo(lex->errormsg,
+							 _("Expected \",\" or \"}\", but found \"%.*s\"."),
+							 toklen, lex->token_start);
+			break;
 		case JSON_EXPECTED_STRING:
-			return psprintf(_("Expected string, but found \"%s\"."),
-							extract_token(lex));
+			appendStringInfo(lex->errormsg,
+							 _("Expected string, but found \"%.*s\"."),
+							 toklen, lex->token_start);
+			break;
 		case JSON_INVALID_TOKEN:
-			return psprintf(_("Token \"%s\" is invalid."),
-							extract_token(lex));
+			appendStringInfo(lex->errormsg,
+							 _("Token \"%.*s\" is invalid."),
+							 toklen, lex->token_start);
+			break;
 		case JSON_UNICODE_CODE_POINT_ZERO:
 			return _("\\u0000 cannot be converted to text.");
 		case JSON_UNICODE_ESCAPE_FORMAT:
@@ -1219,9 +1235,19 @@ json_errdetail(JsonParseErrorType error, JsonLexContext *lex)
 			/* note: this case is only reachable in frontend not backend */
 			return _("Unicode escape values cannot be used for code point values above 007F when the encoding is not UTF8.");
 		case JSON_UNICODE_UNTRANSLATABLE:
-			/* note: this case is only reachable in backend not frontend */
+
+			/*
+			 * note: this case is only reachable in backend not frontend.
+			 * #ifdef it away so the frontend doesn't try to link against
+			 * backend functionality.
+			 */
+#ifndef FRONTEND
 			return psprintf(_("Unicode escape value could not be translated to the server's encoding %s."),
 							GetDatabaseEncodingName());
+#else
+			Assert(false);
+			break;
+#endif
 		case JSON_UNICODE_HIGH_SURROGATE:
 			return _("Unicode high surrogate must not follow a high surrogate.");
 		case JSON_UNICODE_LOW_SURROGATE:
@@ -1236,7 +1262,9 @@ json_errdetail(JsonParseErrorType error, JsonLexContext *lex)
 	 * unhandled enum values.  But this needs to be here anyway to cover the
 	 * possibility of an incorrect input.
 	 */
-	elog(ERROR, "unexpected json parse error type: %d", (int) error);
-	return NULL;
+	if (lex->errormsg->len == 0)
+		appendStringInfo(lex->errormsg,
+						 "unexpected json parse error type: %d", (int) error);
+
+	return lex->errormsg->data;
 }
-#endif
diff --git a/src/common/parse_manifest.c b/src/common/parse_manifest.c
index 92a97714f3..62d93989be 100644
--- a/src/common/parse_manifest.c
+++ b/src/common/parse_manifest.c
@@ -147,7 +147,7 @@ json_parse_manifest(JsonManifestParseContext *context, char *buffer,
 	/* Run the actual JSON parser. */
 	json_error = pg_parse_json(lex, &sem);
 	if (json_error != JSON_SUCCESS)
-		json_manifest_parse_failure(context, "parsing failed");
+		json_manifest_parse_failure(context, json_errdetail(json_error, lex));
 	if (parse.state != JM_EXPECT_EOF)
 		json_manifest_parse_failure(context, "manifest ended unexpectedly");
 
diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c
index c61d5c58f3..da1f3fb5ad 100644
--- a/src/common/stringinfo.c
+++ b/src/common/stringinfo.c
@@ -350,3 +350,19 @@ enlargeStringInfo(StringInfo str, int needed)
 
 	str->maxlen = newlen;
 }
+
+/*
+ * destroyStringInfo
+ *
+ * Frees the StringInfo and its buffer (the opposite of makeStringInfo()). This
+ * must only be called on palloc'd StringInfos.
+ */
+void
+destroyStringInfo(StringInfo str)
+{
+	/* read-only StringInfos must not be destroyed */
+	Assert(str->maxlen != 0);
+
+	pfree(str->data);
+	pfree(str);
+}
diff --git a/src/include/common/jsonapi.h b/src/include/common/jsonapi.h
index 02943cdad8..86a0fc2d00 100644
--- a/src/include/common/jsonapi.h
+++ b/src/include/common/jsonapi.h
@@ -89,6 +89,7 @@ typedef struct JsonLexContext
 	int			line_number;	/* line number, starting from 1 */
 	char	   *line_start;		/* where that line starts within input */
 	StringInfo	strval;
+	StringInfo	errormsg;
 } JsonLexContext;
 
 typedef JsonParseErrorType (*json_struct_action) (void *state);
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index 2cd636b01c..3befe01177 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -87,8 +87,9 @@ typedef StringInfoData *StringInfo;
  *		to be len + 1 in size.
  *
  * To destroy a StringInfo, pfree() the data buffer, and then pfree() the
- * StringInfoData if it was palloc'd.  There's no special support for this.
- * However, if the StringInfo was initialized using initReadOnlyStringInfo()
+ * StringInfoData if it was palloc'd.  (For StringInfos created with
+ * makeStringInfo(), destroyStringInfo() is provided for this purpose.) However,
+ * if the StringInfo was initialized using initReadOnlyStringInfo()
  * then the caller will need to consider if it is safe to pfree the data
  * buffer.
  *
@@ -233,4 +234,10 @@ extern void appendBinaryStringInfoNT(StringInfo str,
  */
 extern void enlargeStringInfo(StringInfo str, int needed);
 
+/*------------------------
+ * destroyStringInfo
+ * Frees the StringInfo and its buffer (the opposite of makeStringInfo()).
+ */
+extern void destroyStringInfo(StringInfo str);
+
 #endif							/* STRINGINFO_H */
-- 
2.34.1

