From b44669b2ca6a510b2c81cbe74c704d59226d729c Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 2 Jul 2021 09:35:12 +0900 Subject: [PATCH] Simplify error handing of jsonapi.c for the frontend This commit removes a dependency to the central logging facilities in the JSON parsing routines of src/common/, which existed to log errors when seeing error codes that do not match any existing values in JsonParseErrorType, which is not something that should never happen. The routine providing a detailed error message based on the error code is made backend-only, the existing code being unsafe to use in the frontend as the error message may finish by being palloc'd or point to a static string, so there is no way to know if the memory of the message should be pfree'd or not. The only user of this routine in the frontend was pg_verifybackup, that is changed to use a more generic error message on parsing failure. Note that making this code more resilient to OOM failures if used in shared libraries would require much more work as a lot of code paths still rely on palloc() & friends, but we are not sure yet if we need to go down to that. Still, removing the dependency to logging is a step toward more portability. This cleans up the handling of check_stack_depth() while on it, as it exists only in the backend. Per discussion with Jacob Champion and Tom Lane. Discussion: https://postgr.es/m/YNwL7kXwn3Cckbd6@paquier.xyz --- src/bin/pg_verifybackup/parse_manifest.c | 2 +- src/bin/pg_verifybackup/t/005_bad_manifest.pl | 2 +- src/common/jsonapi.c | 61 +++++++++---------- 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/src/bin/pg_verifybackup/parse_manifest.c b/src/bin/pg_verifybackup/parse_manifest.c index 3b13ae5b84..c7ccc78c70 100644 --- a/src/bin/pg_verifybackup/parse_manifest.c +++ b/src/bin/pg_verifybackup/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, json_errdetail(json_error, lex)); + json_manifest_parse_failure(context, "parsing failed"); 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 9f8a100a71..4f5b8f5a49 100644 --- a/src/bin/pg_verifybackup/t/005_bad_manifest.pl +++ b/src/bin/pg_verifybackup/t/005_bad_manifest.pl @@ -16,7 +16,7 @@ my $tempdir = TestLib::tempdir; test_bad_manifest( 'input string ended unexpectedly', - qr/could not parse backup manifest: The input string ended unexpectedly/, + qr/could not parse backup manifest: parsing failed/, <semstate); @@ -478,7 +469,9 @@ parse_array(JsonLexContext *lex, JsonSemAction *sem) json_struct_action aend = sem->array_end; JsonParseErrorType result; +#ifndef FRONTEND check_stack_depth(); +#endif if (astart != NULL) (*astart) (sem->semstate); @@ -1044,15 +1037,34 @@ report_parse_error(JsonParseContext ctx, JsonLexContext *lex) /* * 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. + * unhandled enum values. */ - json_log_and_abort("unexpected json parse state: %d", (int) ctx); + Assert(false); 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 a 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 safery pfree'd or not. */ char * json_errdetail(JsonParseErrorType error, JsonLexContext *lex) @@ -1115,20 +1127,7 @@ json_errdetail(JsonParseErrorType error, JsonLexContext *lex) * unhandled enum values. But this needs to be here anyway to cover the * possibility of an incorrect input. */ - json_log_and_abort("unexpected json parse error type: %d", (int) error); - return NULL; /* silence stupider compilers */ -} - -/* - * 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; + elog(ERROR, "unexpected json parse error type: %d", (int) error); + return NULL; } +#endif