This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch feat/WW-5618-json-configurable-limits-s6 in repository https://gitbox.apache.org/repos/asf/struts.git
commit c4c6474c9703ae9929ead3694754316821dc63c8 Author: Lukasz Lenart <[email protected]> AuthorDate: Tue Mar 17 17:13:28 2026 +0100 WW-5618 feat(json): add configurable limits to JSON plugin for DoS prevention Add configurable limits to the JSON plugin to prevent denial-of-service attacks via malicious JSON payloads. Limits are enforced directly in the existing JSONReader class without breaking backward compatibility (no interface extraction or class renames). New configurable constants (struts-plugin.xml defaults): - struts.json.maxElements (10000) - per-container element count - struts.json.maxDepth (64) - maximum nesting depth - struts.json.maxLength (2097152) - maximum input length in chars - struts.json.maxStringLength (262144) - maximum string value length - struts.json.maxKeyLength (512) - maximum object key length Co-Authored-By: Claude Opus 4.6 <[email protected]> --- .../org/apache/struts2/json/JSONConstants.java | 6 + .../org/apache/struts2/json/JSONInterceptor.java | 48 ++++- .../java/org/apache/struts2/json/JSONReader.java | 125 +++++++++---- .../java/org/apache/struts2/json/JSONUtil.java | 38 ++++ plugins/json/src/main/resources/struts-plugin.xml | 5 + .../apache/struts2/json/JSONReaderLimitsTest.java | 196 +++++++++++++++++++++ .../java/org/apache/struts2/json/JSONUtilTest.java | 41 +++++ 7 files changed, 422 insertions(+), 37 deletions(-) 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 f5cb292bd..d32be5af6 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 @@ -31,4 +31,10 @@ public class JSONConstants { public static final String JSON_WRITER = "struts.json.writer"; 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 e92620f20..9096eeb9c 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 = new JSONUtil(); + private int maxElements = 10000; + private int maxDepth = 64; + private int maxLength = 2097152; + private int maxStringLength = 262144; + private int maxKeyLength = 512; + @SuppressWarnings("unchecked") public String intercept(ActionInvocation invocation) throws Exception { HttpServletRequest request = ServletActionContext.getRequest(); @@ -91,7 +98,8 @@ public class JSONInterceptor extends AbstractInterceptor { if (jsonContentType.equalsIgnoreCase(requestContentType)) { // load JSON object - Object obj = JSONUtil.deserialize(request.getReader()); + Object obj = jsonUtil.deserializeInput(request.getReader(), maxLength, maxElements, maxDepth, + maxStringLength, maxKeyLength); // JSON array (this.root cannot be null in this case) if(obj instanceof List && this.root != null) { @@ -133,7 +141,8 @@ public class JSONInterceptor extends AbstractInterceptor { Object result; if (this.enableSMD) { // load JSON object - Object obj = JSONUtil.deserialize(request.getReader()); + Object obj = jsonUtil.deserializeInput(request.getReader(), maxLength, maxElements, maxDepth, + maxStringLength, maxKeyLength); if (obj instanceof Map) { Map smd = (Map) obj; @@ -168,9 +177,8 @@ public class JSONInterceptor extends AbstractInterceptor { result = rpcResponse; } - JSONUtil jsonUtil = invocation.getInvocationContext().getContainer().getInstance(JSONUtil.class); - - String json = jsonUtil.serialize(result, excludeProperties, getIncludeProperties(), + JSONUtil smdJsonUtil = invocation.getInvocationContext().getContainer().getInstance(JSONUtil.class); + String json = smdJsonUtil.serialize(result, excludeProperties, getIncludeProperties(), ignoreHierarchy, excludeNullProperties); json = addCallbackIfApplicable(request, json); boolean writeGzip = enableGZIP && JSONUtil.isGzipInRequest(request); @@ -423,6 +431,36 @@ public class JSONInterceptor extends AbstractInterceptor { setDebug(BooleanUtils.toBoolean(mode)); } + @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); + } + /** * Sets a comma-delimited list of regular expressions to match properties * that should be excluded from the JSON output. 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 4fb1c406e..9ff06cc38 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 @@ -48,11 +48,38 @@ public class JSONReader { escapes.put('t', '\t'); } + private static final int DEFAULT_MAX_ELEMENTS = 10000; + private static final int DEFAULT_MAX_DEPTH = 64; + private static final int DEFAULT_MAX_STRING_LENGTH = 262144; + private static final int DEFAULT_MAX_KEY_LENGTH = 512; + + 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; + private CharacterIterator it; private char c; private Object token; private StringBuilder buf = new StringBuilder(); + public void setMaxElements(int maxElements) { + this.maxElements = maxElements; + } + + public void setMaxDepth(int maxDepth) { + this.maxDepth = maxDepth; + } + + public void setMaxStringLength(int maxStringLength) { + this.maxStringLength = maxStringLength; + } + + public void setMaxKeyLength(int maxKeyLength) { + this.maxKeyLength = maxKeyLength; + } + protected char next() { this.c = this.it.next(); @@ -124,29 +151,51 @@ public class JSONReader { @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(); + this.depth++; + if (this.depth > this.maxDepth) { + throw new JSONException("JSON object depth exceeds maximum allowed depth of " + this.maxDepth); + } + try { + Map ret = new HashMap(); + Object next = this.read(); + if (next != OBJECT_END) { + String key = (String) next; + while (this.token != OBJECT_END) { + validateKeyLength(key); + this.read(); // should be a colon + + if (this.token != OBJECT_END) { + ret.put(key, this.read()); + validateElementCount(ret.size()); + + if (this.read() == COMMA) { + Object name = this.read(); + + if (name instanceof String) { + key = (String) name; + } else + throw buildInvalidInputException(); + } } } } + + return ret; + } finally { + this.depth--; } + } - return ret; + private void validateKeyLength(String key) throws JSONException { + if (key != null && key.length() > this.maxKeyLength) { + throw new JSONException("JSON key length " + key.length() + " exceeds maximum allowed length of " + this.maxKeyLength); + } + } + + private void validateElementCount(int count) throws JSONException { + if (count > this.maxElements) { + throw new JSONException("JSON element count exceeds maximum allowed count of " + this.maxElements); + } } protected JSONException buildInvalidInputException() { @@ -156,21 +205,30 @@ public class JSONReader { @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(); - } + this.depth++; + if (this.depth > this.maxDepth) { + throw new JSONException("JSON array depth exceeds maximum allowed depth of " + this.maxDepth); } + try { + List ret = new ArrayList(); + Object value = this.read(); + + while (this.token != ARRAY_END) { + ret.add(value); + validateElementCount(ret.size()); + + Object read = this.read(); + if (read == COMMA) { + value = this.read(); + } else if (read != ARRAY_END) { + throw buildInvalidInputException(); + } + } - return ret; + return ret; + } finally { + this.depth--; + } } protected Object number() throws JSONException { @@ -215,7 +273,7 @@ public class JSONReader { } } - protected Object string(char quote) { + protected Object string(char quote) throws JSONException { this.buf.setLength(0); while ((this.c != quote) && (this.c != CharacterIterator.DONE)) { @@ -234,6 +292,9 @@ public class JSONReader { } else { this.add(); } + if (this.buf.length() > this.maxStringLength) { + throw new JSONException("JSON string length exceeds maximum allowed length of " + this.maxStringLength); + } } this.next(); 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 f8c5683d6..10f063ec8 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 @@ -323,6 +323,44 @@ public class JSONUtil { return deserialize(buffer.toString()); } + /** + * Deserializes a JSON object from a Reader with configurable limits to prevent DoS attacks. + * + * @param reader Reader to read JSON string from + * @param maxLength maximum allowed input length in characters + * @param maxElements maximum allowed number of elements in a single object/array + * @param maxDepth maximum allowed nesting depth + * @param maxStringLength maximum allowed string value length + * @param maxKeyLength maximum allowed key length + * @return deserialized object + * @throws JSONException when limits are exceeded or input is malformed + */ + public Object deserializeInput(Reader reader, int maxLength, int maxElements, int maxDepth, + int maxStringLength, int maxKeyLength) throws JSONException { + BufferedReader bufferReader = new BufferedReader(reader); + StringBuilder buffer = new StringBuilder(); + String line; + + try { + while ((line = bufferReader.readLine()) != null) { + buffer.append(line); + if (buffer.length() > maxLength) { + throw new JSONException("JSON input length exceeds maximum allowed length of " + maxLength); + } + } + } catch (IOException e) { + throw new JSONException(e); + } + + JSONReader jsonReader = new JSONReader(); + jsonReader.setMaxElements(maxElements); + jsonReader.setMaxDepth(maxDepth); + jsonReader.setMaxStringLength(maxStringLength); + jsonReader.setMaxKeyLength(maxKeyLength); + + return jsonReader.read(buffer.toString()); + } + public static void writeJSONToResponse(SerializationParams serializationParams) throws IOException { StringBuilder stringBuilder = new StringBuilder(); if (StringUtils.isNotBlank(serializationParams.getSerializedJSON())) diff --git a/plugins/json/src/main/resources/struts-plugin.xml b/plugins/json/src/main/resources/struts-plugin.xml index 1291246a7..6df094a0f 100644 --- a/plugins/json/src/main/resources/struts-plugin.xml +++ b/plugins/json/src/main/resources/struts-plugin.xml @@ -27,6 +27,11 @@ <bean type="org.apache.struts2.json.JSONWriter" name="struts" class="org.apache.struts2.json.DefaultJSONWriter" scope="prototype"/> <constant name="struts.json.writer" value="struts"/> + <constant name="struts.json.maxElements" value="10000"/> + <constant name="struts.json.maxDepth" value="64"/> + <constant name="struts.json.maxLength" value="2097152"/> + <constant name="struts.json.maxStringLength" value="262144"/> + <constant name="struts.json.maxKeyLength" value="512"/> <!-- TODO: Make DefaultJSONWriter thread-safe to remove "prototype"s --> <bean class="org.apache.struts2.json.JSONUtil" scope="prototype"/> diff --git a/plugins/json/src/test/java/org/apache/struts2/json/JSONReaderLimitsTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONReaderLimitsTest.java new file mode 100644 index 000000000..2cc49e4f2 --- /dev/null +++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONReaderLimitsTest.java @@ -0,0 +1,196 @@ +/* + * 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 static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Tests for JSON reader configurable limits (WW-5618). + */ +public class JSONReaderLimitsTest { + + // --- maxDepth --- + + @Test + public void testMaxDepthExceededByNestedObjects() { + JSONReader reader = new JSONReader(); + reader.setMaxDepth(2); + // 3 levels of nesting exceeds limit of 2 + try { + reader.read("{\"a\":{\"b\":{\"c\":1}}}"); + fail("Expected JSONException for exceeding max depth"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("2")); + } + } + + @Test + public void testMaxDepthExceededByNestedArrays() { + JSONReader reader = new JSONReader(); + reader.setMaxDepth(2); + try { + reader.read("[[[1]]]"); + fail("Expected JSONException for exceeding max depth"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("2")); + } + } + + @Test + public void testMaxDepthAtBoundaryIsAllowed() throws JSONException { + JSONReader reader = new JSONReader(); + // 3 levels of nesting, limit is exactly 3 — should pass + reader.setMaxDepth(3); + Object result = reader.read("{\"a\":{\"b\":{\"c\":1}}}"); + assertNotNull(result); + } + + @Test + public void testMaxDepthOneBelowBoundaryIsRejected() { + JSONReader reader = new JSONReader(); + // 3 levels of nesting, limit is 2 — should fail + reader.setMaxDepth(2); + try { + reader.read("{\"a\":{\"b\":{\"c\":1}}}"); + fail("Expected JSONException"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("depth")); + } + } + + // --- maxElements (per-container) --- + + @Test + public void testMaxElementsExceededInObject() { + JSONReader reader = new JSONReader(); + reader.setMaxElements(2); + // 3 keys in one object exceeds per-container limit of 2 + try { + reader.read("{\"a\":1,\"b\":2,\"c\":3}"); + fail("Expected JSONException for exceeding max elements"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("2")); + } + } + + @Test + public void testMaxElementsExceededInArray() { + JSONReader reader = new JSONReader(); + reader.setMaxElements(2); + try { + reader.read("[1,2,3]"); + fail("Expected JSONException for exceeding max elements"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("2")); + } + } + + @Test + public void testMaxElementsAtBoundaryIsAllowed() throws JSONException { + JSONReader reader = new JSONReader(); + // exactly 3 elements, limit is 3 — should pass + reader.setMaxElements(3); + Object result = reader.read("{\"a\":1,\"b\":2,\"c\":3}"); + assertNotNull(result); + } + + @Test + public void testMaxElementsIsPerContainerNotGlobal() throws JSONException { + JSONReader reader = new JSONReader(); + // limit is 2; two containers each with 2 elements — should pass + // because the limit applies per-container, not globally + reader.setMaxElements(2); + Object result = reader.read("{\"a\":[1,2],\"b\":[3,4]}"); + assertNotNull(result); + } + + // --- maxStringLength (applies to both values and keys) --- + + @Test + public void testMaxStringLengthExceededByValue() { + JSONReader reader = new JSONReader(); + reader.setMaxStringLength(5); + // "abcdef" is 6 chars, exceeds limit of 5 + try { + reader.read("{\"k\":\"abcdef\"}"); + fail("Expected JSONException for exceeding max string length"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("5")); + } + } + + @Test + public void testMaxStringLengthExceededByKey() { + JSONReader reader = new JSONReader(); + reader.setMaxStringLength(3); + // key "longkey" is 7 chars, parsed through string() before validateKeyLength + try { + reader.read("{\"longkey\":1}"); + fail("Expected JSONException for exceeding max string length on key"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("string length") || e.getMessage().contains("key length")); + } + } + + @Test + public void testMaxStringLengthAtBoundaryIsAllowed() throws JSONException { + JSONReader reader = new JSONReader(); + // "abcdef" is 6 chars, limit is 6 — should pass + reader.setMaxStringLength(6); + Object result = reader.read("{\"k\":\"abcdef\"}"); + assertNotNull(result); + } + + // --- maxKeyLength --- + + @Test + public void testMaxKeyLengthExceeded() { + JSONReader reader = new JSONReader(); + reader.setMaxKeyLength(3); + // "longkey" is 7 chars, exceeds limit of 3 + try { + reader.read("{\"longkey\":1}"); + fail("Expected JSONException for exceeding max key length"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("key length") || e.getMessage().contains("string length")); + } + } + + @Test + public void testMaxKeyLengthAtBoundaryIsAllowed() throws JSONException { + JSONReader reader = new JSONReader(); + // "key" is 3 chars, limit is 3 — should pass + reader.setMaxKeyLength(3); + Object result = reader.read("{\"key\":1}"); + assertNotNull(result); + } + + // --- smoke test --- + + @Test + public void testDefaultLimitsAcceptTypicalPayload() throws JSONException { + JSONReader reader = new JSONReader(); + Object result = reader.read("{\"name\":\"test\",\"values\":[1,2,3],\"nested\":{\"a\":true}}"); + assertNotNull(result); + } +} 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 d03d69e99..45a07fd7f 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 @@ -18,6 +18,7 @@ */ package org.apache.struts2.json; +import java.io.StringReader; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -79,4 +80,44 @@ public class JSONUtilTest extends TestCase { assertEquals("{\"listOfLists\":[[\"1\",\"2\"],[\"3\",\"4\"],[\"5\",\"6\"],[\"7\",\"8\"],[\"9\",\"0\"]]}", actual); } + public void testDeserializeInputWithinLimits() throws Exception { + JSONUtil jsonUtil = new JSONUtil(); + String json = "{\"name\":\"test\",\"value\":123}"; + Object result = jsonUtil.deserializeInput(new StringReader(json), 1000, 100, 10, 1000, 100); + assertNotNull(result); + assertTrue(result instanceof Map); + assertEquals("test", ((Map) result).get("name")); + } + + public void testDeserializeInputMaxLengthExceeded() throws Exception { + JSONUtil jsonUtil = new JSONUtil(); + String json = "{\"name\":\"test\",\"value\":123}"; + try { + jsonUtil.deserializeInput(new StringReader(json), 10, 100, 10, 1000, 100); + fail("Expected JSONException for exceeding max length"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("10")); + } + } + + public void testDeserializeInputMaxLengthAtBoundaryIsAllowed() throws Exception { + JSONUtil jsonUtil = new JSONUtil(); + String json = "{\"a\":1}"; + // length is 7, limit is 7 — should pass + Object result = jsonUtil.deserializeInput(new StringReader(json), 7, 100, 10, 1000, 100); + assertNotNull(result); + } + + public void testDeserializeInputPropagatesReaderLimits() throws Exception { + JSONUtil jsonUtil = new JSONUtil(); + // depth 3, but maxDepth is 2 + String json = "{\"a\":{\"b\":{\"c\":1}}}"; + try { + jsonUtil.deserializeInput(new StringReader(json), 10000, 100, 2, 1000, 100); + fail("Expected JSONException for exceeding max depth"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("depth")); + } + } + }
