From 9d6721fa687510c99ce661f1155cf92753e20ab4 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 17 Mar 2026 13:52:27 +0100 Subject: [PATCH] WW-5618 feat(json): add configurable limits to JSON plugin Add configurable limits to the JSON plugin to prevent denial-of-service attacks via malicious payloads (deeply nested objects, huge arrays, long strings). Changes: - Extract JSONReader interface from class, create StrutsJSONReader impl with maxElements, maxDepth, maxStringLength, maxKeyLength enforcement - Rename DefaultJSONWriter to StrutsJSONWriter (Struts* naming convention) - Add JSONBeanSelectionProvider for bean aliasing via constants - Update JSONUtil with @Inject for reader/writer, add instance deserializeInput() with maxLength check, deprecate static deserialize() - Wire limits into JSONInterceptor with @Inject from constants - Register beans and defaults in struts-plugin.xml Default limits: 10K elements, 64 depth, 2MB length, 256KB strings, 512 keys. All configurable via struts.xml constants or per-action interceptor params. Co-Authored-By: Claude Opus 4.6 --- .../json/JSONBeanSelectionProvider.java | 35 ++ .../apache/struts2/json/JSONConstants.java | 6 + .../apache/struts2/json/JSONInterceptor.java | 53 ++- .../org/apache/struts2/json/JSONReader.java | 282 +---------- .../org/apache/struts2/json/JSONUtil.java | 50 +- .../apache/struts2/json/StrutsJSONReader.java | 338 +++++++++++++ ...tJSONWriter.java => StrutsJSONWriter.java} | 4 +- .../config/entities/JSONConstantConfig.java | 64 +++ .../json/src/main/resources/struts-plugin.xml | 16 +- .../org/apache/struts2/json/JSONEnumTest.java | 8 +- .../struts2/json/JSONInterceptorTest.java | 94 +++- .../struts2/json/JSONPopulatorTest.java | 17 +- .../apache/struts2/json/JSONReaderTest.java | 2 +- .../apache/struts2/json/JSONResultTest.java | 48 +- .../org/apache/struts2/json/JSONUtilTest.java | 6 +- .../struts2/json/StrutsJSONReaderTest.java | 159 +++++++ ...terTest.java => StrutsJSONWriterTest.java} | 47 +- .../struts2/json/jsonwriter-write-bean-02.txt | 2 +- ...16-json-plugin-configurable-limits-plan.md | 450 ++++++++++++++++++ 19 files changed, 1314 insertions(+), 367 deletions(-) create mode 100644 plugins/json/src/main/java/org/apache/struts2/json/JSONBeanSelectionProvider.java create mode 100644 plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONReader.java rename plugins/json/src/main/java/org/apache/struts2/json/{DefaultJSONWriter.java => StrutsJSONWriter.java} (99%) create mode 100644 plugins/json/src/test/java/org/apache/struts2/json/StrutsJSONReaderTest.java rename plugins/json/src/test/java/org/apache/struts2/json/{DefaultJSONWriterTest.java => StrutsJSONWriterTest.java} (86%) create mode 100644 thoughts/shared/research/2026-03-16-json-plugin-configurable-limits-plan.md diff --git a/plugins/json/src/main/java/org/apache/struts2/json/JSONBeanSelectionProvider.java b/plugins/json/src/main/java/org/apache/struts2/json/JSONBeanSelectionProvider.java new file mode 100644 index 0000000000..f13e5af755 --- /dev/null +++ b/plugins/json/src/main/java/org/apache/struts2/json/JSONBeanSelectionProvider.java @@ -0,0 +1,35 @@ +/* + * 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.struts2.json; + +import org.apache.struts2.config.AbstractBeanSelectionProvider; +import org.apache.struts2.config.ConfigurationException; +import org.apache.struts2.inject.ContainerBuilder; +import org.apache.struts2.inject.Scope; +import org.apache.struts2.util.location.LocatableProperties; + +public class JSONBeanSelectionProvider extends AbstractBeanSelectionProvider { + + @Override + public void register(ContainerBuilder builder, LocatableProperties props) throws ConfigurationException { + alias(JSONReader.class, JSONConstants.JSON_READER, builder, props, Scope.PROTOTYPE); + alias(JSONWriter.class, JSONConstants.JSON_WRITER, builder, props, Scope.PROTOTYPE); + } + +} diff --git a/plugins/json/src/main/java/org/apache/struts2/json/JSONConstants.java b/plugins/json/src/main/java/org/apache/struts2/json/JSONConstants.java index f5cb292bda..5e8bb5fdcc 100644 --- a/plugins/json/src/main/java/org/apache/struts2/json/JSONConstants.java +++ b/plugins/json/src/main/java/org/apache/struts2/json/JSONConstants.java @@ -29,6 +29,12 @@ public class JSONConstants { public static final String JSON_WRITER = "struts.json.writer"; + public static final String JSON_READER = "struts.json.reader"; public static final String RESULT_EXCLUDE_PROXY_PROPERTIES = "struts.json.result.excludeProxyProperties"; public static final String DATE_FORMAT = "struts.json.dateformat"; + public static final String JSON_MAX_ELEMENTS = "struts.json.maxElements"; + public static final String JSON_MAX_DEPTH = "struts.json.maxDepth"; + public static final String JSON_MAX_LENGTH = "struts.json.maxLength"; + public static final String JSON_MAX_STRING_LENGTH = "struts.json.maxStringLength"; + public static final String JSON_MAX_KEY_LENGTH = "struts.json.maxKeyLength"; } diff --git a/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java b/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java index df8609ab61..6511da0336 100644 --- a/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java +++ b/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java @@ -71,6 +71,13 @@ public class JSONInterceptor extends AbstractInterceptor { private String jsonContentType = "application/json"; private String jsonRpcContentType = "application/json-rpc"; + private JSONUtil jsonUtil; + private int maxElements = JSONReader.DEFAULT_MAX_ELEMENTS; + private int maxDepth = JSONReader.DEFAULT_MAX_DEPTH; + private int maxLength = 2_097_152; // 2MB + private int maxStringLength = JSONReader.DEFAULT_MAX_STRING_LENGTH; + private int maxKeyLength = JSONReader.DEFAULT_MAX_KEY_LENGTH; + @SuppressWarnings("unchecked") public String intercept(ActionInvocation invocation) throws Exception { HttpServletRequest request = ServletActionContext.getRequest(); @@ -91,7 +98,8 @@ public String intercept(ActionInvocation invocation) throws Exception { if (jsonContentType.equalsIgnoreCase(requestContentType)) { // load JSON object - Object obj = JSONUtil.deserialize(request.getReader()); + applyLimitsToReader(); + Object obj = jsonUtil.deserializeInput(request.getReader(), maxLength); // JSON array (this.root cannot be null in this case) if(obj instanceof List && this.root != null) { @@ -133,7 +141,8 @@ public String intercept(ActionInvocation invocation) throws Exception { Object result; if (this.enableSMD) { // load JSON object - Object obj = JSONUtil.deserialize(request.getReader()); + applyLimitsToReader(); + Object obj = jsonUtil.deserializeInput(request.getReader(), maxLength); if (obj instanceof Map) { Map smd = (Map) obj; @@ -168,8 +177,6 @@ public String intercept(ActionInvocation invocation) throws Exception { result = rpcResponse; } - JSONUtil jsonUtil = invocation.getInvocationContext().getContainer().getInstance(JSONUtil.class); - String json = jsonUtil.serialize(result, excludeProperties, getIncludeProperties(), ignoreHierarchy, excludeNullProperties); json = addCallbackIfApplicable(request, json); @@ -185,6 +192,14 @@ public String intercept(ActionInvocation invocation) throws Exception { return invocation.invoke(); } + private void applyLimitsToReader() { + JSONReader reader = jsonUtil.getReader(); + reader.setMaxElements(maxElements); + reader.setMaxDepth(maxDepth); + reader.setMaxStringLength(maxStringLength); + reader.setMaxKeyLength(maxKeyLength); + } + protected String readContentType(HttpServletRequest request) { String contentType = request.getHeader("Content-Type"); LOG.debug("Content Type from request: {}", contentType); @@ -564,4 +579,34 @@ public void setJsonContentType(String jsonContentType) { public void setJsonRpcContentType(String jsonRpcContentType) { this.jsonRpcContentType = jsonRpcContentType; } + + @Inject + public void setJsonUtil(JSONUtil jsonUtil) { + this.jsonUtil = jsonUtil; + } + + @Inject(value = JSONConstants.JSON_MAX_ELEMENTS, required = false) + public void setMaxElements(String maxElements) { + this.maxElements = Integer.parseInt(maxElements); + } + + @Inject(value = JSONConstants.JSON_MAX_DEPTH, required = false) + public void setMaxDepth(String maxDepth) { + this.maxDepth = Integer.parseInt(maxDepth); + } + + @Inject(value = JSONConstants.JSON_MAX_LENGTH, required = false) + public void setMaxLength(String maxLength) { + this.maxLength = Integer.parseInt(maxLength); + } + + @Inject(value = JSONConstants.JSON_MAX_STRING_LENGTH, required = false) + public void setMaxStringLength(String maxStringLength) { + this.maxStringLength = Integer.parseInt(maxStringLength); + } + + @Inject(value = JSONConstants.JSON_MAX_KEY_LENGTH, required = false) + public void setMaxKeyLength(String maxKeyLength) { + this.maxKeyLength = Integer.parseInt(maxKeyLength); + } } diff --git a/plugins/json/src/main/java/org/apache/struts2/json/JSONReader.java b/plugins/json/src/main/java/org/apache/struts2/json/JSONReader.java index 4fb1c406ec..1af39ac590 100644 --- a/plugins/json/src/main/java/org/apache/struts2/json/JSONReader.java +++ b/plugins/json/src/main/java/org/apache/struts2/json/JSONReader.java @@ -18,285 +18,25 @@ */ package org.apache.struts2.json; -import java.text.CharacterIterator; -import java.text.StringCharacterIterator; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - /** *

- * Deserializes and object from a JSON string + * Deserializes an object from a JSON string. *

*/ -public class JSONReader { - private static final Object OBJECT_END = new Object(); - private static final Object ARRAY_END = new Object(); - private static final Object COLON = new Object(); - private static final Object COMMA = new Object(); - private static Map escapes = new HashMap(); - - static { - escapes.put('"', '"'); - escapes.put('\\', '\\'); - escapes.put('/', '/'); - escapes.put('b', '\b'); - escapes.put('f', '\f'); - escapes.put('n', '\n'); - escapes.put('r', '\r'); - escapes.put('t', '\t'); - } - - private CharacterIterator it; - private char c; - private Object token; - private StringBuilder buf = new StringBuilder(); - - protected char next() { - this.c = this.it.next(); - - return this.c; - } - - protected void skipWhiteSpace() { - while (Character.isWhitespace(this.c)) { - this.next(); - } - } - - public Object read(String string) throws JSONException { - this.it = new StringCharacterIterator(string); - this.c = this.it.first(); - - return this.read(); - } - - protected Object read() throws JSONException { - Object ret; - - this.skipWhiteSpace(); - - if (this.c == '"') { - this.next(); - ret = this.string('"'); - } else if (this.c == '\'') { - this.next(); - ret = this.string('\''); - } else if (this.c == '[') { - this.next(); - ret = this.array(); - } else if (this.c == ']') { - ret = ARRAY_END; - this.next(); - } else if (this.c == ',') { - ret = COMMA; - this.next(); - } else if (this.c == '{') { - this.next(); - ret = this.object(); - } else if (this.c == '}') { - ret = OBJECT_END; - this.next(); - } else if (this.c == ':') { - ret = COLON; - this.next(); - } else if ((this.c == 't') && (this.next() == 'r') && (this.next() == 'u') && (this.next() == 'e')) { - ret = Boolean.TRUE; - this.next(); - } else if ((this.c == 'f') && (this.next() == 'a') && (this.next() == 'l') && (this.next() == 's') - && (this.next() == 'e')) { - ret = Boolean.FALSE; - this.next(); - } else if ((this.c == 'n') && (this.next() == 'u') && (this.next() == 'l') && (this.next() == 'l')) { - ret = null; - this.next(); - } else if (Character.isDigit(this.c) || (this.c == '-')) { - ret = this.number(); - } else { - throw buildInvalidInputException(); - } - - this.token = ret; - - return ret; - } - - @SuppressWarnings("unchecked") - protected Map object() throws JSONException { - Map ret = new HashMap(); - Object next = this.read(); - if (next != OBJECT_END) { - String key = (String) next; - while (this.token != OBJECT_END) { - this.read(); // should be a colon - - if (this.token != OBJECT_END) { - ret.put(key, this.read()); - - if (this.read() == COMMA) { - Object name = this.read(); - - if (name instanceof String) { - key = (String) name; - } else - throw buildInvalidInputException(); - } - } - } - } - - return ret; - } - - protected JSONException buildInvalidInputException() { - return new JSONException("Input string is not well formed JSON (invalid char " + this.c + ")"); - } - - - @SuppressWarnings("unchecked") - protected List array() throws JSONException { - List ret = new ArrayList(); - Object value = this.read(); - - while (this.token != ARRAY_END) { - ret.add(value); - - Object read = this.read(); - if (read == COMMA) { - value = this.read(); - } else if (read != ARRAY_END) { - throw buildInvalidInputException(); - } - } - - return ret; - } - - protected Object number() throws JSONException { - this.buf.setLength(0); - boolean toDouble = false; - - if (this.c == '-') { - this.add(); - } - - this.addDigits(); - - if (this.c == '.') { - toDouble = true; - this.add(); - this.addDigits(); - } - - if ((this.c == 'e') || (this.c == 'E')) { - toDouble = true; - this.add(); - - if ((this.c == '+') || (this.c == '-')) { - this.add(); - } - - this.addDigits(); - } - - if (toDouble) { - try { - return Double.parseDouble(this.buf.toString()); - } catch (NumberFormatException e) { - throw buildInvalidInputException(); - } - } else { - try { - return Long.parseLong(this.buf.toString()); - } catch (NumberFormatException e) { - throw buildInvalidInputException(); - } - } - } - - protected Object string(char quote) { - this.buf.setLength(0); - - while ((this.c != quote) && (this.c != CharacterIterator.DONE)) { - if (this.c == '\\') { - this.next(); - - if (this.c == 'u') { - this.add(this.unicode()); - } else { - Object value = escapes.get(this.c); - - if (value != null) { - this.add((Character) value); - } - } - } else { - this.add(); - } - } - - this.next(); - - return this.buf.toString(); - } - - protected void add(char cc) { - this.buf.append(cc); - this.next(); - } - - protected void add() { - this.add(this.c); - } - - protected void addDigits() { - while (Character.isDigit(this.c)) { - this.add(); - } - } - - protected char unicode() { - int value = 0; - - for (int i = 0; i < 4; ++i) { - switch (this.next()) { - case '0': - case '1': - case '2': - case '3': - case '4': - case '5': - case '6': - case '7': - case '8': - case '9': - value = (value << 4) + (this.c - '0'); +public interface JSONReader { - break; + int DEFAULT_MAX_ELEMENTS = 10_000; + int DEFAULT_MAX_DEPTH = 64; + int DEFAULT_MAX_STRING_LENGTH = 262_144; // 256KB + int DEFAULT_MAX_KEY_LENGTH = 512; - case 'a': - case 'b': - case 'c': - case 'd': - case 'e': - case 'f': - value = (value << 4) + (this.c - 'W'); + Object read(String string) throws JSONException; - break; + void setMaxElements(int maxElements); - case 'A': - case 'B': - case 'C': - case 'D': - case 'E': - case 'F': - value = (value << 4) + (this.c - '7'); + void setMaxDepth(int maxDepth); - break; - } - } + void setMaxStringLength(int maxStringLength); - return (char) value; - } + void setMaxKeyLength(int maxKeyLength); } diff --git a/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java b/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java index ab23f4dcdb..78bceaacf3 100644 --- a/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java +++ b/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java @@ -40,7 +40,6 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.apache.struts2.inject.Container; import org.apache.struts2.inject.Inject; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; @@ -60,16 +59,21 @@ public class JSONUtil { private static final Logger LOG = LogManager.getLogger(JSONUtil.class); + private JSONReader reader; private JSONWriter writer; - public void setWriter(JSONWriter writer) { - this.writer = writer; + @Inject + public void setReader(JSONReader reader) { + this.reader = reader; + } + + public JSONReader getReader() { + return reader; } @Inject - public void setContainer(Container container) { - setWriter(container.getInstance(JSONWriter.class, container.getInstance(String.class, - JSONConstants.JSON_WRITER))); + public void setWriter(JSONWriter writer) { + this.writer = writer; } /** @@ -284,6 +288,34 @@ public void serialize(Writer writer, Object object, Collection excludeP writer.write(serialize(object, excludeProperties, includeProperties, true, excludeNullProperties, cacheBeanInfo)); } + /** + * Deserializes an object from JSON using the injected reader with limit enforcement. + * + * @param reader Reader to read a JSON string from + * @param maxLength maximum allowed length of the JSON input + * @return deserialized object + * @throws JSONException when IOException happens or limits are exceeded + */ + public Object deserializeInput(Reader reader, int maxLength) throws JSONException { + BufferedReader bufferReader = new BufferedReader(reader); + String line; + StringBuilder buffer = new StringBuilder(); + + try { + while ((line = bufferReader.readLine()) != null) { + buffer.append(line); + if (buffer.length() > maxLength) { + throw new JSONException("JSON input exceeds maximum allowed length (" + + maxLength + "). Use " + JSONConstants.JSON_MAX_LENGTH + " to increase the limit."); + } + } + } catch (IOException e) { + throw new JSONException(e); + } + + return this.reader.read(buffer.toString()); + } + /** * Deserializes a object from JSON * @@ -291,9 +323,11 @@ public void serialize(Writer writer, Object object, Collection excludeP * string in JSON * @return desrialized object * @throws JSONException in case of error during serialize + * @deprecated Use instance method {@link #deserializeInput(Reader, int)} with injected JSONUtil instead */ + @Deprecated( forRemoval = true, since = "7.2.0") public static Object deserialize(String json) throws JSONException { - JSONReader reader = new JSONReader(); + StrutsJSONReader reader = new StrutsJSONReader(); return reader.read(json); } @@ -305,7 +339,9 @@ public static Object deserialize(String json) throws JSONException { * @return deserialized object * @throws JSONException * when IOException happens + * @deprecated Use instance method {@link #deserializeInput(Reader, int)} with injected JSONUtil instead */ + @Deprecated( forRemoval = true, since = "7.2.0") public static Object deserialize(Reader reader) throws JSONException { // read content BufferedReader bufferReader = new BufferedReader(reader); diff --git a/plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONReader.java b/plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONReader.java new file mode 100644 index 0000000000..530a479cc6 --- /dev/null +++ b/plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONReader.java @@ -0,0 +1,338 @@ +/* + * 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.struts2.json; + +import java.text.CharacterIterator; +import java.text.StringCharacterIterator; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + *

+ * Deserializes an object from a JSON string with configurable limits + * to prevent denial-of-service attacks via malicious payloads. + *

+ */ +public class StrutsJSONReader implements JSONReader { + private static final Object OBJECT_END = new Object(); + private static final Object ARRAY_END = new Object(); + private static final Object COLON = new Object(); + private static final Object COMMA = new Object(); + private static final Map escapes = Map.of( + '"', '"', + '\\', '\\', + '/', '/', + 'b', '\b', + 'f', '\f', + 'n', '\n', + 'r', '\r', + 't', '\t' + ); + + private CharacterIterator it; + private char c; + private Object token; + private final StringBuilder buf = new StringBuilder(); + + private int maxElements = DEFAULT_MAX_ELEMENTS; + private int maxDepth = DEFAULT_MAX_DEPTH; + private int maxStringLength = DEFAULT_MAX_STRING_LENGTH; + private int maxKeyLength = DEFAULT_MAX_KEY_LENGTH; + private int depth; + + @Override + public void setMaxElements(int maxElements) { + this.maxElements = maxElements; + } + + @Override + public void setMaxDepth(int maxDepth) { + this.maxDepth = maxDepth; + } + + @Override + public void setMaxStringLength(int maxStringLength) { + this.maxStringLength = maxStringLength; + } + + @Override + public void setMaxKeyLength(int maxKeyLength) { + this.maxKeyLength = maxKeyLength; + } + + protected char next() { + this.c = this.it.next(); + + return this.c; + } + + protected void skipWhiteSpace() { + while (Character.isWhitespace(this.c)) { + this.next(); + } + } + + @Override + public Object read(String string) throws JSONException { + this.it = new StringCharacterIterator(string); + this.c = this.it.first(); + this.depth = 0; + + return this.read(); + } + + protected Object read() throws JSONException { + Object ret; + + this.skipWhiteSpace(); + + if (this.c == '"') { + this.next(); + ret = this.string('"'); + } else if (this.c == '\'') { + this.next(); + ret = this.string('\''); + } else if (this.c == '[') { + this.next(); + ret = this.array(); + } else if (this.c == ']') { + ret = ARRAY_END; + this.next(); + } else if (this.c == ',') { + ret = COMMA; + this.next(); + } else if (this.c == '{') { + this.next(); + ret = this.object(); + } else if (this.c == '}') { + ret = OBJECT_END; + this.next(); + } else if (this.c == ':') { + ret = COLON; + this.next(); + } else if ((this.c == 't') && (this.next() == 'r') && (this.next() == 'u') && (this.next() == 'e')) { + ret = Boolean.TRUE; + this.next(); + } else if ((this.c == 'f') && (this.next() == 'a') && (this.next() == 'l') && (this.next() == 's') + && (this.next() == 'e')) { + ret = Boolean.FALSE; + this.next(); + } else if ((this.c == 'n') && (this.next() == 'u') && (this.next() == 'l') && (this.next() == 'l')) { + ret = null; + this.next(); + } else if (Character.isDigit(this.c) || (this.c == '-')) { + ret = this.number(); + } else { + throw buildInvalidInputException(); + } + + this.token = ret; + + return ret; + } + + protected Map object() throws JSONException { + if (this.depth >= this.maxDepth) { + throw new JSONException("JSON object nesting exceeds maximum allowed depth (" + + this.maxDepth + "). Use " + JSONConstants.JSON_MAX_DEPTH + " to increase the limit."); + } + this.depth++; + try { + Map ret = new HashMap<>(); + Object next = this.read(); + if (next != OBJECT_END) { + String key = (String) next; + validateKeyLength(key); + while (this.token != OBJECT_END) { + this.read(); // should be a colon + + if (this.token != OBJECT_END) { + if (ret.size() >= this.maxElements) { + throw new JSONException("JSON object exceeds maximum allowed elements (" + + this.maxElements + "). Use " + JSONConstants.JSON_MAX_ELEMENTS + " to increase the limit."); + } + ret.put(key, this.read()); + + if (this.read() == COMMA) { + Object name = this.read(); + + if (name instanceof String nextKey) { + key = nextKey; + validateKeyLength(key); + } else { + throw buildInvalidInputException(); + } + } + } + } + } + + return ret; + } finally { + this.depth--; + } + } + + private void validateKeyLength(String key) throws JSONException { + if (key.length() > this.maxKeyLength) { + throw new JSONException("JSON object key exceeds maximum allowed length (" + + this.maxKeyLength + "). Use " + JSONConstants.JSON_MAX_KEY_LENGTH + " to increase the limit."); + } + } + + protected JSONException buildInvalidInputException() { + return new JSONException("Input string is not well formed JSON (invalid char " + this.c + ")"); + } + + + protected List array() throws JSONException { + if (this.depth >= this.maxDepth) { + throw new JSONException("JSON array nesting exceeds maximum allowed depth (" + + this.maxDepth + "). Use " + JSONConstants.JSON_MAX_DEPTH + " to increase the limit."); + } + this.depth++; + try { + List ret = new ArrayList<>(); + Object value = this.read(); + + while (this.token != ARRAY_END) { + if (ret.size() >= this.maxElements) { + throw new JSONException("JSON array exceeds maximum allowed elements (" + + this.maxElements + "). Use " + JSONConstants.JSON_MAX_ELEMENTS + " to increase the limit."); + } + ret.add(value); + + Object read = this.read(); + if (read == COMMA) { + value = this.read(); + } else if (read != ARRAY_END) { + throw buildInvalidInputException(); + } + } + + return ret; + } finally { + this.depth--; + } + } + + protected Object number() throws JSONException { + this.buf.setLength(0); + boolean toDouble = false; + + if (this.c == '-') { + this.add(); + } + + this.addDigits(); + + if (this.c == '.') { + toDouble = true; + this.add(); + this.addDigits(); + } + + if ((this.c == 'e') || (this.c == 'E')) { + toDouble = true; + this.add(); + + if ((this.c == '+') || (this.c == '-')) { + this.add(); + } + + this.addDigits(); + } + + if (toDouble) { + try { + return Double.parseDouble(this.buf.toString()); + } catch (NumberFormatException e) { + throw buildInvalidInputException(); + } + } else { + try { + return Long.parseLong(this.buf.toString()); + } catch (NumberFormatException e) { + throw buildInvalidInputException(); + } + } + } + + protected Object string(char quote) throws JSONException { + this.buf.setLength(0); + + while ((this.c != quote) && (this.c != CharacterIterator.DONE)) { + if (this.c == '\\') { + this.next(); + + if (this.c == 'u') { + this.add(this.unicode()); + } else { + Character value = escapes.get(this.c); + + if (value != null) { + this.add(value); + } + } + } else { + this.add(); + } + if (this.buf.length() > this.maxStringLength) { + throw new JSONException("JSON string exceeds maximum allowed length (" + + this.maxStringLength + "). Use " + JSONConstants.JSON_MAX_STRING_LENGTH + " to increase the limit."); + } + } + + this.next(); + + return this.buf.toString(); + } + + protected void add(char cc) { + this.buf.append(cc); + this.next(); + } + + protected void add() { + this.add(this.c); + } + + protected void addDigits() { + while (Character.isDigit(this.c)) { + this.add(); + } + } + + protected char unicode() { + int value = 0; + + for (int i = 0; i < 4; ++i) { + value = switch (this.next()) { + case '0', '1', '2', '3', '4', '5', '6', '7', '8', '9' -> (value << 4) + (this.c - '0'); + case 'a', 'b', 'c', 'd', 'e', 'f' -> (value << 4) + (this.c - 'W'); + case 'A', 'B', 'C', 'D', 'E', 'F' -> (value << 4) + (this.c - '7'); + default -> value; + }; + } + + return (char) value; + } +} diff --git a/plugins/json/src/main/java/org/apache/struts2/json/DefaultJSONWriter.java b/plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONWriter.java similarity index 99% rename from plugins/json/src/main/java/org/apache/struts2/json/DefaultJSONWriter.java rename to plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONWriter.java index f911139fc3..c75e54e0a1 100644 --- a/plugins/json/src/main/java/org/apache/struts2/json/DefaultJSONWriter.java +++ b/plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONWriter.java @@ -67,9 +67,9 @@ * references are detected they will be nulled out. *

*/ -public class DefaultJSONWriter implements JSONWriter { +public class StrutsJSONWriter implements JSONWriter { - private static final Logger LOG = LogManager.getLogger(DefaultJSONWriter.class); + private static final Logger LOG = LogManager.getLogger(StrutsJSONWriter.class); private static final char[] hex = "0123456789ABCDEF".toCharArray(); diff --git a/plugins/json/src/main/java/org/apache/struts2/json/config/entities/JSONConstantConfig.java b/plugins/json/src/main/java/org/apache/struts2/json/config/entities/JSONConstantConfig.java index ee7fab60f4..580ee7e7a1 100644 --- a/plugins/json/src/main/java/org/apache/struts2/json/config/entities/JSONConstantConfig.java +++ b/plugins/json/src/main/java/org/apache/struts2/json/config/entities/JSONConstantConfig.java @@ -27,16 +27,28 @@ public class JSONConstantConfig extends ConstantConfig { private BeanConfig jsonWriter; + private BeanConfig jsonReader; private Boolean jsonResultExcludeProxyProperties; private String jsonDateFormat; + private Integer jsonMaxElements; + private Integer jsonMaxDepth; + private Integer jsonMaxLength; + private Integer jsonMaxStringLength; + private Integer jsonMaxKeyLength; @Override public Map getAllAsStringsMap() { Map map = super.getAllAsStringsMap(); map.put(JSONConstants.JSON_WRITER, beanConfToString(jsonWriter)); + map.put(JSONConstants.JSON_READER, beanConfToString(jsonReader)); map.put(JSONConstants.RESULT_EXCLUDE_PROXY_PROPERTIES, Objects.toString(jsonResultExcludeProxyProperties, null)); map.put(JSONConstants.DATE_FORMAT, jsonDateFormat); + map.put(JSONConstants.JSON_MAX_ELEMENTS, Objects.toString(jsonMaxElements, null)); + map.put(JSONConstants.JSON_MAX_DEPTH, Objects.toString(jsonMaxDepth, null)); + map.put(JSONConstants.JSON_MAX_LENGTH, Objects.toString(jsonMaxLength, null)); + map.put(JSONConstants.JSON_MAX_STRING_LENGTH, Objects.toString(jsonMaxStringLength, null)); + map.put(JSONConstants.JSON_MAX_KEY_LENGTH, Objects.toString(jsonMaxKeyLength, null)); return map; } @@ -68,4 +80,56 @@ public String getJsonDateFormat() { public void setJsonDateFormat(String jsonDateFormat) { this.jsonDateFormat = jsonDateFormat; } + + public BeanConfig getJsonReader() { + return jsonReader; + } + + public void setJsonReader(BeanConfig jsonReader) { + this.jsonReader = jsonReader; + } + + public void setJsonReader(Class clazz) { + this.jsonReader = new BeanConfig(clazz, clazz.getName()); + } + + public Integer getJsonMaxElements() { + return jsonMaxElements; + } + + public void setJsonMaxElements(Integer jsonMaxElements) { + this.jsonMaxElements = jsonMaxElements; + } + + public Integer getJsonMaxDepth() { + return jsonMaxDepth; + } + + public void setJsonMaxDepth(Integer jsonMaxDepth) { + this.jsonMaxDepth = jsonMaxDepth; + } + + public Integer getJsonMaxLength() { + return jsonMaxLength; + } + + public void setJsonMaxLength(Integer jsonMaxLength) { + this.jsonMaxLength = jsonMaxLength; + } + + public Integer getJsonMaxStringLength() { + return jsonMaxStringLength; + } + + public void setJsonMaxStringLength(Integer jsonMaxStringLength) { + this.jsonMaxStringLength = jsonMaxStringLength; + } + + public Integer getJsonMaxKeyLength() { + return jsonMaxKeyLength; + } + + public void setJsonMaxKeyLength(Integer jsonMaxKeyLength) { + this.jsonMaxKeyLength = jsonMaxKeyLength; + } } diff --git a/plugins/json/src/main/resources/struts-plugin.xml b/plugins/json/src/main/resources/struts-plugin.xml index 1291246a71..88451b39b4 100644 --- a/plugins/json/src/main/resources/struts-plugin.xml +++ b/plugins/json/src/main/resources/struts-plugin.xml @@ -24,12 +24,20 @@ "https://struts.apache.org/dtds/struts-6.0.dtd"> - + - - + + + + + + + + @@ -54,4 +62,6 @@ + + diff --git a/plugins/json/src/test/java/org/apache/struts2/json/JSONEnumTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONEnumTest.java index a9ee11331a..648e8f7b43 100644 --- a/plugins/json/src/test/java/org/apache/struts2/json/JSONEnumTest.java +++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONEnumTest.java @@ -44,11 +44,11 @@ public void testEnumAsNameValue() throws Exception { bean1.setEnumField(AnEnum.ValueA); bean1.setEnumBean(AnEnumBean.Two); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); jsonWriter.setEnumAsBean(false); String json = jsonWriter.write(bean1); - Map result = (Map) JSONUtil.deserialize(json); + Map result = (Map) new StrutsJSONReader().read(json); assertEquals("str", result.get("stringField")); assertEquals(true, result.get("booleanField")); assertEquals("s", result.get("charField")); // note: this is a @@ -86,11 +86,11 @@ public void testEnumAsBean() throws Exception { bean1.setEnumField(AnEnum.ValueA); bean1.setEnumBean(AnEnumBean.Two); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); jsonWriter.setEnumAsBean(true); String json = jsonWriter.write(bean1); - Map result = (Map) JSONUtil.deserialize(json); + Map result = (Map) new StrutsJSONReader().read(json); assertEquals("str", result.get("stringField")); assertEquals(true, result.get("booleanField")); assertEquals("s", result.get("charField")); // note: this is a diff --git a/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java index a5cfd2e1c5..9f5c4a75f5 100644 --- a/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java +++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java @@ -41,6 +41,15 @@ private void setRequestContent(String fileName) throws Exception { this.request.setContent(content.getBytes()); } + private JSONInterceptor createInterceptor() { + JSONInterceptor interceptor = new JSONInterceptor(); + JSONUtil jsonUtil = new JSONUtil(); + jsonUtil.setReader(new StrutsJSONReader()); + jsonUtil.setWriter(new StrutsJSONWriter()); + interceptor.setJsonUtil(jsonUtil); + return interceptor; + } + public void testBadJSON1() throws Exception { tryBadJSON("bad-1.txt"); } @@ -70,7 +79,7 @@ private void tryBadJSON(String fileName) throws Exception { setRequestContent(fileName); this.request.addHeader("Content-Type", "application/json; charset=UTF-8"); - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); interceptor.setEnableSMD(true); SMDActionTest1 action = new SMDActionTest1(); @@ -91,7 +100,7 @@ public void testSMDDisabledSMD() throws Exception { setRequestContent("smd-3.txt"); this.request.addHeader("Content-Type", "application/json-rpc"); - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); SMDActionTest1 action = new SMDActionTest1(); this.invocation.setAction(action); @@ -110,7 +119,7 @@ public void testSMDAliasedMethodCall1() throws Exception { setRequestContent("smd-14.txt"); this.request.addHeader("Content-Type", "application/json-rpc"); - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); interceptor.setEnableSMD(true); SMDActionTest2 action = new SMDActionTest2(); @@ -128,7 +137,7 @@ public void testSMDAliasedMethodCall2() throws Exception { setRequestContent("smd-15.txt"); this.request.addHeader("Content-Type", "application/json-rpc"); - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); interceptor.setEnableSMD(true); SMDActionTest2 action = new SMDActionTest2(); @@ -146,7 +155,7 @@ public void testSMDNoMethod() throws Exception { setRequestContent("smd-4.txt"); this.request.addHeader("Content-Type", "application/json-rpc"); - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); interceptor.setEnableSMD(true); SMDActionTest1 action = new SMDActionTest1(); @@ -170,7 +179,7 @@ public void testSMDMethodWithoutAnnotations() throws Exception { setRequestContent("smd-9.txt"); this.request.addHeader("Content-Type", "application/json-rpc"); - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); interceptor.setEnableSMD(true); SMDActionTest1 action = new SMDActionTest1(); @@ -191,7 +200,7 @@ public void testSMDPrimitivesNoResult() throws Exception { setRequestContent("smd-6.txt"); this.request.addHeader("Content-Type", "application/json-rpc"); - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); interceptor.setEnableSMD(true); SMDActionTest1 action = new SMDActionTest1(); @@ -226,7 +235,7 @@ public void testSMDReturnObject() throws Exception { setRequestContent("smd-10.txt"); this.request.addHeader("Content-Type", "application/json-rpc"); - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); interceptor.setEnableSMD(true); SMDActionTest2 action = new SMDActionTest2(); @@ -251,7 +260,7 @@ public void testSMDObjectsNoResult() throws Exception { setRequestContent("smd-7.txt"); this.request.addHeader("Content-Type", "application/json-rpc"); - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); interceptor.setEnableSMD(true); SMDActionTest1 action = new SMDActionTest1(); @@ -300,7 +309,7 @@ public void testReadEmpty() throws Exception { this.request.addHeader("Content-Type", "application/json"); // interceptor - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); TestAction action = new TestAction(); this.invocation.setAction(action); @@ -315,7 +324,7 @@ public void test() throws Exception { this.request.addHeader("Content-Type", "application/json"); // interceptor - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); TestAction action = new TestAction(); this.invocation.setAction(action); @@ -437,7 +446,7 @@ public void testRoot() throws Exception { this.request.addHeader("Content-Type", "application/json"); // interceptor - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); interceptor.setRoot("bean"); TestAction4 action = new TestAction4(); @@ -462,7 +471,7 @@ public void testJSONArray() throws Exception { this.request.addHeader("Content-Type", "application/json"); // interceptor - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); interceptor.setRoot("beans"); TestAction5 action = new TestAction5(); @@ -488,7 +497,7 @@ public void testJSONArray2() throws Exception { this.request.addHeader("Content-Type", "application/json"); // interceptor - JSONInterceptor interceptor = new JSONInterceptor(); + JSONInterceptor interceptor = createInterceptor(); interceptor.setRoot("anotherBean.yetAnotherBean.beans"); TestAction5 action = new TestAction5(); @@ -509,6 +518,63 @@ public void testJSONArray2() throws Exception { assertEquals(beans.get(0).getByteField(), 3); } + public void testMaxLengthExceededThrows() throws Exception { + // Body is 27 bytes, set maxLength to 10 + this.request.setContent("{\"a\":1, \"b\":2, \"c\":\"hello\"}".getBytes()); + this.request.addHeader("Content-Type", "application/json"); + + JSONInterceptor interceptor = createInterceptor(); + interceptor.setMaxLength("10"); + TestAction action = new TestAction(); + + this.invocation.setAction(action); + + try { + interceptor.intercept(this.invocation); + fail("Should have thrown JSONException for exceeding maxLength"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains(JSONConstants.JSON_MAX_LENGTH)); + } + } + + public void testMaxDepthEnforcedThroughInterceptor() throws Exception { + // Nested 3 levels deep, set maxDepth to 2 + this.request.setContent("{\"a\":{\"b\":{\"c\":1}}}".getBytes()); + this.request.addHeader("Content-Type", "application/json"); + + JSONInterceptor interceptor = createInterceptor(); + interceptor.setMaxDepth("2"); + TestAction action = new TestAction(); + + this.invocation.setAction(action); + + try { + interceptor.intercept(this.invocation); + fail("Should have thrown JSONException for exceeding maxDepth"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("maximum allowed depth")); + } + } + + public void testMaxElementsEnforcedThroughInterceptor() throws Exception { + // JSON object with 5 keys, set maxElements to 3 + this.request.setContent("{\"a\":1, \"b\":2, \"c\":3, \"d\":4, \"e\":5}".getBytes()); + this.request.addHeader("Content-Type", "application/json"); + + JSONInterceptor interceptor = createInterceptor(); + interceptor.setMaxElements("3"); + TestAction action = new TestAction(); + + this.invocation.setAction(action); + + try { + interceptor.intercept(this.invocation); + fail("Should have thrown JSONException for exceeding maxElements"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("maximum allowed elements")); + } + } + @Override protected void setUp() throws Exception { super.setUp(); diff --git a/plugins/json/src/test/java/org/apache/struts2/json/JSONPopulatorTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONPopulatorTest.java index bf44b2ef9e..e61b81ea6c 100644 --- a/plugins/json/src/test/java/org/apache/struts2/json/JSONPopulatorTest.java +++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONPopulatorTest.java @@ -22,7 +22,6 @@ import org.junit.Test; import java.beans.IntrospectionException; -import java.io.StringReader; import java.lang.reflect.InvocationTargetException; import java.math.BigDecimal; import java.math.BigInteger; @@ -74,9 +73,8 @@ public void testNulls() throws IntrospectionException, InvocationTargetException @Test public void testPrimitiveBean() throws Exception { - StringReader stringReader = new StringReader(TestUtils.readContent(JSONInterceptorTest.class - .getResource("json-7.txt"))); - Object json = JSONUtil.deserialize(stringReader); + String text = TestUtils.readContent(JSONInterceptorTest.class.getResource("json-7.txt")); + Object json = new StrutsJSONReader().read(text); assertNotNull(json); assertTrue(json instanceof Map); Map jsonMap = (Map) json; @@ -96,7 +94,7 @@ public void testPrimitiveBean() throws Exception { @Test public void testObjectBean() throws Exception { String text = TestUtils.readContent(JSONInterceptorTest.class.getResource("json-7.txt")); - Object json = JSONUtil.deserialize(text); + Object json = new StrutsJSONReader().read(text); assertNotNull(json); assertTrue(json instanceof Map); Map jsonMap = (Map) json; @@ -162,9 +160,8 @@ public void testObjectBean() throws Exception { @Test public void testObjectBeanWithStrings() throws Exception { - StringReader stringReader = new StringReader(TestUtils.readContent(JSONInterceptorTest.class - .getResource("json-8.txt"))); - Object json = JSONUtil.deserialize(stringReader); + String text = TestUtils.readContent(JSONInterceptorTest.class.getResource("json-8.txt")); + Object json = new StrutsJSONReader().read(text); assertNotNull(json); assertTrue(json instanceof Map); Map jsonMap = (Map) json; @@ -187,7 +184,7 @@ public void testObjectBeanWithStrings() throws Exception { @Test public void testInfiniteLoop() { try { - JSONReader reader = new JSONReader(); + JSONReader reader = new StrutsJSONReader(); reader.read("[1,\"a]"); fail("Should have thrown an exception"); } catch (JSONException e) { @@ -198,7 +195,7 @@ public void testInfiniteLoop() { @Test public void testParseBadInput() { try { - JSONReader reader = new JSONReader(); + JSONReader reader = new StrutsJSONReader(); reader.read("[1,\"a\"1]"); fail("Should have thrown an exception"); } catch (JSONException e) { diff --git a/plugins/json/src/test/java/org/apache/struts2/json/JSONReaderTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONReaderTest.java index 9c466c3f4d..80050db6c0 100644 --- a/plugins/json/src/test/java/org/apache/struts2/json/JSONReaderTest.java +++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONReaderTest.java @@ -29,7 +29,7 @@ * Time: 17.26 */ public class JSONReaderTest { - private JSONReader reader = new JSONReader(); + private StrutsJSONReader reader = new StrutsJSONReader(); @Test public void testExponentialNumber() throws Exception { diff --git a/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java index 77831ed945..a5c938f0aa 100644 --- a/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java +++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java @@ -66,7 +66,7 @@ public void testJSONUtilNPEOnNullMehtod() { map.put("createtime", new Date()); try { JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); jsonUtil.serialize(map, JSONUtil.CACHE_BEAN_INFO_DEFAULT); } catch (JSONException e) { fail(e.getMessage()); @@ -76,14 +76,14 @@ public void testJSONUtilNPEOnNullMehtod() { public void testJSONWriterEndlessLoopOnExludedProperties() throws JSONException { Pattern all = Pattern.compile(".*"); - JSONWriter writer = new DefaultJSONWriter(); + JSONWriter writer = new StrutsJSONWriter(); writer.write(Arrays.asList("a", "b"), Arrays.asList(all), null, false); } public void testSMDDisabledSMD() throws Exception { JSONResult result = new JSONResult(); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); SMDActionTest1 action = new SMDActionTest1(); stack.push(action); @@ -102,7 +102,7 @@ public void testSMDDefault() throws Exception { JSONResult result = new JSONResult(); result.setEnableSMD(true); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); SMDActionTest1 action = new SMDActionTest1(); stack.push(action); @@ -122,7 +122,7 @@ public void testSMDDefaultAnnotations() throws Exception { JSONResult result = new JSONResult(); result.setEnableSMD(true); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); SMDActionTest2 action = new SMDActionTest2(); stack.push(action); @@ -142,7 +142,7 @@ public void testExcludeNullPropeties() throws Exception { JSONResult result = new JSONResult(); result.setExcludeNullProperties(true); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction action = new TestAction(); stack.push(action); @@ -161,7 +161,7 @@ public void testExcludeNullPropeties() throws Exception { public void testNotTraverseOrIncludeProxyInfo() throws Exception { JSONResult result = new JSONResult(); JSONUtil jsonUtil = new JSONUtil(); - DefaultJSONWriter writer = new DefaultJSONWriter(); + StrutsJSONWriter writer = new StrutsJSONWriter(); writer.setProxyService(proxyService); jsonUtil.setWriter(writer); result.setJsonUtil(jsonUtil); @@ -195,7 +195,7 @@ public void testWrapPrefix() throws Exception { JSONResult result = new JSONResult(); result.setWrapPrefix("_prefix_"); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction2 action = new TestAction2(); stack.push(action); @@ -214,7 +214,7 @@ public void testSuffix() throws Exception { JSONResult result = new JSONResult(); result.setWrapSuffix("_suffix_"); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction2 action = new TestAction2(); stack.push(action); @@ -233,7 +233,7 @@ public void testCustomDateFormat() throws Exception { JSONResult result = new JSONResult(); result.setDefaultDateFormat("MM-dd-yyyy"); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss z"); @@ -254,7 +254,7 @@ public void testPrefixAndSuffix() throws Exception { result.setWrapPrefix("_prefix_"); result.setWrapSuffix("_suffix_"); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction2 action = new TestAction2(); stack.push(action); @@ -274,7 +274,7 @@ public void testPrefix() throws Exception { result.setExcludeNullProperties(true); result.setPrefix(true); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction action = new TestAction(); stack.push(action); @@ -294,7 +294,7 @@ public void testPrefix() throws Exception { public void test() throws Exception { JSONResult result = new JSONResult(); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction action = new TestAction(); @@ -380,7 +380,7 @@ public void testHierarchy() throws Exception { JSONResult result = new JSONResult(); result.setIgnoreHierarchy(false); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction3 action = new TestAction3(); @@ -399,7 +399,7 @@ public void testHierarchy() throws Exception { public void testCommentWrap() throws Exception { JSONResult result = new JSONResult(); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction action = new TestAction(); @@ -481,7 +481,7 @@ public void testCommentWrap() throws Exception { private void executeTest2Action(JSONResult result) throws Exception { JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction action = new TestAction(); stack.push(action); @@ -526,7 +526,7 @@ public void testJSONP() throws Exception { JSONResult result = new JSONResult(); result.setCallbackParameter("callback"); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); request.addParameter("callback", "exec"); @@ -543,7 +543,7 @@ public void testNoCache() throws Exception { JSONResult result = new JSONResult(); result.setNoCache(true); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); executeTest2Action(result); @@ -557,7 +557,7 @@ public void testContentType() throws Exception { JSONResult result = new JSONResult(); result.setContentType("some_super_content"); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); executeTest2Action(result); @@ -569,7 +569,7 @@ public void testStatusCode() throws Exception { JSONResult result = new JSONResult(); result.setStatusCode(HttpServletResponse.SC_CONTINUE); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); executeTest2Action(result); @@ -584,7 +584,7 @@ public void test2WithEnumBean() throws Exception { JSONResult result = new JSONResult(); result.setEnumAsBean(true); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); executeTest2Action(result); @@ -605,7 +605,7 @@ public void testIncludeProperties() throws Exception { JSONResult result = new JSONResult(); result.setIncludeProperties("foo"); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction action = new TestAction(); stack.push(action); @@ -625,7 +625,7 @@ public void testIncludePropertiesWithList() throws Exception { JSONResult result = new JSONResult(); result.setIncludeProperties("^list\\[\\d+\\]\\.booleanField"); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction action = new TestAction(); stack.push(action); @@ -652,7 +652,7 @@ public void testIncludePropertiesWithSetList() throws Exception { JSONResult result = new JSONResult(); result.setIncludeProperties("^set\\[\\d+\\]\\.list\\[\\d+\\]\\.booleanField"); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); result.setJsonUtil(jsonUtil); TestAction action = new TestAction(); stack.push(action); diff --git a/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilTest.java index d03d69e999..b2dcf8bff6 100644 --- a/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilTest.java +++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilTest.java @@ -44,10 +44,10 @@ public void testSerializeDeserialize() throws Exception { bean1.setEnumBean(AnEnumBean.Two); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); String json = jsonUtil.serialize(bean1, JSONUtil.CACHE_BEAN_INFO_DEFAULT); - Map result = (Map) JSONUtil.deserialize(json); + Map result = (Map) new StrutsJSONReader().read(json); assertEquals("str", result.get("stringField")); assertEquals(true, result.get("booleanField")); assertEquals("s", result.get("charField")); // note: this is a @@ -73,7 +73,7 @@ public void testSerializeListOfList() throws Exception { List includeProperties = JSONUtil.processIncludePatterns(JSONUtil.asSet("listOfLists,listOfLists\\[\\d+\\]\\[\\d+\\]"), JSONUtil.REGEXP_PATTERN); JSONUtil jsonUtil = new JSONUtil(); - jsonUtil.setWriter(new DefaultJSONWriter()); + jsonUtil.setWriter(new StrutsJSONWriter()); String actual = jsonUtil.serialize(bean, null, new ArrayList(includeProperties), false, false); assertEquals("{\"listOfLists\":[[\"1\",\"2\"],[\"3\",\"4\"],[\"5\",\"6\"],[\"7\",\"8\"],[\"9\",\"0\"]]}", actual); diff --git a/plugins/json/src/test/java/org/apache/struts2/json/StrutsJSONReaderTest.java b/plugins/json/src/test/java/org/apache/struts2/json/StrutsJSONReaderTest.java new file mode 100644 index 0000000000..48c69c8654 --- /dev/null +++ b/plugins/json/src/test/java/org/apache/struts2/json/StrutsJSONReaderTest.java @@ -0,0 +1,159 @@ +/* + * 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.struts2.json; + +import org.junit.Test; + +import java.util.List; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +public class StrutsJSONReaderTest { + + @Test + public void testArrayExceedingMaxElementsThrows() { + var reader = new StrutsJSONReader(); + reader.setMaxElements(3); + String json = "[1, 2, 3, 4]"; + var ex = assertThrows(JSONException.class, () -> reader.read(json)); + assertTrue(ex.getMessage().contains("maximum allowed elements (3)")); + assertTrue(ex.getMessage().contains(JSONConstants.JSON_MAX_ELEMENTS)); + } + + @Test + public void testArrayAtExactMaxElementsAllowed() throws Exception { + var reader = new StrutsJSONReader(); + reader.setMaxElements(3); + // Exactly 3 elements: check is >= before add, so 3 elements fit (size 0,1,2 when checked) + var result = reader.read("[1, 2, 3]"); + assertNotNull(result); + assertEquals(3, ((List) result).size()); + } + + @Test + public void testObjectExceedingMaxElementsThrows() { + var reader = new StrutsJSONReader(); + reader.setMaxElements(2); + String json = "{\"a\":1, \"b\":2, \"c\":3}"; + var ex = assertThrows(JSONException.class, () -> reader.read(json)); + assertTrue(ex.getMessage().contains("maximum allowed elements (2)")); + } + + @Test + public void testNestingExceedingMaxDepthThrows() { + var reader = new StrutsJSONReader(); + reader.setMaxDepth(2); + String json = "{\"a\":{\"b\":{\"c\":1}}}"; + var ex = assertThrows(JSONException.class, () -> reader.read(json)); + assertTrue(ex.getMessage().contains("maximum allowed depth (2)")); + assertTrue(ex.getMessage().contains(JSONConstants.JSON_MAX_DEPTH)); + } + + @Test + public void testArrayNestingExceedingMaxDepthThrows() { + var reader = new StrutsJSONReader(); + reader.setMaxDepth(2); + String json = "[[[1]]]"; + var ex = assertThrows(JSONException.class, () -> reader.read(json)); + assertTrue(ex.getMessage().contains("maximum allowed depth (2)")); + } + + @Test + public void testStringAtExactMaxStringLengthAllowed() throws Exception { + var reader = new StrutsJSONReader(); + reader.setMaxStringLength(5); + // String "abcde" has length exactly 5, should be allowed (check is >) + Object result = reader.read("\"abcde\""); + assertEquals("abcde", result); + } + + @Test + public void testStringExceedingMaxStringLengthThrows() { + var reader = new StrutsJSONReader(); + reader.setMaxStringLength(5); + String json = "\"abcdefghij\""; + var ex = assertThrows(JSONException.class, () -> reader.read(json)); + assertTrue(ex.getMessage().contains("maximum allowed length (5)")); + assertTrue(ex.getMessage().contains(JSONConstants.JSON_MAX_STRING_LENGTH)); + } + + @Test + public void testObjectKeyExceedingMaxKeyLengthThrowsOnFirstKey() { + var reader = new StrutsJSONReader(); + reader.setMaxKeyLength(3); + String json = "{\"longkey\":1}"; + var ex = assertThrows(JSONException.class, () -> reader.read(json)); + assertTrue(ex.getMessage().contains("maximum allowed length (3)")); + assertTrue(ex.getMessage().contains(JSONConstants.JSON_MAX_KEY_LENGTH)); + } + + @Test + public void testObjectKeyExceedingMaxKeyLengthThrowsOnSubsequentKey() { + var reader = new StrutsJSONReader(); + reader.setMaxKeyLength(3); + // First key "a" is within limit, second key "longkey" exceeds it + String json = "{\"a\":1, \"longkey\":2}"; + var ex = assertThrows(JSONException.class, () -> reader.read(json)); + assertTrue(ex.getMessage().contains("maximum allowed length (3)")); + assertTrue(ex.getMessage().contains(JSONConstants.JSON_MAX_KEY_LENGTH)); + } + + @Test + public void testObjectKeyAtExactMaxKeyLengthAllowed() throws Exception { + var reader = new StrutsJSONReader(); + reader.setMaxKeyLength(3); + // Key "abc" has length exactly 3, should be allowed (check is >) + var result = reader.read("{\"abc\":1}"); + assertTrue(result instanceof Map); + assertEquals(1L, ((Map) result).get("abc")); + } + + @Test + public void testDefaultLimitsAllowTypicalPayload() throws Exception { + var reader = new StrutsJSONReader(); + var json = "{\"name\":\"test\", \"values\":[1, 2, 3], \"nested\":{\"key\":\"value\"}}"; + assertTrue(reader.read(json) instanceof Map); + } + + @Test + public void testDepthCounterResetsAfterParsing() throws Exception { + var reader = new StrutsJSONReader(); + reader.setMaxDepth(3); + + // First parse: uses depth up to 2 + assertTrue(reader.read("{\"a\":{\"b\":1}}") instanceof Map); + + // Second parse: should work fine if depth was reset + assertTrue(reader.read("{\"a\":{\"b\":1}}") instanceof Map); + } + + @Test + public void testMixedNestingDepth() { + var reader = new StrutsJSONReader(); + reader.setMaxDepth(2); + // array inside object inside array = depth 3, should fail at depth 2 + String json = "[{\"a\":[1]}]"; + var ex = assertThrows(JSONException.class, () -> reader.read(json)); + assertTrue(ex.getMessage().contains("maximum allowed depth")); + } +} diff --git a/plugins/json/src/test/java/org/apache/struts2/json/DefaultJSONWriterTest.java b/plugins/json/src/test/java/org/apache/struts2/json/StrutsJSONWriterTest.java similarity index 86% rename from plugins/json/src/test/java/org/apache/struts2/json/DefaultJSONWriterTest.java rename to plugins/json/src/test/java/org/apache/struts2/json/StrutsJSONWriterTest.java index 27b127c0f2..acc52dcaa8 100644 --- a/plugins/json/src/test/java/org/apache/struts2/json/DefaultJSONWriterTest.java +++ b/plugins/json/src/test/java/org/apache/struts2/json/StrutsJSONWriterTest.java @@ -22,6 +22,7 @@ import org.apache.struts2.junit.util.TestUtils; import org.junit.Test; +import java.net.URI; import java.net.URL; import java.text.SimpleDateFormat; import java.time.Instant; @@ -43,7 +44,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -public class DefaultJSONWriterTest { +public class StrutsJSONWriterTest { @Test public void testWrite() throws Exception { @@ -58,10 +59,10 @@ public void testWrite() throws Exception { bean1.setEnumField(AnEnum.ValueA); bean1.setEnumBean(AnEnumBean.Two); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); jsonWriter.setEnumAsBean(false); String json = jsonWriter.write(bean1); - TestUtils.assertEquals(DefaultJSONWriter.class.getResource("jsonwriter-write-bean-01.txt"), json); + TestUtils.assertEquals(StrutsJSONWriter.class.getResource("jsonwriter-write-bean-01.txt"), json); } @Test @@ -83,11 +84,11 @@ public void testWriteExcludeNull() throws Exception { m.put("c", "z"); bean1.setMap(m); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); jsonWriter.setEnumAsBean(false); jsonWriter.setIgnoreHierarchy(false); String json = jsonWriter.write(bean1, null, null, true); - TestUtils.assertEquals(DefaultJSONWriter.class.getResource("jsonwriter-write-bean-03.txt"), json); + TestUtils.assertEquals(StrutsJSONWriter.class.getResource("jsonwriter-write-bean-03.txt"), json); } private static class BeanWithMap extends Bean { @@ -114,13 +115,13 @@ public void testWriteAnnotatedBean() throws Exception { bean1.setLongField(100); bean1.setEnumField(AnEnum.ValueA); bean1.setEnumBean(AnEnumBean.Two); - bean1.setUrl(new URL("http://www.google.com")); + bean1.setUrl(URI.create("https://www.google.com").toURL()); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); jsonWriter.setEnumAsBean(false); jsonWriter.setIgnoreHierarchy(false); String json = jsonWriter.write(bean1); - TestUtils.assertEquals(DefaultJSONWriter.class.getResource("jsonwriter-write-bean-02.txt"), json); + TestUtils.assertEquals(StrutsJSONWriter.class.getResource("jsonwriter-write-bean-02.txt"), json); } @Test @@ -139,11 +140,11 @@ public void testWriteBeanWithList() throws Exception { errors.add("Field is required"); bean1.setErrors(errors); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); jsonWriter.setEnumAsBean(false); jsonWriter.setIgnoreHierarchy(false); String json = jsonWriter.write(bean1); - TestUtils.assertEquals(DefaultJSONWriter.class.getResource("jsonwriter-write-bean-04.txt"), json); + TestUtils.assertEquals(StrutsJSONWriter.class.getResource("jsonwriter-write-bean-04.txt"), json); } private static class BeanWithList extends Bean { @@ -178,7 +179,7 @@ public void testCanSerializeADate() throws Exception { SingleDateBean dateBean = new SingleDateBean(); dateBean.setDate(sdf.parse("2012-12-23 10:10:10 GMT")); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); jsonWriter.setEnumAsBean(false); TimeZone.setDefault(TimeZone.getTimeZone("GMT")); @@ -193,7 +194,7 @@ public void testCanSetDefaultDateFormat() throws Exception { SingleDateBean dateBean = new SingleDateBean(); dateBean.setDate(sdf.parse("2012-12-23 10:10:10 GMT")); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); jsonWriter.setEnumAsBean(false); jsonWriter.setDateFormatter("MM-dd-yyyy"); String json = jsonWriter.write(dateBean); @@ -205,7 +206,7 @@ public void testSerializeLocalDate() throws Exception { TemporalBean bean = new TemporalBean(); bean.setLocalDate(LocalDate.of(2026, 2, 27)); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); String json = jsonWriter.write(bean); assertTrue(json.contains("\"localDate\":\"2026-02-27\"")); } @@ -215,7 +216,7 @@ public void testSerializeLocalDateTime() throws Exception { TemporalBean bean = new TemporalBean(); bean.setLocalDateTime(LocalDateTime.of(2026, 2, 27, 12, 0, 0)); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); String json = jsonWriter.write(bean); assertTrue(json.contains("\"localDateTime\":\"2026-02-27T12:00:00\"")); } @@ -225,7 +226,7 @@ public void testSerializeLocalTime() throws Exception { TemporalBean bean = new TemporalBean(); bean.setLocalTime(LocalTime.of(12, 0, 0)); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); String json = jsonWriter.write(bean); assertTrue(json.contains("\"localTime\":\"12:00:00\"")); } @@ -235,7 +236,7 @@ public void testSerializeZonedDateTime() throws Exception { TemporalBean bean = new TemporalBean(); bean.setZonedDateTime(ZonedDateTime.of(2026, 2, 27, 12, 0, 0, 0, ZoneId.of("Europe/Paris"))); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); String json = jsonWriter.write(bean); assertTrue(json.contains("\"zonedDateTime\":\"2026-02-27T12:00:00+01:00[Europe\\/Paris]\"")); } @@ -245,7 +246,7 @@ public void testSerializeOffsetDateTime() throws Exception { TemporalBean bean = new TemporalBean(); bean.setOffsetDateTime(OffsetDateTime.of(2026, 2, 27, 12, 0, 0, 0, ZoneOffset.ofHours(1))); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); String json = jsonWriter.write(bean); assertTrue(json.contains("\"offsetDateTime\":\"2026-02-27T12:00:00+01:00\"")); } @@ -255,7 +256,7 @@ public void testSerializeInstant() throws Exception { TemporalBean bean = new TemporalBean(); bean.setInstant(Instant.parse("2026-02-27T11:00:00Z")); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); String json = jsonWriter.write(bean); assertTrue(json.contains("\"instant\":\"2026-02-27T11:00:00Z\"")); } @@ -265,7 +266,7 @@ public void testSerializeLocalDateWithCustomFormat() throws Exception { TemporalBean bean = new TemporalBean(); bean.setCustomFormatDate(LocalDate.of(2026, 2, 27)); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); String json = jsonWriter.write(bean); assertTrue(json.contains("\"customFormatDate\":\"27\\/02\\/2026\"")); } @@ -275,7 +276,7 @@ public void testSerializeLocalDateTimeWithCustomFormat() throws Exception { TemporalBean bean = new TemporalBean(); bean.setCustomFormatDateTime(LocalDateTime.of(2026, 2, 27, 14, 30)); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); String json = jsonWriter.write(bean); assertTrue(json.contains("\"customFormatDateTime\":\"27\\/02\\/2026 14:30\"")); } @@ -284,7 +285,7 @@ public void testSerializeLocalDateTimeWithCustomFormat() throws Exception { public void testSerializeNullTemporalField() throws Exception { TemporalBean bean = new TemporalBean(); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); String json = jsonWriter.write(bean, null, null, true); assertFalse(json.contains("\"localDate\"")); } @@ -294,7 +295,7 @@ public void testSerializeInstantWithCustomFormat() throws Exception { TemporalBean bean = new TemporalBean(); bean.setCustomFormatInstant(Instant.parse("2026-02-27T11:00:00Z")); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); String json = jsonWriter.write(bean); assertTrue(json.contains("\"customFormatInstant\":\"2026-02-27 11:00:00\"")); } @@ -308,7 +309,7 @@ public void testSerializeCalendar() throws Exception { TemporalBean bean = new TemporalBean(); bean.setCalendar(cal); - JSONWriter jsonWriter = new DefaultJSONWriter(); + JSONWriter jsonWriter = new StrutsJSONWriter(); TimeZone.setDefault(TimeZone.getTimeZone("GMT")); String json = jsonWriter.write(bean); assertTrue(json.contains("\"calendar\":\"2012-12-23T10:10:10\"")); diff --git a/plugins/json/src/test/resources/org/apache/struts2/json/jsonwriter-write-bean-02.txt b/plugins/json/src/test/resources/org/apache/struts2/json/jsonwriter-write-bean-02.txt index 31308a5244..09a52d6c5b 100644 --- a/plugins/json/src/test/resources/org/apache/struts2/json/jsonwriter-write-bean-02.txt +++ b/plugins/json/src/test/resources/org/apache/struts2/json/jsonwriter-write-bean-02.txt @@ -12,5 +12,5 @@ "longField":100, "objectField":null, "stringField":"str", - "url":"http:\/\/www.google.com" + "url":"https:\/\/www.google.com" } diff --git a/thoughts/shared/research/2026-03-16-json-plugin-configurable-limits-plan.md b/thoughts/shared/research/2026-03-16-json-plugin-configurable-limits-plan.md new file mode 100644 index 0000000000..41ac3b716c --- /dev/null +++ b/thoughts/shared/research/2026-03-16-json-plugin-configurable-limits-plan.md @@ -0,0 +1,450 @@ +--- +date: 2026-03-16T12:00:00+01:00 +topic: "JSON Plugin Configurable Limits - Implementation Plan" +tags: [plan, json-plugin, configuration, hardening, WW-5618] +status: ready +--- + +# Implementation Plan: JSON Plugin Configurable Limits + +## JIRA Issue + +**Ticket:** [WW-5618](https://issues.apache.org/jira/browse/WW-5618) + +--- + +## Key Design Decisions + +1. **Instance methods over static:** `JSONUtil.deserialize()` becomes instance methods. `JSONInterceptor` gets `JSONUtil` injected via Container (it's already a registered prototype bean). `JSONUtil` gets `JSONReader` and `JSONWriter` injected via simple `@Inject` (no manual Container lookup). + +2. **Naming convention:** Both reader and writer follow the `Struts*` naming convention for framework implementations. `JSONReader` becomes an interface with `StrutsJSONReader` as the implementation. `DefaultJSONWriter` is renamed to `StrutsJSONWriter`. Both registered in `struts-plugin.xml` as default beans. + +3. **Bean selection via `BeanSelectionProvider`:** A new `JSONBeanSelectionProvider` (extending `AbstractBeanSelectionProvider`) uses the `alias()` mechanism to resolve `JSONReader` and `JSONWriter` beans by constant-configured name. This replaces the manual two-step `container.getInstance()` lookup in `JSONUtil.setContainer()` and follows the same pattern as `VelocityBeanSelectionProvider`. Users can swap implementations by name, class name, or Spring bean ID. + +4. **Backward compatibility:** The current `JSONReader` class is used only internally — `JSONUtil.deserialize()` calls `new JSONReader()` in a static method. No external code should be extending it. The static `deserialize()` methods on `JSONUtil` will be deprecated but kept (delegating to instance methods) to avoid breaking any direct callers. + +5. **`@Inject` pattern:** All injected constants use `String` parameter type with conversion in the setter body, following the established Struts convention (e.g., `setDefaultEncoding(String val)` in `JSONInterceptor`). + +6. **Limits flow:** `JSONInterceptor` → sets limits on `JSONUtil` instance → `JSONUtil` passes limits to `JSONReader` before each `deserialize()` call. Per-action `` overrides on the interceptor take precedence over global constants. + +--- + +## Implementation Steps + +### Step 1: Add constants to `JSONConstants` + +**File:** `plugins/json/src/main/java/org/apache/struts2/json/JSONConstants.java` + +Add new constants: + +```java +public static final String JSON_READER = "struts.json.reader"; +public static final String JSON_MAX_ELEMENTS = "struts.json.maxElements"; +public static final String JSON_MAX_DEPTH = "struts.json.maxDepth"; +public static final String JSON_MAX_LENGTH = "struts.json.maxLength"; +public static final String JSON_MAX_STRING_LENGTH = "struts.json.maxStringLength"; +public static final String JSON_MAX_KEY_LENGTH = "struts.json.maxKeyLength"; +``` + +**Expected outcome:** Constants available for injection and XML configuration. + +--- + +### Step 2: Update `JSONConstantConfig` + +**File:** `plugins/json/src/main/java/org/apache/struts2/json/config/entities/JSONConstantConfig.java` + +Add fields, getters, setters, and `getAllAsStringsMap()` entries for each new constant. Follow the existing pattern (`jsonWriter`, `jsonDateFormat`): + +```java +private BeanConfig jsonReader; +private Integer jsonMaxElements; +private Integer jsonMaxDepth; +private Integer jsonMaxLength; +private Integer jsonMaxStringLength; +private Integer jsonMaxKeyLength; +``` + +**Expected outcome:** New constants wired into the ConstantConfig system. + +--- + +### Step 3: Extract `JSONReader` interface and create `StrutsJSONReader` + +**Current file:** `plugins/json/src/main/java/org/apache/struts2/json/JSONReader.java` +**New file:** `plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONReader.java` + +#### 3a: Create `JSONReader` interface + +Replace the current class with an interface: + +```java +public interface JSONReader { + + int DEFAULT_MAX_ELEMENTS = 10_000; + int DEFAULT_MAX_DEPTH = 64; + int DEFAULT_MAX_STRING_LENGTH = 262_144; // 256KB + int DEFAULT_MAX_KEY_LENGTH = 512; + + Object read(String string) throws JSONException; + + void setMaxElements(int maxElements); + void setMaxDepth(int maxDepth); + void setMaxStringLength(int maxStringLength); + void setMaxKeyLength(int maxKeyLength); +} +``` + +#### 3b: Create `StrutsJSONReader` implementation + +Rename current `JSONReader` class to `StrutsJSONReader implements JSONReader`. Add: + +- Limit fields with defaults from the interface constants +- A `depth` counter field, incremented on entry to `array()`/`object()`, decremented on exit (use try/finally) +- In `array()`: check `ret.size() >= maxElements` before `ret.add()`, throw `JSONException` if exceeded +- In `object()`: check `ret.size() >= maxElements` before `ret.put()`, throw `JSONException` if exceeded +- In `read()`: check `depth >= maxDepth` before calling `array()`/`object()`, throw `JSONException` if exceeded +- In `string()`: check `buf.length() >= maxStringLength` in the character loop, throw `JSONException` if exceeded +- In `object()`: check key length against `maxKeyLength` after reading key string + +Error messages should be clear and include the limit value, e.g.: +`"JSON array exceeds maximum allowed elements (10000). Use struts.json.maxElements to increase the limit."` + +**Expected outcome:** `JSONReader` is an interface; `StrutsJSONReader` enforces configurable bounds. + +--- + +### Step 4: Rename `DefaultJSONWriter` to `StrutsJSONWriter` + +**Current file:** `plugins/json/src/main/java/org/apache/struts2/json/DefaultJSONWriter.java` +**New file:** `plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONWriter.java` + +Rename the class from `DefaultJSONWriter` to `StrutsJSONWriter`. This is a mechanical rename — the `JSONWriter` interface stays unchanged. + +Changes required: +- Rename class file and class declaration +- Update `struts-plugin.xml` bean registration: `class="org.apache.struts2.json.StrutsJSONWriter"` +- Update all test references (~40 occurrences across `JSONResultTest.java`, `DefaultJSONWriterTest.java`, `JSONUtilTest.java`, `JSONEnumTest.java`) +- Rename `DefaultJSONWriterTest.java` to `StrutsJSONWriterTest.java` +- Update resource references (e.g., `DefaultJSONWriter.class.getResource(...)` → `StrutsJSONWriter.class.getResource(...)`) + +**Expected outcome:** Writer follows the same `Struts*` naming convention as the reader. `JSONWriter` interface is unchanged — no impact on custom implementations. + +--- + +### Step 5: Create `JSONBeanSelectionProvider` + +**New file:** `plugins/json/src/main/java/org/apache/struts2/json/JSONBeanSelectionProvider.java` + +Create a bean selection provider following the `VelocityBeanSelectionProvider` pattern: + +```java +package org.apache.struts2.json; + +import org.apache.struts2.config.AbstractBeanSelectionProvider; +import org.apache.struts2.config.ConfigurationException; +import org.apache.struts2.inject.ContainerBuilder; +import org.apache.struts2.inject.Scope; +import org.apache.struts2.util.location.LocatableProperties; + +public class JSONBeanSelectionProvider extends AbstractBeanSelectionProvider { + + @Override + public void register(ContainerBuilder builder, LocatableProperties props) + throws ConfigurationException { + alias(JSONReader.class, JSONConstants.JSON_READER, builder, props, Scope.PROTOTYPE); + alias(JSONWriter.class, JSONConstants.JSON_WRITER, builder, props, Scope.PROTOTYPE); + } +} +``` + +This uses the standard `alias()` mechanism from `AbstractBeanSelectionProvider` which: +1. Reads the constant value (e.g., `struts.json.reader` → `"struts"`) +2. Finds the bean registered under that name +3. Aliases it to `Container.DEFAULT_NAME` so plain `@Inject` resolves it +4. Falls back to class name loading or Spring bean ID delegation if the name isn't a registered bean + +**Expected outcome:** `JSONReader` and `JSONWriter` beans are selectable via constants using the standard Struts bean aliasing mechanism. Users can swap implementations by bean name, fully qualified class name, or Spring bean ID. + +--- + +### Step 6: Update `JSONUtil` to use instance methods and simple `@Inject` + +**File:** `plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java` + +#### 6a: Replace manual Container lookup with simple `@Inject` + +Remove the existing `setContainer()` method with its manual two-step resolution. Replace with direct injection (the `BeanSelectionProvider` aliases handle the name resolution): + +```java +private JSONReader reader; +private JSONWriter writer; + +@Inject +public void setReader(JSONReader reader) { + this.reader = reader; +} + +@Inject +public void setWriter(JSONWriter writer) { + this.writer = writer; +} + +public JSONReader getReader() { + return reader; +} +``` + +#### 6b: Add instance `deserialize()` methods with `maxLength` check + +```java +public Object deserialize(Reader reader, int maxLength) throws JSONException { + BufferedReader bufferReader = new BufferedReader(reader); + String line; + StringBuilder buffer = new StringBuilder(); + try { + while ((line = bufferReader.readLine()) != null) { + buffer.append(line); + if (buffer.length() > maxLength) { + throw new JSONException("JSON input exceeds maximum allowed length (" + + maxLength + "). Use struts.json.maxLength to increase the limit."); + } + } + } catch (IOException e) { + throw new JSONException(e); + } + return this.reader.read(buffer.toString()); +} +``` + +#### 6c: Deprecate static `deserialize()` methods + +Keep existing static methods but mark `@Deprecated` and delegate internally (create a default `StrutsJSONReader` for backward compatibility): + +```java +@Deprecated +public static Object deserialize(String json) throws JSONException { + StrutsJSONReader reader = new StrutsJSONReader(); + return reader.read(json); +} +``` + +**Expected outcome:** `JSONUtil` uses simple `@Inject` for both reader and writer; limits flow through instance methods; static API preserved but deprecated. + +--- + +### Step 7: Wire limits into `JSONInterceptor` + +**File:** `plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java` + +#### 7a: Inject `JSONUtil` instance instead of static access + +```java +private JSONUtil jsonUtil; + +@Inject +public void setJsonUtil(JSONUtil jsonUtil) { + this.jsonUtil = jsonUtil; +} +``` + +#### 7b: Add limit fields with `@Inject` from constants + +```java +private int maxElements = JSONReader.DEFAULT_MAX_ELEMENTS; +private int maxDepth = JSONReader.DEFAULT_MAX_DEPTH; +private int maxLength = 2_097_152; // 2MB +private int maxStringLength = JSONReader.DEFAULT_MAX_STRING_LENGTH; +private int maxKeyLength = JSONReader.DEFAULT_MAX_KEY_LENGTH; + +@Inject(value = JSONConstants.JSON_MAX_ELEMENTS, required = false) +public void setMaxElements(String maxElements) { + this.maxElements = Integer.parseInt(maxElements); +} + +@Inject(value = JSONConstants.JSON_MAX_DEPTH, required = false) +public void setMaxDepth(String maxDepth) { + this.maxDepth = Integer.parseInt(maxDepth); +} + +// ... same pattern for maxLength, maxStringLength, maxKeyLength +``` + +#### 7c: Update `intercept()` to use instance `jsonUtil` and pass limits + +Replace the static call: +```java +// Before: +Object obj = JSONUtil.deserialize(request.getReader()); + +// After: +jsonUtil.getReader().setMaxElements(maxElements); +jsonUtil.getReader().setMaxDepth(maxDepth); +jsonUtil.getReader().setMaxStringLength(maxStringLength); +jsonUtil.getReader().setMaxKeyLength(maxKeyLength); +Object obj = jsonUtil.deserialize(request.getReader(), maxLength); +``` + +Do the same for the SMD deserialization path (line 136). + +Note: Since `JSONUtil` is prototype-scoped, the reader instance is per-interceptor invocation when injected properly. But to be safe, limits should be set before each deserialization call. + +#### 7d: Also update `JSONResult` and `JSONValidationInterceptor` if they use static `JSONUtil.deserialize()` + +Check these classes and update them to use injected `JSONUtil` if they call the static deserialize methods. + +**Expected outcome:** Limits flow from interceptor → JSONUtil → JSONReader per request. Configurable globally and per-action. + +--- + +### Step 8: Register beans and defaults in `struts-plugin.xml` + +**File:** `plugins/json/src/main/resources/struts-plugin.xml` + +```xml + + + + + + + + + + + + + + + + + + + + +``` + +**Expected outcome:** Sensible defaults applied out-of-the-box; all values overridable. Users can swap implementations via constants. + +--- + +### Step 9: Write tests + +**File:** `plugins/json/src/test/java/org/apache/struts2/json/StrutsJSONReaderTest.java` (new, for the implementation) +**File:** `plugins/json/src/test/java/org/apache/struts2/json/JSONReaderTest.java` (existing, keep for backward compat of deprecated static API) + +#### Unit tests for `StrutsJSONReader`: +- [ ] Array with elements under limit parses successfully +- [ ] Array exceeding `maxElements` throws `JSONException` with descriptive message +- [ ] Object exceeding `maxElements` throws `JSONException` +- [ ] Nesting within `maxDepth` parses successfully +- [ ] Nesting exceeding `maxDepth` throws `JSONException` +- [ ] String within `maxStringLength` parses successfully +- [ ] String exceeding `maxStringLength` throws `JSONException` +- [ ] Object key exceeding `maxKeyLength` throws `JSONException` +- [ ] Default limits allow typical JSON payloads (regression) +- [ ] Custom limits via setters work correctly +- [ ] Depth counter resets correctly after parsing (no state leakage) + +#### Unit tests for `JSONUtil` instance methods: +- [ ] Input exceeding `maxLength` throws `JSONException` before parsing begins +- [ ] Input within `maxLength` parses successfully +- [ ] Deprecated static methods still work (backward compat) + +#### Integration test (if feasible): +- [ ] Per-action `` override applies different limits than global default + +**Expected outcome:** All limit boundaries tested; existing tests continue to pass. + +--- + +### Step 10: Update documentation + +- Update JSON plugin documentation page to describe new configuration options +- Add migration notes: mention that new defaults may reject unusually large payloads +- Document per-action override pattern with XML example +- Document custom `JSONReader`/`JSONWriter` implementation pattern + +--- + +## Files Modified (Summary) + +| File | Change | +|------|--------| +| `JSONConstants.java` | Add 6 new constants | +| `JSONConstantConfig.java` | Add fields, getters, setters for new constants | +| `JSONReader.java` | **Rewrite** — becomes an interface with limit setters and defaults | +| `StrutsJSONReader.java` | **New** — implementation with depth tracking and bounds checks | +| `DefaultJSONWriter.java` | **Rename** → `StrutsJSONWriter.java` (class name + file) | +| `JSONBeanSelectionProvider.java` | **New** — bean aliasing for reader/writer via constants | +| `JSONUtil.java` | Replace manual Container lookup with `@Inject`, add instance `deserialize()`, deprecate static ones | +| `JSONInterceptor.java` | Inject `JSONUtil`, add `@Inject` + setter methods for limits | +| `struts-plugin.xml` | Add `bean-selection`, register `StrutsJSONReader`, rename writer bean, add defaults | +| `StrutsJSONReaderTest.java` | **New** — test cases for all limits | +| `DefaultJSONWriterTest.java` | **Rename** → `StrutsJSONWriterTest.java`, update all references | +| `JSONResultTest.java` | Update `DefaultJSONWriter` → `StrutsJSONWriter` (~25 occurrences) | +| `JSONUtilTest.java` | Update `DefaultJSONWriter` → `StrutsJSONWriter` references | +| `JSONEnumTest.java` | Update `DefaultJSONWriter` → `StrutsJSONWriter` references | +| `JSONReaderTest.java` | Keep existing tests, verify backward compat | + +## Configuration Examples + +### Global defaults (struts.xml) +```xml + + + +``` + +### Per-action override +```xml + + + 100000 + 10 + 10485760 + + + +``` + +### Custom JSONReader implementation +```xml + + +``` + +### Custom JSONWriter implementation +```xml + + +``` + +### Custom implementation via Spring bean ID +```xml + + +``` + +## Risks and Considerations + +1. **Breaking change — `JSONReader` becomes an interface:** Code that directly instantiated `new JSONReader()` will break. However, `JSONReader` was only used internally by `JSONUtil.deserialize()` static methods. External code calling the static API will still work via the deprecated path. Risk: **low**. + +2. **Breaking change — static `deserialize()` deprecated:** Callers using `JSONUtil.deserialize(String)` or `JSONUtil.deserialize(Reader)` statically will get deprecation warnings but the methods still work. They just won't have limit enforcement. Risk: **none** (functional). + +3. **New defaults may reject large payloads:** A payload with >10,000 array elements or >64 nesting depth will now be rejected. Mitigation: defaults are generous for typical use; clear error messages include the constant name to override. Risk: **low**. + +4. **Performance:** Bounds checks add negligible overhead (integer comparison per element/depth increment). + +5. **Breaking change — `DefaultJSONWriter` renamed to `StrutsJSONWriter`:** Code that directly references `DefaultJSONWriter` (e.g., `new DefaultJSONWriter()` in tests or custom code) will break. However, `DefaultJSONWriter` was never part of the public API — users should reference the `JSONWriter` interface. Bean registration in `struts-plugin.xml` is internal. Risk: **low** — affects custom code that bypasses the Container. + +6. **Thread safety:** `StrutsJSONReader` is not thread-safe (instance fields for parser state). Registered as `scope="prototype"` — same as `StrutsJSONWriter`. Each deserialization gets a fresh instance. + +7. **Prototype scope and limit setting:** Since `JSONUtil` is prototype-scoped and holds a prototype `JSONReader`, the interceptor sets limits on the reader before each deserialization. This is safe because each interceptor invocation gets its own `JSONUtil` instance from the Container. + +8. **`BeanSelectionProvider` ordering:** The `` element in `struts-plugin.xml` must appear so that the beans it references (`StrutsJSONReader`, `StrutsJSONWriter`) are already registered. Since `` elements are processed before ``, this works naturally.