Support json_errdetail in FRONTEND code

danielgustafsson · danielgustafsson · commit d6607016c738 · 2024-03-17T23:56:15.000+01:00
Allocate memory for the error message inside memory owned by the JsonLexContext and move responsibility away from the caller for freeing it. This means that we can partially revert b44669b as this is now safe to use in FRONTEND code. The motivation for this comes from the OAuth and incremental JSON patchsets but it also adds value on its own. Author: Jacob Champion <jacob.champion@enterprisedb.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CAOYmi+mWdTd6ujtyF7MsvXvk7ToLRVG_tYAcaGbQLvf=N4KrQw@mail.gmail.com
diff --git a/src/bin/pg_verifybackup/t/005_bad_manifest.pl b/src/bin/pg_verifybackup/t/005_bad_manifest.pl
@@ -13,7 +13,8 @@
 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
@@ -161,6 +161,7 @@ makeJsonLexContextCstringLen(JsonLexContext *lex, char *json,
 	else
 		memset(lex, 0, sizeof(JsonLexContext));
 
+	lex->errormsg = NULL;
 	lex->input = lex->token_terminator = lex->line_start = json;
 	lex->line_number = 1;
 	lex->input_length = len;
@@ -175,18 +176,21 @@ makeJsonLexContextCstringLen(JsonLexContext *lex, char *json,
 }
 
 /*
- * 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,71 @@ 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 pointer should not be freed, the allocation is either static
+ * or owned by the JsonLexContext.
  */
 char *
 json_errdetail(JsonParseErrorType error, JsonLexContext *lex)
 {
+	if (lex->errormsg)
+		resetStringInfo(lex->errormsg);
+	else
+		lex->errormsg = makeStringInfo();
+
+	/*
+	 * A helper for error messages that should print the current token. The
+	 * format must contain exactly one %.*s specifier.
+	 */
+#define token_error(lex, format) \
+	appendStringInfo((lex)->errormsg, _(format), \
+					 (int) ((lex)->token_terminator - (lex)->token_start), \
+					 (lex)->token_start);
+
 	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));
+			token_error(lex, "Escape sequence \"\\%.*s\" is invalid.");
+			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));
+			token_error(lex, "Expected end of input, but found \"%.*s\".");
+			break;
 		case JSON_EXPECTED_ARRAY_FIRST:
-			return psprintf(_("Expected array element or \"]\", but found \"%s\"."),
-							extract_token(lex));
+			token_error(lex, "Expected array element or \"]\", but found \"%.*s\".");
+			break;
 		case JSON_EXPECTED_ARRAY_NEXT:
-			return psprintf(_("Expected \",\" or \"]\", but found \"%s\"."),
-							extract_token(lex));
+			token_error(lex, "Expected \",\" or \"]\", but found \"%.*s\".");
+			break;
 		case JSON_EXPECTED_COLON:
-			return psprintf(_("Expected \":\", but found \"%s\"."),
-							extract_token(lex));
+			token_error(lex, "Expected \":\", but found \"%.*s\".");
+			break;
 		case JSON_EXPECTED_JSON:
-			return psprintf(_("Expected JSON value, but found \"%s\"."),
-							extract_token(lex));
+			token_error(lex, "Expected JSON value, but found \"%.*s\".");
+			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));
+			token_error(lex, "Expected string or \"}\", but found \"%.*s\".");
+			break;
 		case JSON_EXPECTED_OBJECT_NEXT:
-			return psprintf(_("Expected \",\" or \"}\", but found \"%s\"."),
-							extract_token(lex));
+			token_error(lex, "Expected \",\" or \"}\", but found \"%.*s\".");
+			break;
 		case JSON_EXPECTED_STRING:
-			return psprintf(_("Expected string, but found \"%s\"."),
-							extract_token(lex));
+			token_error(lex, "Expected string, but found \"%.*s\".");
+			break;
 		case JSON_INVALID_TOKEN:
-			return psprintf(_("Token \"%s\" is invalid."),
-							extract_token(lex));
+			token_error(lex, "Token \"%.*s\" is invalid.");
+			break;
 		case JSON_UNICODE_CODE_POINT_ZERO:
 			return _("\\u0000 cannot be converted to text.");
 		case JSON_UNICODE_ESCAPE_FORMAT:
@@ -1219,9 +1222,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 and 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:
@@ -1230,13 +1243,17 @@ json_errdetail(JsonParseErrorType error, JsonLexContext *lex)
 			/* fall through to the error code after switch */
 			break;
 	}
+#undef token_error
 
 	/*
 	 * We don't use a default: case, so that the compiler will warn about
 	 * 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
@@ -152,7 +152,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/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);

Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,8 @@`
`13`	`13`	`my $tempdir = PostgreSQL::Test::Utils::tempdir;`
`14`	`14`
`15`	`15`	`test_bad_manifest('input string ended unexpectedly',`
`16`		`- qr/could not parse backup manifest: parsing failed/, <<EOM);`
	`16`	`+ qr/could not parse backup manifest: The input string ended unexpectedly/,`
	`17`	`+ <<EOM);`
`17`	`18`	`{`
`18`	`19`	`EOM`
`19`	`20`