From e2b7fec5c3bdf12f8339309dd6b59f061e0d12dc Mon Sep 17 00:00:00 2001 From: David Rowley Date: Tue, 21 May 2024 17:14:06 +1200 Subject: [PATCH v2 1/3] Add 'len' parameter to escape_json() --- contrib/hstore/hstore_io.c | 41 +++----- src/backend/backup/backup_manifest.c | 2 +- src/backend/commands/explain.c | 30 ++++-- src/backend/parser/parse_jsontable.c | 2 +- src/backend/utils/adt/json.c | 150 +++++++++++++++++---------- src/backend/utils/adt/jsonb.c | 2 +- src/backend/utils/adt/jsonfuncs.c | 15 +-- src/backend/utils/adt/jsonpath.c | 15 ++- src/backend/utils/error/jsonlog.c | 8 +- src/include/utils/json.h | 4 +- 10 files changed, 162 insertions(+), 107 deletions(-) diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c index 999ddad76d..374b8d05ef 100644 --- a/contrib/hstore/hstore_io.c +++ b/contrib/hstore/hstore_io.c @@ -1343,23 +1343,20 @@ hstore_to_json_loose(PG_FUNCTION_ARGS) int count = HS_COUNT(in); char *base = STRPTR(in); HEntry *entries = ARRPTR(in); - StringInfoData tmp, - dst; + StringInfoData dst; if (count == 0) PG_RETURN_TEXT_P(cstring_to_text_with_len("{}", 2)); - initStringInfo(&tmp); initStringInfo(&dst); appendStringInfoChar(&dst, '{'); for (i = 0; i < count; i++) { - resetStringInfo(&tmp); - appendBinaryStringInfo(&tmp, HSTORE_KEY(entries, base, i), - HSTORE_KEYLEN(entries, i)); - escape_json(&dst, tmp.data); + escape_json(&dst, + HSTORE_KEY(entries, base, i), + HSTORE_KEYLEN(entries, i)); appendStringInfoString(&dst, ": "); if (HSTORE_VALISNULL(entries, i)) appendStringInfoString(&dst, "null"); @@ -1372,13 +1369,13 @@ hstore_to_json_loose(PG_FUNCTION_ARGS) appendStringInfoString(&dst, "false"); else { - resetStringInfo(&tmp); - appendBinaryStringInfo(&tmp, HSTORE_VAL(entries, base, i), - HSTORE_VALLEN(entries, i)); - if (IsValidJsonNumber(tmp.data, tmp.len)) - appendBinaryStringInfo(&dst, tmp.data, tmp.len); + char *str = HSTORE_VAL(entries, base, i); + int len = HSTORE_VALLEN(entries, i); + + if (IsValidJsonNumber(str, len)) + appendBinaryStringInfo(&dst, str, len); else - escape_json(&dst, tmp.data); + escape_json(&dst, str, len); } if (i + 1 != count) @@ -1398,32 +1395,28 @@ hstore_to_json(PG_FUNCTION_ARGS) int count = HS_COUNT(in); char *base = STRPTR(in); HEntry *entries = ARRPTR(in); - StringInfoData tmp, - dst; + StringInfoData dst; if (count == 0) PG_RETURN_TEXT_P(cstring_to_text_with_len("{}", 2)); - initStringInfo(&tmp); initStringInfo(&dst); appendStringInfoChar(&dst, '{'); for (i = 0; i < count; i++) { - resetStringInfo(&tmp); - appendBinaryStringInfo(&tmp, HSTORE_KEY(entries, base, i), - HSTORE_KEYLEN(entries, i)); - escape_json(&dst, tmp.data); + escape_json(&dst, + HSTORE_KEY(entries, base, i), + HSTORE_KEYLEN(entries, i)); appendStringInfoString(&dst, ": "); if (HSTORE_VALISNULL(entries, i)) appendStringInfoString(&dst, "null"); else { - resetStringInfo(&tmp); - appendBinaryStringInfo(&tmp, HSTORE_VAL(entries, base, i), - HSTORE_VALLEN(entries, i)); - escape_json(&dst, tmp.data); + escape_json(&dst, + HSTORE_VAL(entries, base, i), + HSTORE_VALLEN(entries, i)); } if (i + 1 != count) diff --git a/src/backend/backup/backup_manifest.c b/src/backend/backup/backup_manifest.c index b360a13547..3da8da00d6 100644 --- a/src/backend/backup/backup_manifest.c +++ b/src/backend/backup/backup_manifest.c @@ -148,7 +148,7 @@ AddFileToBackupManifest(backup_manifest_info *manifest, Oid spcoid, pg_verify_mbstr(PG_UTF8, pathname, pathlen, true)) { appendStringInfoString(&buf, "{ \"Path\": "); - escape_json(&buf, pathname); + escape_json(&buf, pathname, pathlen); appendStringInfoString(&buf, ", "); } else diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 94511a5a02..399025aed3 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -4661,13 +4661,15 @@ ExplainPropertyList(const char *qlabel, List *data, ExplainState *es) case EXPLAIN_FORMAT_JSON: ExplainJSONLineEnding(es); appendStringInfoSpaces(es->str, es->indent * 2); - escape_json(es->str, qlabel); + escape_json_cstring(es->str, qlabel); appendStringInfoString(es->str, ": ["); foreach(lc, data) { + const char *str = (const char *) lfirst(lc); + if (!first) appendStringInfoString(es->str, ", "); - escape_json(es->str, (const char *) lfirst(lc)); + escape_json_cstring(es->str, str); first = false; } appendStringInfoChar(es->str, ']'); @@ -4678,10 +4680,12 @@ ExplainPropertyList(const char *qlabel, List *data, ExplainState *es) appendStringInfo(es->str, "%s: ", qlabel); foreach(lc, data) { + const char *str = (const char *) lfirst(lc); + appendStringInfoChar(es->str, '\n'); appendStringInfoSpaces(es->str, es->indent * 2 + 2); appendStringInfoString(es->str, "- "); - escape_yaml(es->str, (const char *) lfirst(lc)); + escape_yaml(es->str, str); } break; } @@ -4710,9 +4714,11 @@ ExplainPropertyListNested(const char *qlabel, List *data, ExplainState *es) appendStringInfoChar(es->str, '['); foreach(lc, data) { + const char *str = (const char *) lfirst(lc); + if (!first) appendStringInfoString(es->str, ", "); - escape_json(es->str, (const char *) lfirst(lc)); + escape_json_cstring(es->str, str); first = false; } appendStringInfoChar(es->str, ']'); @@ -4723,9 +4729,11 @@ ExplainPropertyListNested(const char *qlabel, List *data, ExplainState *es) appendStringInfoString(es->str, "- ["); foreach(lc, data) { + const char *str = (const char *) lfirst(lc); + if (!first) appendStringInfoString(es->str, ", "); - escape_yaml(es->str, (const char *) lfirst(lc)); + escape_yaml(es->str, str); first = false; } appendStringInfoChar(es->str, ']'); @@ -4775,12 +4783,12 @@ ExplainProperty(const char *qlabel, const char *unit, const char *value, case EXPLAIN_FORMAT_JSON: ExplainJSONLineEnding(es); appendStringInfoSpaces(es->str, es->indent * 2); - escape_json(es->str, qlabel); + escape_json_cstring(es->str, qlabel); appendStringInfoString(es->str, ": "); if (numeric) appendStringInfoString(es->str, value); else - escape_json(es->str, value); + escape_json_cstring(es->str, value); break; case EXPLAIN_FORMAT_YAML: @@ -4882,7 +4890,7 @@ ExplainOpenGroup(const char *objtype, const char *labelname, appendStringInfoSpaces(es->str, 2 * es->indent); if (labelname) { - escape_json(es->str, labelname); + escape_json_cstring(es->str, labelname); appendStringInfoString(es->str, ": "); } appendStringInfoChar(es->str, labeled ? '{' : '['); @@ -5090,10 +5098,10 @@ ExplainDummyGroup(const char *objtype, const char *labelname, ExplainState *es) appendStringInfoSpaces(es->str, 2 * es->indent); if (labelname) { - escape_json(es->str, labelname); + escape_json_cstring(es->str, labelname); appendStringInfoString(es->str, ": "); } - escape_json(es->str, objtype); + escape_json_cstring(es->str, objtype); break; case EXPLAIN_FORMAT_YAML: @@ -5297,7 +5305,7 @@ ExplainYAMLLineStarting(ExplainState *es) static void escape_yaml(StringInfo buf, const char *str) { - escape_json(buf, str); + escape_json_cstring(buf, str); } diff --git a/src/backend/parser/parse_jsontable.c b/src/backend/parser/parse_jsontable.c index b2519c2f32..03aac2f0cd 100644 --- a/src/backend/parser/parse_jsontable.c +++ b/src/backend/parser/parse_jsontable.c @@ -427,7 +427,7 @@ transformJsonTableColumn(JsonTableColumn *jtc, Node *contextItemExpr, initStringInfo(&path); appendStringInfoString(&path, "$."); - escape_json(&path, jtc->name); + escape_json_cstring(&path, jtc->name); pathspec = makeStringConst(path.data, -1); } diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c index d719a61f16..7934cf62fb 100644 --- a/src/backend/utils/adt/json.c +++ b/src/backend/utils/adt/json.c @@ -286,7 +286,7 @@ datum_to_json_internal(Datum val, bool is_null, StringInfo result, break; default: outputstr = OidOutputFunctionCall(outfuncoid, val); - escape_json(result, outputstr); + escape_json_cstring(result, outputstr); pfree(outputstr); break; } @@ -560,7 +560,7 @@ composite_to_json(Datum composite, StringInfo result, bool use_line_feeds) needsep = true; attname = NameStr(att->attname); - escape_json(result, attname); + escape_json_cstring(result, attname); appendStringInfoChar(result, ':'); val = heap_getattr(tuple, i + 1, tupdesc, &isnull); @@ -1391,7 +1391,6 @@ json_object(PG_FUNCTION_ARGS) count, i; text *rval; - char *v; switch (ndims) { @@ -1434,19 +1433,17 @@ json_object(PG_FUNCTION_ARGS) (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), errmsg("null value not allowed for object key"))); - v = TextDatumGetCString(in_datums[i * 2]); if (i > 0) appendStringInfoString(&result, ", "); - escape_json(&result, v); + escape_json_from_text(&result, + (text *) DatumGetPointer(in_datums[i * 2])); appendStringInfoString(&result, " : "); - pfree(v); if (in_nulls[i * 2 + 1]) appendStringInfoString(&result, "null"); else { - v = TextDatumGetCString(in_datums[i * 2 + 1]); - escape_json(&result, v); - pfree(v); + escape_json_from_text(&result, + (text *) DatumGetPointer(in_datums[i * 2 + 1])); } } @@ -1483,7 +1480,6 @@ json_object_two_arg(PG_FUNCTION_ARGS) val_count, i; text *rval; - char *v; if (nkdims > 1 || nkdims != nvdims) ereport(ERROR, @@ -1512,20 +1508,17 @@ json_object_two_arg(PG_FUNCTION_ARGS) (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), errmsg("null value not allowed for object key"))); - v = TextDatumGetCString(key_datums[i]); if (i > 0) appendStringInfoString(&result, ", "); - escape_json(&result, v); + escape_json_from_text(&result, + (text *) DatumGetPointer(key_datums[i])); + appendStringInfoString(&result, " : "); - pfree(v); if (val_nulls[i]) appendStringInfoString(&result, "null"); else - { - v = TextDatumGetCString(val_datums[i]); - escape_json(&result, v); - pfree(v); - } + escape_json_from_text(&result, + (text *) DatumGetPointer(val_datums[i])); } appendStringInfoChar(&result, '}'); @@ -1541,52 +1534,101 @@ json_object_two_arg(PG_FUNCTION_ARGS) PG_RETURN_TEXT_P(rval); } +/* + * escape_json_char + * Inline helper function for escape_json* functions + */ +static pg_attribute_always_inline void +escape_json_char(StringInfo buf, char c) +{ + switch (c) + { + case '\b': + appendStringInfoString(buf, "\\b"); + break; + case '\f': + appendStringInfoString(buf, "\\f"); + break; + case '\n': + appendStringInfoString(buf, "\\n"); + break; + case '\r': + appendStringInfoString(buf, "\\r"); + break; + case '\t': + appendStringInfoString(buf, "\\t"); + break; + case '"': + appendStringInfoString(buf, "\\\""); + break; + case '\\': + appendStringInfoString(buf, "\\\\"); + break; + default: + if ((unsigned char) c < ' ') + appendStringInfo(buf, "\\u%04x", (int) c); + else + appendStringInfoCharMacro(buf, c); + break; + } +} /* - * Produce a JSON string literal, properly escaping characters in the text. + * escape_json_cstring + * Produce a JSON string literal. Same as escape_json() except takes a + * NUL-terminated string as input. */ void -escape_json(StringInfo buf, const char *str) +escape_json_cstring(StringInfo buf, const char *str) { - const char *p; + appendStringInfoCharMacro(buf, '"'); + + for (; *str != '\0'; str++) + escape_json_char(buf, *str); appendStringInfoCharMacro(buf, '"'); - for (p = str; *p; p++) - { - switch (*p) - { - case '\b': - appendStringInfoString(buf, "\\b"); - break; - case '\f': - appendStringInfoString(buf, "\\f"); - break; - case '\n': - appendStringInfoString(buf, "\\n"); - break; - case '\r': - appendStringInfoString(buf, "\\r"); - break; - case '\t': - appendStringInfoString(buf, "\\t"); - break; - case '"': - appendStringInfoString(buf, "\\\""); - break; - case '\\': - appendStringInfoString(buf, "\\\\"); - break; - default: - if ((unsigned char) *p < ' ') - appendStringInfo(buf, "\\u%04x", (int) *p); - else - appendStringInfoCharMacro(buf, *p); - break; - } - } +} + +/* + * Produce a JSON string literal, properly escaping the possibly not + * NUL-terminated characters in 'str'. 'len' defines the number of bytes from + * 'str' to process. + */ +void +escape_json(StringInfo buf, const char *str, int len) +{ + appendStringInfoCharMacro(buf, '"'); + + for (int i = 0; i < len; i++) + escape_json_char(buf, str[i]); + appendStringInfoCharMacro(buf, '"'); } +/* + * escape_json_from_text + * Append 't' onto 'buf' and escape using escape_json. + * + * This is more efficient than calling text_to_cstring and appending the + * result as that could require an additional palloc and memcpy. + */ +void +escape_json_from_text(StringInfo buf, const text *t) +{ + /* must cast away the const, unfortunately */ + text *tunpacked = pg_detoast_datum_packed(unconstify(text *, t)); + int len = VARSIZE_ANY_EXHDR(tunpacked); + char *str; + + str = VARDATA_ANY(tunpacked); + + escape_json(buf, str, len); + + /* pfree any detoasted values */ + if (tunpacked != t) + pfree(tunpacked); +} + /* Semantic actions for key uniqueness check */ static JsonParseErrorType json_unique_object_start(void *_state) diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index e4562b3c6c..f24d8003be 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -354,7 +354,7 @@ jsonb_put_escaped_value(StringInfo out, JsonbValue *scalarVal) appendBinaryStringInfo(out, "null", 4); break; case jbvString: - escape_json(out, pnstrdup(scalarVal->val.string.val, scalarVal->val.string.len)); + escape_json(out, scalarVal->val.string.val, scalarVal->val.string.len); break; case jbvNumeric: appendStringInfoString(out, diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 83125b06a4..743d47dae6 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -3139,7 +3139,10 @@ populate_scalar(ScalarIOData *io, Oid typid, int32 typmod, JsValue *jsv, str[len] = '\0'; } else - str = json; /* string is already null-terminated */ + { + str = json; /* string is already null-terminated */ + len = strlen(str); + } /* If converting to json/jsonb, make string into valid JSON literal */ if ((typid == JSONOID || typid == JSONBOID) && @@ -3148,7 +3151,7 @@ populate_scalar(ScalarIOData *io, Oid typid, int32 typmod, JsValue *jsv, StringInfoData buf; initStringInfo(&buf); - escape_json(&buf, str); + escape_json(&buf, str, len); /* free temporary buffer */ if (str != json) pfree(str); @@ -4425,7 +4428,7 @@ sn_object_field_start(void *state, char *fname, bool isnull) * Unfortunately we don't have the quoted and escaped string any more, so * we have to re-escape it. */ - escape_json(_state->strval, fname); + escape_json_cstring(_state->strval, fname); appendStringInfoCharMacro(_state->strval, ':'); @@ -4456,7 +4459,7 @@ sn_scalar(void *state, char *token, JsonTokenType tokentype) } if (tokentype == JSON_TOKEN_STRING) - escape_json(_state->strval, token); + escape_json_cstring(_state->strval, token); else appendStringInfoString(_state->strval, token); @@ -5888,7 +5891,7 @@ transform_string_values_object_field_start(void *state, char *fname, bool isnull * Unfortunately we don't have the quoted and escaped string any more, so * we have to re-escape it. */ - escape_json(_state->strval, fname); + escape_json_cstring(_state->strval, fname); appendStringInfoCharMacro(_state->strval, ':'); return JSON_SUCCESS; @@ -5914,7 +5917,7 @@ transform_string_values_scalar(void *state, char *token, JsonTokenType tokentype { text *out = _state->action(_state->action_state, token, strlen(token)); - escape_json(_state->strval, text_to_cstring(out)); + escape_json_from_text(_state->strval, out); } else appendStringInfoString(_state->strval, token); diff --git a/src/backend/utils/adt/jsonpath.c b/src/backend/utils/adt/jsonpath.c index 11e6193e96..82d65e4d4f 100644 --- a/src/backend/utils/adt/jsonpath.c +++ b/src/backend/utils/adt/jsonpath.c @@ -523,6 +523,8 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey, { JsonPathItem elem; int i; + int32 len; + char *str; check_stack_depth(); CHECK_FOR_INTERRUPTS(); @@ -533,7 +535,8 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey, appendStringInfoString(buf, "null"); break; case jpiString: - escape_json(buf, jspGetString(v, NULL)); + str = jspGetString(v, &len); + escape_json(buf, str, len); break; case jpiNumeric: if (jspHasNext(v)) @@ -662,7 +665,8 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey, case jpiKey: if (inKey) appendStringInfoChar(buf, '.'); - escape_json(buf, jspGetString(v, NULL)); + str = jspGetString(v, &len); + escape_json(buf, str, len); break; case jpiCurrent: Assert(!inKey); @@ -674,7 +678,8 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey, break; case jpiVariable: appendStringInfoChar(buf, '$'); - escape_json(buf, jspGetString(v, NULL)); + str = jspGetString(v, &len); + escape_json(buf, str, len); break; case jpiFilter: appendStringInfoString(buf, "?("); @@ -732,7 +737,9 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey, appendStringInfoString(buf, " like_regex "); - escape_json(buf, v->content.like_regex.pattern); + escape_json(buf, + v->content.like_regex.pattern, + v->content.like_regex.patternlen); if (v->content.like_regex.flags) { diff --git a/src/backend/utils/error/jsonlog.c b/src/backend/utils/error/jsonlog.c index bd0124869d..bd21f6ef58 100644 --- a/src/backend/utils/error/jsonlog.c +++ b/src/backend/utils/error/jsonlog.c @@ -49,11 +49,11 @@ appendJSONKeyValue(StringInfo buf, const char *key, const char *value, return; appendStringInfoChar(buf, ','); - escape_json(buf, key); + escape_json_cstring(buf, key); appendStringInfoChar(buf, ':'); if (escape_value) - escape_json(buf, value); + escape_json_cstring(buf, value); else appendStringInfoString(buf, value); } @@ -143,9 +143,9 @@ write_jsonlog(ErrorData *edata) * First property does not use appendJSONKeyValue as it does not have * comma prefix. */ - escape_json(&buf, "timestamp"); + escape_json(&buf, "timestamp", strlen("timestamp")); appendStringInfoChar(&buf, ':'); - escape_json(&buf, log_time); + escape_json_cstring(&buf, log_time); /* username */ if (MyProcPort) diff --git a/src/include/utils/json.h b/src/include/utils/json.h index 6d7f1b387d..8ede8d0bd6 100644 --- a/src/include/utils/json.h +++ b/src/include/utils/json.h @@ -17,7 +17,9 @@ #include "lib/stringinfo.h" /* functions in json.c */ -extern void escape_json(StringInfo buf, const char *str); +extern void escape_json_cstring(StringInfo buf, const char *str); +extern void escape_json(StringInfo buf, const char *str, int len); +extern void escape_json_from_text(StringInfo buf, const text *t); extern char *JsonEncodeDateTime(char *buf, Datum value, Oid typid, const int *tzp); extern bool to_json_is_immutable(Oid typoid); -- 2.34.1