twalthr commented on code in PR #29342:
URL: https://github.com/apache/flink/pull/29342#discussion_r4145340995
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/JsonFunctionsITCase.java:
##########
@@ -1096,6 +1096,33 @@ private static List<TestSetSpec> parseJsonSpec() {
"JSON_STRING(TRY_PARSE_JSON(f2, true))",
"{\"a\":2}",
STRING()),
+ // Input is parsed from its UTF-8 bytes. A NUL or a BOM must
not be read as a
+ // UTF-16 or UTF-32 hint that turns invalid JSON into a value.
+ TestSetSpec.forFunction(
+ BuiltInFunctionDefinitions.PARSE_JSON,
+ "input bytes are always read as UTF-8")
+ .onFieldsWithData("1\u0000", "\u00001", "\uFEFF1")
+ .andDataTypes(STRING().notNull(), STRING().notNull(),
STRING().notNull())
+ .testSqlRuntimeError(
+ "PARSE_JSON(f0)",
+ TableRuntimeException.class,
+ "Failed to parse json string")
+ .testSqlRuntimeError(
+ "PARSE_JSON(f1)",
+ TableRuntimeException.class,
+ "Failed to parse json string")
+ .testSqlRuntimeError(
+ "PARSE_JSON(f2)",
+ TableRuntimeException.class,
+ "Failed to parse json string"),
Review Comment:
What is the exact error message? This should be user friendly. Is there a
hint to the charset?
##########
flink-core/src/test/java/org/apache/flink/types/variant/BinaryVariantInternalBuilderTest.java:
##########
@@ -134,6 +141,37 @@ void testParseJsonWithNonAsciiStringsAndKeys() throws
IOException {
assertThat(variant.toJson()).isEqualTo(json);
}
+ @Test
+ void testParseJsonFromUtf8Bytes() throws IOException {
+ final String json = "{\"schlüssel\":\"Grüße, 世界 🚀\",\"キー\":[\"äöü\"]}";
+
+
assertThat(BinaryVariantInternalBuilder.parseJson(json.getBytes(UTF_8), false))
+ .isEqualTo(BinaryVariantInternalBuilder.parseJson(json,
false));
+ }
+
+ private static Stream<Arguments> nonUtf8JsonBytes() {
+ return Stream.of(
+ // Charset detection would read these as UTF-16 or UTF-32, or
skip the BOM.
+ Arguments.of("trailing NUL", "1\u0000".getBytes(UTF_8)),
+ Arguments.of("leading NUL", "\u00001".getBytes(UTF_8)),
+ Arguments.of("UTF-8 BOM", "\uFEFF1".getBytes(UTF_8)),
+ Arguments.of("UTF-16BE", "{\"a\":1}".getBytes(UTF_16BE)),
+ Arguments.of("UTF-16LE", "{\"a\":1}".getBytes(UTF_16LE)),
+ Arguments.of("UTF-16 with BOM", "{\"a\":1}".getBytes(UTF_16)),
+ Arguments.of("invalid start byte", new byte[] {'"', (byte)
0xFF, '"'}),
+ Arguments.of("truncated sequence", new byte[] {'"', 'a',
(byte) 0xC3, '"'}),
+ Arguments.of(
+ "surrogate code point",
+ new byte[] {'"', (byte) 0xED, (byte) 0xA0, (byte)
0x80, '"'}));
+ }
+
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("nonUtf8JsonBytes")
+ void testParseJsonRejectsBytesThatAreNotUtf8Json(final String name, final
byte[] bytes) {
+ assertThatThrownBy(() -> BinaryVariantInternalBuilder.parseJson(bytes,
false))
+ .isInstanceOf(IOException.class);
Review Comment:
Will this be a VariantTypeException for users of Variant interface in UDFs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
