From 8c4b82940efb7e0f0f33ac915d5f7969a36e3644 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Mon, 3 May 2021 15:38:26 -0700 Subject: [PATCH v2 1/5] common/jsonapi: support FRONTEND clients Based on a patch by Michael Paquier. For frontend code, use PQExpBuffer instead of StringInfo. This requires us to track allocation failures so that we can return JSON_OUT_OF_MEMORY as needed. json_errdetail() now allocates its error message inside memory owned by the JsonLexContext, so clients don't need to worry about freeing it. For convenience, the backend now has destroyJsonLexContext() to mirror other create/destroy APIs. The frontend has init/term versions of the API to handle stack-allocated JsonLexContexts. We can now partially revert b44669b2ca, now that json_errdetail() works correctly. --- src/backend/utils/adt/jsonfuncs.c | 4 +- src/bin/pg_verifybackup/parse_manifest.c | 13 +- src/bin/pg_verifybackup/t/005_bad_manifest.pl | 2 +- src/common/Makefile | 2 +- src/common/jsonapi.c | 290 +++++++++++++----- src/include/common/jsonapi.h | 47 ++- 6 files changed, 270 insertions(+), 88 deletions(-) diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 5fd54b64b5..fa39751188 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -723,9 +723,7 @@ json_object_keys(PG_FUNCTION_ARGS) pg_parse_json_or_ereport(lex, sem); /* keys are now in state->result */ - pfree(lex->strval->data); - pfree(lex->strval); - pfree(lex); + destroyJsonLexContext(lex); pfree(sem); MemoryContextSwitchTo(oldcontext); diff --git a/src/bin/pg_verifybackup/parse_manifest.c b/src/bin/pg_verifybackup/parse_manifest.c index c7ccc78c70..6cedb7435f 100644 --- a/src/bin/pg_verifybackup/parse_manifest.c +++ b/src/bin/pg_verifybackup/parse_manifest.c @@ -119,7 +119,7 @@ void json_parse_manifest(JsonManifestParseContext *context, char *buffer, size_t size) { - JsonLexContext *lex; + JsonLexContext lex = {0}; JsonParseErrorType json_error; JsonSemAction sem; JsonManifestParseState parse; @@ -129,8 +129,8 @@ json_parse_manifest(JsonManifestParseContext *context, char *buffer, parse.state = JM_EXPECT_TOPLEVEL_START; parse.saw_version_field = false; - /* Create a JSON lexing context. */ - lex = makeJsonLexContextCstringLen(buffer, size, PG_UTF8, true); + /* Initialize a JSON lexing context. */ + initJsonLexContextCstringLen(&lex, buffer, size, PG_UTF8, true); /* Set up semantic actions. */ sem.semstate = &parse; @@ -145,14 +145,17 @@ json_parse_manifest(JsonManifestParseContext *context, char *buffer, sem.scalar = json_manifest_scalar; /* Run the actual JSON parser. */ - json_error = pg_parse_json(lex, &sem); + 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"); /* Verify the manifest checksum. */ verify_manifest_checksum(&parse, buffer, size); + + /* Clean up. */ + termJsonLexContext(&lex); } /* diff --git a/src/bin/pg_verifybackup/t/005_bad_manifest.pl b/src/bin/pg_verifybackup/t/005_bad_manifest.pl index 4f5b8f5a49..9f8a100a71 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: parsing failed/, + qr/could not parse backup manifest: The input string ended unexpectedly/, <input = lex->token_terminator = lex->line_start = json; lex->line_number = 1; lex->input_length = len; lex->input_encoding = encoding; - if (need_escapes) - lex->strval = makeStringInfo(); - return lex; + lex->parse_strval = need_escapes; + if (lex->parse_strval) + { + /* + * This call can fail in FRONTEND code. We defer error handling to time + * of use (json_lex_string()) since there's no way to signal failure + * here, and we might not need to parse any strings anyway. + */ + lex->strval = createStrVal(); + } + lex->errormsg = NULL; +} + +void +termJsonLexContext(JsonLexContext *lex) +{ + static const JsonLexContext empty = {0}; + + if (lex->strval) + { +#ifdef FRONTEND + destroyPQExpBuffer(lex->strval); +#else + pfree(lex->strval->data); + pfree(lex->strval); +#endif + } + + if (lex->errormsg) + { +#ifdef FRONTEND + destroyPQExpBuffer(lex->errormsg); +#else + pfree(lex->errormsg->data); + pfree(lex->errormsg); +#endif + } + + *lex = empty; } /* @@ -217,7 +301,7 @@ json_count_array_elements(JsonLexContext *lex, int *elements) * etc, so doing this with a copy makes that safe. */ memcpy(©lex, lex, sizeof(JsonLexContext)); - copylex.strval = NULL; /* not interested in values here */ + copylex.parse_strval = false; /* not interested in values here */ copylex.lex_level++; count = 0; @@ -279,14 +363,21 @@ parse_scalar(JsonLexContext *lex, JsonSemAction *sem) /* extract the de-escaped string value, or the raw lexeme */ if (lex_peek(lex) == JSON_TOKEN_STRING) { - if (lex->strval != NULL) - val = pstrdup(lex->strval->data); + if (lex->parse_strval) + { + val = STRDUP(lex->strval->data); + if (val == NULL) + return JSON_OUT_OF_MEMORY; + } } else { int len = (lex->token_terminator - lex->token_start); - val = palloc(len + 1); + val = ALLOC(len + 1); + if (val == NULL) + return JSON_OUT_OF_MEMORY; + memcpy(val, lex->token_start, len); val[len] = '\0'; } @@ -320,8 +411,12 @@ parse_object_field(JsonLexContext *lex, JsonSemAction *sem) if (lex_peek(lex) != JSON_TOKEN_STRING) return report_parse_error(JSON_PARSE_STRING, lex); - if ((ostart != NULL || oend != NULL) && lex->strval != NULL) - fname = pstrdup(lex->strval->data); + if ((ostart != NULL || oend != NULL) && lex->parse_strval) + { + fname = STRDUP(lex->strval->data); + if (fname == NULL) + return JSON_OUT_OF_MEMORY; + } result = json_lex(lex); if (result != JSON_SUCCESS) return result; @@ -368,6 +463,10 @@ parse_object(JsonLexContext *lex, JsonSemAction *sem) JsonParseErrorType result; #ifndef FRONTEND + /* + * TODO: clients need some way to put a bound on stack growth. Parse level + * limits maybe? + */ check_stack_depth(); #endif @@ -676,8 +775,15 @@ json_lex_string(JsonLexContext *lex) int len; int hi_surrogate = -1; - if (lex->strval != NULL) - resetStringInfo(lex->strval); + if (lex->parse_strval) + { +#ifdef FRONTEND + /* make sure initialization succeeded */ + if (lex->strval == NULL) + return JSON_OUT_OF_MEMORY; +#endif + resetStrVal(lex->strval); + } Assert(lex->input_length > 0); s = lex->token_start; @@ -737,7 +843,7 @@ json_lex_string(JsonLexContext *lex) return JSON_UNICODE_ESCAPE_FORMAT; } } - if (lex->strval != NULL) + if (lex->parse_strval) { /* * Combine surrogate pairs. @@ -797,19 +903,19 @@ json_lex_string(JsonLexContext *lex) unicode_to_utf8(ch, (unsigned char *) utf8str); utf8len = pg_utf_mblen((unsigned char *) utf8str); - appendBinaryStringInfo(lex->strval, utf8str, utf8len); + appendBinaryPQExpBuffer(lex->strval, utf8str, utf8len); } else if (ch <= 0x007f) { /* The ASCII range is the same in all encodings */ - appendStringInfoChar(lex->strval, (char) ch); + appendPQExpBufferChar(lex->strval, (char) ch); } else return JSON_UNICODE_HIGH_ESCAPE; #endif /* FRONTEND */ } } - else if (lex->strval != NULL) + else if (lex->parse_strval) { if (hi_surrogate != -1) return JSON_UNICODE_LOW_SURROGATE; @@ -819,22 +925,22 @@ json_lex_string(JsonLexContext *lex) case '"': case '\\': case '/': - appendStringInfoChar(lex->strval, *s); + appendStrValChar(lex->strval, *s); break; case 'b': - appendStringInfoChar(lex->strval, '\b'); + appendStrValChar(lex->strval, '\b'); break; case 'f': - appendStringInfoChar(lex->strval, '\f'); + appendStrValChar(lex->strval, '\f'); break; case 'n': - appendStringInfoChar(lex->strval, '\n'); + appendStrValChar(lex->strval, '\n'); break; case 'r': - appendStringInfoChar(lex->strval, '\r'); + appendStrValChar(lex->strval, '\r'); break; case 't': - appendStringInfoChar(lex->strval, '\t'); + appendStrValChar(lex->strval, '\t'); break; default: /* Not a valid string escape, so signal error. */ @@ -858,12 +964,12 @@ json_lex_string(JsonLexContext *lex) } } - else if (lex->strval != NULL) + else if (lex->parse_strval) { if (hi_surrogate != -1) return JSON_UNICODE_LOW_SURROGATE; - appendStringInfoChar(lex->strval, *s); + appendStrValChar(lex->strval, *s); } } @@ -871,6 +977,11 @@ json_lex_string(JsonLexContext *lex) if (hi_surrogate != -1) return JSON_UNICODE_LOW_SURROGATE; +#ifdef FRONTEND + if (lex->parse_strval && PQExpBufferBroken(lex->strval)) + return JSON_OUT_OF_MEMORY; +#endif + /* Hooray, we found the end of the string! */ lex->prev_token_terminator = lex->token_terminator; lex->token_terminator = s + 1; @@ -1043,72 +1154,93 @@ 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 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. + * 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 (error == JSON_OUT_OF_MEMORY) + { + /* Short circuit. Allocating anything for this case is unhelpful. */ + return _("out of memory"); + } + + if (lex->errormsg) + resetStrVal(lex->errormsg); + else + lex->errormsg = createStrVal(); + 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)); + appendStrVal(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)); + appendStrVal(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)); + appendStrVal(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)); + appendStrVal(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)); + appendStrVal(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)); + appendStrVal(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)); + appendStrVal(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)); + appendStrVal(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)); + appendStrVal(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)); + appendStrVal(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)); + appendStrVal(lex->errormsg, + _("Token \"%.*s\" is invalid."), + toklen, lex->token_start); + break; + case JSON_OUT_OF_MEMORY: + /* should have been handled above; use the error path */ + break; case JSON_UNICODE_CODE_POINT_ZERO: return _("\\u0000 cannot be converted to text."); case JSON_UNICODE_ESCAPE_FORMAT: @@ -1122,12 +1254,22 @@ json_errdetail(JsonParseErrorType error, JsonLexContext *lex) return _("Unicode low surrogate must follow a high surrogate."); } - /* - * 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; -} + /* Note that lex->errormsg can be NULL in FRONTEND code. */ + if (lex->errormsg && !lex->errormsg->data[0]) + { + /* + * 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. + */ + appendStrVal(lex->errormsg, + "unexpected json parse error type: %d", (int) error); + } + +#ifdef FRONTEND + if (PQExpBufferBroken(lex->errormsg)) + return _("out of memory while constructing error description"); #endif + + return lex->errormsg->data; +} diff --git a/src/include/common/jsonapi.h b/src/include/common/jsonapi.h index ec3dfce9c3..dc71ab2cd3 100644 --- a/src/include/common/jsonapi.h +++ b/src/include/common/jsonapi.h @@ -14,8 +14,6 @@ #ifndef JSONAPI_H #define JSONAPI_H -#include "lib/stringinfo.h" - typedef enum { JSON_TOKEN_INVALID, @@ -48,6 +46,7 @@ typedef enum JSON_EXPECTED_OBJECT_NEXT, JSON_EXPECTED_STRING, JSON_INVALID_TOKEN, + JSON_OUT_OF_MEMORY, JSON_UNICODE_CODE_POINT_ZERO, JSON_UNICODE_ESCAPE_FORMAT, JSON_UNICODE_HIGH_ESCAPE, @@ -55,6 +54,17 @@ typedef enum JSON_UNICODE_LOW_SURROGATE } JsonParseErrorType; +/* + * Don't depend on the internal type header for strval; if callers need access + * then they can include the appropriate header themselves. + */ +#ifdef FRONTEND +#define StrValType PQExpBufferData +#else +#define StrValType StringInfoData +#endif + +typedef struct StrValType StrValType; /* * All the fields in this structure should be treated as read-only. @@ -81,7 +91,9 @@ typedef struct JsonLexContext int lex_level; int line_number; /* line number, starting from 1 */ char *line_start; /* where that line starts within input */ - StringInfo strval; + bool parse_strval; + StrValType *strval; /* only used if parse_strval == true */ + StrValType *errormsg; } JsonLexContext; typedef void (*json_struct_action) (void *state); @@ -141,9 +153,10 @@ extern JsonSemAction nullSemAction; */ extern JsonParseErrorType json_count_array_elements(JsonLexContext *lex, int *elements); +#ifndef FRONTEND /* - * constructor for JsonLexContext, with or without strval element. + * allocating constructor for JsonLexContext, with or without strval element. * If supplied, the strval element will contain a de-escaped version of * the lexeme. However, doing this imposes a performance penalty, so * it should be avoided if the de-escaped lexeme is not required. @@ -153,6 +166,32 @@ extern JsonLexContext *makeJsonLexContextCstringLen(char *json, int encoding, bool need_escapes); +/* + * Counterpart to makeJsonLexContextCstringLen(): clears and deallocates lex. + * The context pointer should not be used after this call. + */ +extern void destroyJsonLexContext(JsonLexContext *lex); + +#endif /* !FRONTEND */ + +/* + * stack constructor for JsonLexContext, with or without strval element. + * If supplied, the strval element will contain a de-escaped version of + * the lexeme. However, doing this imposes a performance penalty, so + * it should be avoided if the de-escaped lexeme is not required. + */ +extern void initJsonLexContextCstringLen(JsonLexContext *lex, + char *json, + int len, + int encoding, + bool need_escapes); + +/* + * Counterpart to initJsonLexContextCstringLen(): clears the contents of lex, + * but does not deallocate lex itself. + */ +extern void termJsonLexContext(JsonLexContext *lex); + /* lex one token */ extern JsonParseErrorType json_lex(JsonLexContext *lex); -- 2.25.1