This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git
commit e08ca8c5df5dee154ae1a1e6adfa2f9ce693ccf7 Author: Julian Hyde <[email protected]> AuthorDate: Fri Apr 8 08:51:12 2022 +0200 [CALCITE-5088] JsonBuilder should escape backslashes in JSON strings This would cause JsonParseException when parsing model JSON; also the cause of "[CALCITE-4893] JsonParseException happens when externalizing expressions with escape character from JSON". In RelWriterTest, add test case from [CALCITE-4893]. --- .../java/org/apache/calcite/util/JsonBuilder.java | 61 ++++++++++++++++++++-- .../org/apache/calcite/plan/RelWriterTest.java | 18 +++++++ .../java/org/apache/calcite/util/UtilTest.java | 41 +++++++++------ 3 files changed, 100 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/util/JsonBuilder.java b/core/src/main/java/org/apache/calcite/util/JsonBuilder.java index b9fdec835..8c9cd7787 100644 --- a/core/src/main/java/org/apache/calcite/util/JsonBuilder.java +++ b/core/src/main/java/org/apache/calcite/util/JsonBuilder.java @@ -32,6 +32,43 @@ import java.util.Map; * {@link String}, {@link Boolean}, {@link Long}). */ public class JsonBuilder { + /** Maps control characters (0 .. 31) to JSON escaped strings. + * Tab, newline, form feed and carriage return are mapped to '\t', '\n', + * '\f', 'r' respectively; others are mapped to '\\u00xx' for some 'xx'. */ + private static final ImmutableList<String> ESCAPED = ImmutableList.of( + "\\u0000", + "\\u0001", + "\\u0002", + "\\u0003", + "\\u0004", + "\\u0005", + "\\u0006", + "\\u0007", + "\\u0008", + "\\t", // tab, ASCII 9 x09 + "\\n", // newline, ASCII 10 x0A + "\\u000B", + "\\f", // form feed, ASCII 12 x0C + "\\r", // carriage return, ASCII 13 x0D + "\\u000E", + "\\u000F", + "\\u0010", + "\\u0011", + "\\u0012", + "\\u0013", + "\\u0014", + "\\u0015", + "\\u0016", + "\\u0017", + "\\u0018", + "\\u0019", + "\\u001A", + "\\u001B", + "\\u001C", + "\\u001D", + "\\u001E", + "\\u001F"); + /** * Creates a JSON object (represented by a {@link Map}). */ @@ -91,17 +128,31 @@ public class JsonBuilder { } else if (o instanceof List) { appendList(buf, indent, (List<?>) o); } else if (o instanceof String) { - buf.append('"') - .append( - ((String) o).replace("\"", "\\\"") - .replace("\n", "\\n")) - .append('"'); + appendString(buf, (String) o); } else { assert o instanceof Number || o instanceof Boolean; buf.append(o); } } + private void appendString(StringBuilder buf, String s) { + buf.append('"'); + final int n = s.length(); + for (int i = 0; i < n; i++) { + char c = s.charAt(i); + if (c < 32) { + buf.append(ESCAPED.get(c)); + } else if (c == '\\') { + buf.append("\\\\"); + } else if (c == '"') { + buf.append("\\\""); + } else { + buf.append(c); + } + } + buf.append('"'); + } + private void appendMap( StringBuilder buf, int indent, Map<String, @Nullable Object> map) { if (map.isEmpty()) { diff --git a/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java b/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java index b75d09f70..f2277fdc4 100644 --- a/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java +++ b/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java @@ -709,6 +709,24 @@ class RelWriterTest { + " LogicalTableScan(table=[[hr, emps]])\n")); } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-4893";>[CALCITE-4893] + * JsonParseException happens when externalizing expressions with escape + * character from JSON</a>. */ + @Test void testEscapeCharacter() { + final Function<RelBuilder, RelNode> relFn = b -> b + .scan("EMP") + .project( + b.call(new MockSqlOperatorTable.SplitFunction(), + b.field("ENAME"), b.literal("\r"))) + .build(); + final String expected = "" + + "LogicalProject($f0=[SPLIT($1, '\r')])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n"; + relFn(relFn) + .assertThatPlan(isLinux(expected)); + } + @Test void testJsonToRex() { // Test simple literal without inputs final String jsonString1 = "{\n" diff --git a/core/src/test/java/org/apache/calcite/util/UtilTest.java b/core/src/test/java/org/apache/calcite/util/UtilTest.java index ff1c05147..6e7fd73f3 100644 --- a/core/src/test/java/org/apache/calcite/util/UtilTest.java +++ b/core/src/test/java/org/apache/calcite/util/UtilTest.java @@ -1588,29 +1588,40 @@ class UtilTest { map.put("foo", 1); map.put("baz", true); map.put("bar", "can't"); + final String tricky = "string with doublequote\", singlequote', " + + "backslash\\, percent20%20, plus+, ampersand&, linefeed\n" + + ", carriage return\r, lfcr\n" + + "\r."; + map.put("tricky", tricky); List<Object> list = builder.list(); map.put("list", list); list.add(2); list.add(3); list.add(builder.list()); list.add(builder.map()); + list.add(tricky); list.add(null); map.put("nullValue", null); - assertEquals( - "{\n" - + " \"foo\": 1,\n" - + " \"baz\": true,\n" - + " \"bar\": \"can't\",\n" - + " \"list\": [\n" - + " 2,\n" - + " 3,\n" - + " [],\n" - + " {},\n" - + " null\n" - + " ],\n" - + " \"nullValue\": null\n" - + "}", - builder.toJsonString(map)); + final String expected = "{\n" + + " \"foo\": 1,\n" + + " \"baz\": true,\n" + + " \"bar\": \"can't\",\n" + + " \"tricky\": \"string with doublequote\\\", singlequote', " + + "backslash\\\\, percent20%20, plus+, ampersand&, linefeed\\n" + + ", carriage return\\r, lfcr\\n\\r.\",\n" + + " \"list\": [\n" + + " 2,\n" + + " 3,\n" + + " [],\n" + + " {},\n" + + " \"string with doublequote\\\", singlequote', backslash\\\\, " + + "percent20%20, plus+, ampersand&, linefeed\\n" + + ", carriage return\\r, lfcr\\n\\r.\",\n" + + " null\n" + + " ],\n" + + " \"nullValue\": null\n" + + "}"; + assertThat(builder.toJsonString(map), is(expected)); } @Test void testCompositeMap() {
