From d5c487cfea12da4f0b6d15912fe7b20000307fd3 Mon Sep 17 00:00:00 2001 From: Mark Thomas Date: Wed, 9 Nov 2022 12:39:15 +0000 Subject: [PATCH] Avoid invalid JSON in JSONErrorReportValve output --- .../catalina/valves/JsonErrorReportValve.java | 7 +- .../apache/tomcat/util/json/JSONFilter.java | 61 ++++++++++++++ .../tomcat/util/json/TestJSONFilter.java | 82 +++++++++++++++++++ webapps/docs/changelog.xml | 5 ++ 4 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 java/org/apache/tomcat/util/json/JSONFilter.java create mode 100644 test/org/apache/tomcat/util/json/TestJSONFilter.java diff --git a/java/org/apache/catalina/valves/JsonErrorReportValve.java b/java/org/apache/catalina/valves/JsonErrorReportValve.java index 8af7968d3056..2004df0c746a 100644 --- a/java/org/apache/catalina/valves/JsonErrorReportValve.java +++ b/java/org/apache/catalina/valves/JsonErrorReportValve.java @@ -22,6 +22,7 @@ import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; import org.apache.tomcat.util.ExceptionUtils; +import org.apache.tomcat.util.json.JSONFilter; import org.apache.tomcat.util.res.StringManager; /** @@ -62,9 +63,9 @@ protected void report(Request request, Response response, Throwable throwable) { } } String jsonReport = "{\n" + - " \"type\": \"" + type + "\",\n" + - " \"message\": \"" + message + "\",\n" + - " \"description\": \"" + description + "\"\n" + + " \"type\": \"" + JSONFilter.escape(type) + "\",\n" + + " \"message\": \"" + JSONFilter.escape(message) + "\",\n" + + " \"description\": \"" + JSONFilter.escape(description) + "\"\n" + "}"; try { try { diff --git a/java/org/apache/tomcat/util/json/JSONFilter.java b/java/org/apache/tomcat/util/json/JSONFilter.java new file mode 100644 index 000000000000..cb255dc41bae --- /dev/null +++ b/java/org/apache/tomcat/util/json/JSONFilter.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.json; + +/** + * Provides escaping of values so they can be included in a JSON document. + * Escaping is based on the definition of JSON found in + * RFC 8259. + */ +public class JSONFilter { + + private JSONFilter() { + // Utility class. Hide the default constructor. + } + + public static String escape(String input) { + /* + * While any character MAY be escaped, only U+0000 to U+001F (control + * characters), U+0022 (quotation mark) and U+005C (reverse solidus) + * MUST be escaped. + */ + char[] chars = input.toCharArray(); + StringBuffer escaped = null; + int lastUnescapedStart = 0; + for (int i = 0; i < chars.length; i++) { + if (chars[i] < 0x20 || chars[i] == 0x22 || chars[i] == 0x5c) { + if (escaped == null) { + escaped = new StringBuffer(chars.length + 20); + } + if (lastUnescapedStart < i) { + escaped.append(input.subSequence(lastUnescapedStart, i)); + } + lastUnescapedStart = i + 1; + escaped.append("\\u"); + escaped.append(String.format("%04X", Integer.valueOf(chars[i]))); + } + } + if (escaped == null) { + return input; + } else { + if (lastUnescapedStart < chars.length) { + escaped.append(input.subSequence(lastUnescapedStart, chars.length)); + } + return escaped.toString(); + } + } +} diff --git a/test/org/apache/tomcat/util/json/TestJSONFilter.java b/test/org/apache/tomcat/util/json/TestJSONFilter.java new file mode 100644 index 000000000000..0e064337357c --- /dev/null +++ b/test/org/apache/tomcat/util/json/TestJSONFilter.java @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.json; + +import java.util.ArrayList; +import java.util.Collection; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; + + +@RunWith(Parameterized.class) +public class TestJSONFilter { + + @Parameterized.Parameters(name = "{index}: input[{0}], output[{1}]") + public static Collection parameters() { + Collection parameterSets = new ArrayList<>(); + + // Empty + parameterSets.add(new String[] { "", "" }); + + // Must escape + parameterSets.add(new String[] { "\"", "\\u0022" }); + parameterSets.add(new String[] { "\\", "\\u005C" }); + // Sample of controls + parameterSets.add(new String[] { "\t", "\\u0009" }); + parameterSets.add(new String[] { "\n", "\\u000A" }); + parameterSets.add(new String[] { "\r", "\\u000D" }); + + // No escape + parameterSets.add(new String[] { "aaa", "aaa" }); + + // Start + parameterSets.add(new String[] { "\naaa", "\\u000Aaaa" }); + parameterSets.add(new String[] { "\n\naaa", "\\u000A\\u000Aaaa" }); + + // Middle + parameterSets.add(new String[] { "aaa\naaa", "aaa\\u000Aaaa" }); + parameterSets.add(new String[] { "aaa\n\naaa", "aaa\\u000A\\u000Aaaa" }); + + // End + parameterSets.add(new String[] { "aaa\n", "aaa\\u000A" }); + parameterSets.add(new String[] { "aaa\n\n", "aaa\\u000A\\u000A" }); + + // Start, middle and end + parameterSets.add(new String[] { "\naaa\naaa\n", "\\u000Aaaa\\u000Aaaa\\u000A" }); + parameterSets.add(new String[] { "\n\naaa\n\naaa\n\n", "\\u000A\\u000Aaaa\\u000A\\u000Aaaa\\u000A\\u000A" }); + + // Multiple + parameterSets.add(new String[] { "\n\n", "\\u000A\\u000A" }); + + return parameterSets; + } + + @Parameter(0) + public String input; + + @Parameter(1) + public String output; + + @Test + public void testStringEscaping() { + Assert.assertEquals(output, JSONFilter.escape(input));; + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index c9537bf7b488..3db1ed331c6d 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -166,6 +166,11 @@ provided error code during error page processing rather than assuming an error code of 500. (markt) + + Escape values used to construct output for the + JsonErrorReportValve to ensure that it always outputs valid + JSON. (markt) +