From 348b32ff2d3886783c173b4b6d721bfabbf9798b Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 14 Mar 2024 17:00:33 +0900 Subject: [PATCH v3 2/2] 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. --- src/include/common/jsonapi.h | 1 + src/common/jsonapi.c | 121 ++++++++++-------- src/common/parse_manifest.c | 2 +- src/bin/pg_verifybackup/t/005_bad_manifest.pl | 3 +- 4 files changed, 73 insertions(+), 54 deletions(-) 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/common/jsonapi.c b/src/common/jsonapi.c index 32931ded82..ec58109837 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,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 allocation is either static or owned by the JsonLexContext and + * should not be freed. */ 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. 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 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 index 3f6d1dfd3d..40ec3b4f58 100644 --- 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/bin/pg_verifybackup/t/005_bad_manifest.pl b/src/bin/pg_verifybackup/t/005_bad_manifest.pl index 77fdfbb9d0..2068babac5 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/, <