From a7bc2bacbac476b78a3a2673e4484653beb3453d Mon Sep 17 00:00:00 2001 From: kasiafi <30203062+kasiafi@users.noreply.github.com> Date: Wed, 26 Jan 2022 12:16:05 +0100 Subject: [PATCH] Check input end when parsing JSON Before this change, the end of input after reading a JSON value was not properly validated: - if there were remaining characters which did not parse as JSON, exception was thrown. E.g. `'["correct JSON"]and more'` failed - if there were remaining characters which formed a valid token, they were silently ignored. E.g. `'["correct JSON"]{and more'` succeeded and returned `'["correct JSON"]'` After this change, any trailing characters are caught and reported as error. --- .../io/trino/operator/scalar/TestJsonFunctions.java | 5 +++++ .../test/java/io/trino/sql/analyzer/TestAnalyzer.java | 11 +++++++++++ .../test/java/io/trino/type/TestJsonOperators.java | 5 +++++ .../java/io/trino/plugin/base/util/JsonTypeUtil.java | 9 ++++++--- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/core/trino-main/src/test/java/io/trino/operator/scalar/TestJsonFunctions.java b/core/trino-main/src/test/java/io/trino/operator/scalar/TestJsonFunctions.java index fecbdaf6f385..5d5661b225cd 100644 --- a/core/trino-main/src/test/java/io/trino/operator/scalar/TestJsonFunctions.java +++ b/core/trino-main/src/test/java/io/trino/operator/scalar/TestJsonFunctions.java @@ -278,6 +278,11 @@ public void testInvalidJsonParse() assertInvalidFunction("JSON 'INVALID'", INVALID_FUNCTION_ARGUMENT); assertInvalidFunction("JSON_PARSE('INVALID')", INVALID_FUNCTION_ARGUMENT); assertInvalidFunction("JSON_PARSE('\"x\": 1')", INVALID_FUNCTION_ARGUMENT); + assertInvalidFunction("JSON_PARSE('{}{')", INVALID_FUNCTION_ARGUMENT); + assertInvalidFunction("JSON_PARSE('{} \"a\"')", INVALID_FUNCTION_ARGUMENT); + assertInvalidFunction("JSON_PARSE('{}{abc')", INVALID_FUNCTION_ARGUMENT); + assertInvalidFunction("JSON_PARSE('{}abc')", INVALID_FUNCTION_ARGUMENT); + assertInvalidFunction("JSON_PARSE('')", INVALID_FUNCTION_ARGUMENT); } @Test diff --git a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java index 877254842631..f9d0d4824a4d 100644 --- a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java +++ b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java @@ -3254,6 +3254,17 @@ public void testLiteral() .hasErrorCode(INVALID_LITERAL); } + @Test + public void testJsonLiteral() + { + // TODO All the below should fail. Literals should be validated during analysis https://github.com/trinodb/trino/issues/10719 + analyze("SELECT JSON '{}{'"); + analyze("SELECT JSON '{} \"a\"'"); + analyze("SELECT JSON '{}{abc'"); + analyze("SELECT JSON '{}abc'"); + analyze("SELECT JSON ''"); + } + @Test public void testLambda() { diff --git a/core/trino-main/src/test/java/io/trino/type/TestJsonOperators.java b/core/trino-main/src/test/java/io/trino/type/TestJsonOperators.java index ce15d0cfb1bc..5ca52abb675b 100644 --- a/core/trino-main/src/test/java/io/trino/type/TestJsonOperators.java +++ b/core/trino-main/src/test/java/io/trino/type/TestJsonOperators.java @@ -180,6 +180,11 @@ public void testTypeConstructor() assertFunction("JSON '[null]'", JSON, "[null]"); assertFunction("JSON '[13,null,42]'", JSON, "[13,null,42]"); assertFunction("JSON '{\"x\": null}'", JSON, "{\"x\":null}"); + assertInvalidFunction("JSON '{}{'", INVALID_FUNCTION_ARGUMENT); + assertInvalidFunction("JSON '{} \"a\"'", INVALID_FUNCTION_ARGUMENT); + assertInvalidFunction("JSON '{}{abc'", INVALID_FUNCTION_ARGUMENT); + assertInvalidFunction("JSON '{}abc'", INVALID_FUNCTION_ARGUMENT); + assertInvalidFunction("JSON ''", INVALID_FUNCTION_ARGUMENT); } @Test diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonTypeUtil.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonTypeUtil.java index cc8526183d7c..92286a81b098 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonTypeUtil.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonTypeUtil.java @@ -30,6 +30,7 @@ import static com.fasterxml.jackson.core.JsonFactory.Feature.CANONICALIZE_FIELD_NAMES; import static com.fasterxml.jackson.databind.SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS; +import static com.google.common.base.Preconditions.checkState; import static io.trino.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT; import static java.lang.String.format; import static java.nio.charset.StandardCharsets.UTF_8; @@ -50,9 +51,11 @@ public static Slice jsonParse(Slice slice) try (JsonParser parser = createJsonParser(JSON_FACTORY, slice)) { SliceOutput output = new DynamicSliceOutput(slice.length()); SORTED_MAPPER.writeValue((OutputStream) output, SORTED_MAPPER.readValue(parser, Object.class)); - // nextToken() returns null if the input is parsed correctly, - // but will throw an exception if there are trailing characters. - parser.nextToken(); + // At this point, the end of input should be reached. nextToken() has three possible results: + // - null, if the end of the input was reached + // - token, if a correct JSON token is found (e.g. '{', 'null', '1') + // - exception, if there are characters which do not form a valid JSON token (e.g. 'abc') + checkState(parser.nextToken() == null, "Found characters after the expected end of input"); return output.slice(); } catch (IOException | RuntimeException e) {