On Tue, Jun 22, 2021 at 10:59:37PM +0000, Jacob Champion wrote: > Hmm. I'm honestly hesitant to start splitting files apart, mostly > because json_log_and_abort() is only called from two places, and both > those places are triggered by programmer error as opposed to user > error. > > Would it make more sense to switch to an fprintf-and-abort case, to > match the approach taken by PGTHREAD_ERROR and the out-of-memory > conditions in fe-print.c? Or is there already precedent for handling > can't-happen code paths with in-band errors, through the PGconn?
Not really.. Looking more closely at that, I actually find a bit crazy the requirement for any logging within jsonapi.c just to cope with the fact that json_errdetail() and report_parse_error() just want to track down if the caller is giving some garbage or not, which should never be the case, really. So I would be tempted to eliminate this dependency to begin with. The second thing is how we should try to handle the way the error message gets allocated in json_errdetail(). libpq cannot rely on psprintf(), so I can think about two options here: - Let the caller of json_errdetail() allocate the memory area of the error message by itself, pass it down to the function. - Do the allocation within json_errdetail(), and let callers cope the case where json_errdetail() itself fails on OOM for any frontend code using it. Looking at HEAD, the OAUTH patch and the token handling, the second option looks more interesting. I have to admit that handling the token part makes the patch a bit special, but that avoids duplicating all those error messages for libpq. Please see the idea as attached. -- Michael
diff --git a/src/include/common/jsonapi.h b/src/include/common/jsonapi.h
index ec3dfce9c3..f5e25ca3e3 100644
--- a/src/include/common/jsonapi.h
+++ b/src/include/common/jsonapi.h
@@ -48,6 +48,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,
diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c
index d376ab152d..cf70a735cc 100644
--- a/src/common/jsonapi.c
+++ b/src/common/jsonapi.c
@@ -19,19 +19,22 @@
#include "common/jsonapi.h"
#include "mb/pg_wchar.h"
-
-#ifdef FRONTEND
-#include "common/logging.h"
-#else
+#ifndef FRONTEND
#include "miscadmin.h"
#endif
-#ifdef FRONTEND
-#define check_stack_depth()
-#define json_log_and_abort(...) \
- do { pg_log_fatal(__VA_ARGS__); exit(1); } while(0)
+/*
+ * In backend, we will use palloc/pfree. In frontend, use malloc, and
+ * return JSON_OUT_OF_MEMORY on out-of-memory.
+ */
+#ifndef FRONTEND
+#define STRDUP(s) pstrdup(s)
+#define ALLOC(size) palloc(size)
+#define FREE(s) pfree(s)
#else
-#define json_log_and_abort(...) elog(ERROR, __VA_ARGS__)
+#define STRDUP(s) strdup(s)
+#define ALLOC(size) malloc(size)
+#define FREE(s) free(s)
#endif
/*
@@ -291,12 +294,17 @@ parse_scalar(JsonLexContext *lex, JsonSemAction *sem)
if (lex_peek(lex) == JSON_TOKEN_STRING)
{
if (lex->strval != NULL)
- val = pstrdup(lex->strval->data);
+ {
+ val = STRDUP(lex->strval->data);
+ if (val == NULL)
+ return JSON_OUT_OF_MEMORY;
+ }
}
else
{
int len = (lex->token_terminator - lex->token_start);
+ //allocation here
val = palloc(len + 1);
memcpy(val, lex->token_start, len);
val[len] = '\0';
@@ -332,7 +340,11 @@ 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);
+ {
+ fname = STRDUP(lex->strval->data);
+ if (fname == NULL)
+ return JSON_OUT_OF_MEMORY;
+ }
result = json_lex(lex);
if (result != JSON_SUCCESS)
return result;
@@ -378,7 +390,9 @@ parse_object(JsonLexContext *lex, JsonSemAction *sem)
JsonTokenType tok;
JsonParseErrorType result;
+#ifndef FRONTEND
check_stack_depth();
+#endif
if (ostart != NULL)
(*ostart) (sem->semstate);
@@ -478,7 +492,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);
@@ -1047,76 +1063,138 @@ report_parse_error(JsonParseContext ctx, 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 state: %d", (int) ctx);
return JSON_SUCCESS; /* silence stupider compilers */
}
/*
- * Construct a detail message for a JSON error.
+ * Construct a detail message for a JSON error, palloc'd or malloc'd.
+ * note that this may return NULL on OOM in the frontend.
*/
char *
json_errdetail(JsonParseErrorType error, JsonLexContext *lex)
{
+ char *errstr;
+ char *tok = NULL;
+
+ /* allocate a string large enough to include the error */
+ errstr = ALLOC(256 * sizeof(char));
+ if (errstr == NULL)
+ return NULL;
+
+ /* Grab current token from the lexing context, if necessary */
+ switch (error)
+ {
+ case JSON_ESCAPING_INVALID:
+ case JSON_EXPECTED_ARRAY_FIRST:
+ case JSON_EXPECTED_ARRAY_NEXT:
+ case JSON_EXPECTED_COLON:
+ case JSON_EXPECTED_END:
+ case JSON_EXPECTED_JSON:
+ case JSON_EXPECTED_OBJECT_FIRST:
+ case JSON_EXPECTED_OBJECT_NEXT:
+ case JSON_EXPECTED_STRING:
+ case JSON_INVALID_TOKEN:
+ tok = extract_token(lex);
+ /* if allocating the token failed, just give up */
+ if (tok == NULL)
+ return NULL;
+ break;
+ default:
+ break;
+ }
+
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));
+ sprintf(errstr,
+ _("Escape sequence \"\\%s\" is invalid."),
+ tok);
+ break;
case JSON_ESCAPING_REQUIRED:
- return psprintf(_("Character with value 0x%02x must be escaped."),
- (unsigned char) *(lex->token_terminator));
+ sprintf(errstr,
+ _("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));
+ sprintf(errstr,
+ _("Expected end of input, but found \"%s\"."), tok);
+ break;
case JSON_EXPECTED_ARRAY_FIRST:
- return psprintf(_("Expected array element or \"]\", but found \"%s\"."),
- extract_token(lex));
+ sprintf(errstr,
+ _("Expected array element or \"]\", but found \"%s\"."),
+ tok);
+ break;
case JSON_EXPECTED_ARRAY_NEXT:
- return psprintf(_("Expected \",\" or \"]\", but found \"%s\"."),
- extract_token(lex));
+ sprintf(errstr,
+ _("Expected \",\" or \"]\", but found \"%s\"."),
+ tok);
+ break;
case JSON_EXPECTED_COLON:
- return psprintf(_("Expected \":\", but found \"%s\"."),
- extract_token(lex));
+ sprintf(errstr,
+ _("Expected \":\", but found \"%s\"."),
+ tok);
+ break;
case JSON_EXPECTED_JSON:
- return psprintf(_("Expected JSON value, but found \"%s\"."),
- extract_token(lex));
+ sprintf(errstr,
+ _("Expected JSON value, but found \"%s\"."),
+ tok);
+ break;
case JSON_EXPECTED_MORE:
- return _("The input string ended unexpectedly.");
+ sprintf(errstr,
+ _("The input string ended unexpectedly."));
+ break;
case JSON_EXPECTED_OBJECT_FIRST:
- return psprintf(_("Expected string or \"}\", but found \"%s\"."),
- extract_token(lex));
+ sprintf(errstr,
+ _("Expected string or \"}\", but found \"%s\"."),
+ tok);
+ break;
case JSON_EXPECTED_OBJECT_NEXT:
- return psprintf(_("Expected \",\" or \"}\", but found \"%s\"."),
- extract_token(lex));
+ sprintf(errstr,
+ _("Expected \",\" or \"}\", but found \"%s\"."),
+ tok);
+ break;
case JSON_EXPECTED_STRING:
- return psprintf(_("Expected string, but found \"%s\"."),
- extract_token(lex));
+ sprintf(errstr,
+ _("Expected string, but found \"%s\"."),
+ tok);
+ break;
case JSON_INVALID_TOKEN:
- return psprintf(_("Token \"%s\" is invalid."),
- extract_token(lex));
+ sprintf(errstr,
+ _("Token \"%s\" is invalid."),
+ tok);
+ break;
+ case JSON_OUT_OF_MEMORY:
+ sprintf(errstr, _("out of memory."));
+ break;
+
case JSON_UNICODE_CODE_POINT_ZERO:
- return _("\\u0000 cannot be converted to text.");
+ sprintf(errstr, _("\\u0000 cannot be converted to text."));
+ break;
case JSON_UNICODE_ESCAPE_FORMAT:
- return _("\"\\u\" must be followed by four hexadecimal digits.");
+ sprintf(errstr,
+ _("\"\\u\" must be followed by four hexadecimal digits."));
+ break;
case JSON_UNICODE_HIGH_ESCAPE:
/* 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.");
+ sprintf(errstr,
+ _("Unicode escape values cannot be used for code point values above 007F when the encoding is not UTF8."));
+ break;
case JSON_UNICODE_HIGH_SURROGATE:
- return _("Unicode high surrogate must not follow a high surrogate.");
+ sprintf(errstr,
+ _("Unicode high surrogate must not follow a high surrogate."));
+ break;
case JSON_UNICODE_LOW_SURROGATE:
- return _("Unicode low surrogate must follow a high surrogate.");
+ sprintf(errstr,
+ _("Unicode low surrogate must follow a high surrogate."));
+ break;
}
- /*
- * 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.
- */
- json_log_and_abort("unexpected json parse error type: %d", (int) error);
- return NULL; /* silence stupider compilers */
+ if (tok)
+ FREE(tok);
+ return errstr;
}
/*
@@ -1126,7 +1204,14 @@ static char *
extract_token(JsonLexContext *lex)
{
int toklen = lex->token_terminator - lex->token_start;
- char *token = palloc(toklen + 1);
+ char *token = ALLOC(toklen + 1);
+
+ /*
+ * There is not much we can do here on OOM, so just return some dummy
+ * value.
+ */
+ if (token == NULL)
+ return NULL;
memcpy(token, lex->token_start, toklen);
token[toklen] = '\0';
diff --git a/src/bin/pg_verifybackup/parse_manifest.c b/src/bin/pg_verifybackup/parse_manifest.c
index 3b13ae5b84..e31a05dc35 100644
--- a/src/bin/pg_verifybackup/parse_manifest.c
+++ b/src/bin/pg_verifybackup/parse_manifest.c
@@ -147,7 +147,10 @@ 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));
+ {
+ char *errstr = json_errdetail(json_error, lex);
+ json_manifest_parse_failure(context, errstr ? errstr : "out of memory");
+ }
if (parse.state != JM_EXPECT_EOF)
json_manifest_parse_failure(context, "manifest ended unexpectedly");
signature.asc
Description: PGP signature
