From 34df73736b431b03aa1db94ce683bd37f031f3a5 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 29 Jul 2026 08:42:48 +0200 Subject: [PATCH 1/2] WW-5666 fix(json): apply the input length limit while reading The configured JSON input length limit was evaluated after accumulating each line of input. It is now evaluated as the input is read, in fixed-size chunks, so enforcement no longer varies with the structure of the input. Line terminators are no longer stripped while reading. They are insignificant whitespace between tokens, but an unescaped control character inside a string value is now preserved rather than silently removed. --- .../org/apache/struts2/json/JSONUtil.java | 13 +- .../struts2/json/JSONUtilInputLimitTest.java | 114 ++++++++++++++++++ 2 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 plugins/json/src/test/java/org/apache/struts2/json/JSONUtilInputLimitTest.java 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 78bceaacf3..e1c20f7b1c 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 @@ -59,6 +59,9 @@ public class JSONUtil { private static final Logger LOG = LogManager.getLogger(JSONUtil.class); + /** Chunk size used to read input incrementally while applying the length limit. */ + private static final int READ_CHUNK_SIZE = 8192; + private JSONReader reader; private JSONWriter writer; @@ -297,13 +300,15 @@ public void serialize(Writer writer, Object object, Collection excludeP * @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(); + char[] chunk = new char[READ_CHUNK_SIZE]; try { - while ((line = bufferReader.readLine()) != null) { - buffer.append(line); + int read; + // Apply the limit while reading rather than afterwards, so input that contains no + // line terminator is not accumulated in full before the limit can be evaluated. + while ((read = reader.read(chunk)) != -1) { + buffer.append(chunk, 0, read); if (buffer.length() > maxLength) { throw new JSONException("JSON input exceeds maximum allowed length (" + maxLength + "). Use " + JSONConstants.JSON_MAX_LENGTH + " to increase the limit."); diff --git a/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilInputLimitTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilInputLimitTest.java new file mode 100644 index 0000000000..4dbe70b251 --- /dev/null +++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilInputLimitTest.java @@ -0,0 +1,114 @@ +/* + * 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.io.Reader; +import java.io.StringReader; +import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +/** + * Verifies that {@link JSONUtil#deserializeInput(Reader, int)} applies the configured input length + * limit while reading, bounding how much input is consumed before the limit takes effect, and that + * input within the limit still parses. + */ +public class JSONUtilInputLimitTest { + + /** + * Emits {@code total} characters with no line terminator anywhere, and records how many + * characters the caller actually consumed. + */ + private static final class UnterminatedReader extends Reader { + private final long total; + private final AtomicLong consumed; + private long produced = 0; + + UnterminatedReader(long total, AtomicLong consumed) { + this.total = total; + this.consumed = consumed; + } + + @Override + public int read(char[] cbuf, int off, int len) { + if (produced >= total) { + return -1; + } + int count = (int) Math.min(len, total - produced); + for (int i = 0; i < count; i++) { + cbuf[off + i] = 'a'; + } + produced += count; + consumed.addAndGet(count); + return count; + } + + @Override + public void close() { + // characters are generated on demand, so there is nothing to release + } + } + + @Test + public void inputWithoutLineTerminatorIsLimitedWhileReading() { + int maxLength = 1024; + long inputSize = 64L * 1024 * 1024; + AtomicLong consumed = new AtomicLong(); + + JSONUtil util = new JSONUtil(); + Reader input = new UnterminatedReader(inputSize, consumed); + + assertThrows(JSONException.class, () -> util.deserializeInput(input, maxLength)); + + long read = consumed.get(); + // Reading proceeds in chunks, so a single chunk of overshoot beyond the limit is expected. + assertTrue("Consumed " + read + " characters for a limit of " + maxLength, + read < maxLength + 65_536L); + } + + @Test + public void inputWithinLimitIsParsed() throws JSONException { + JSONUtil util = new JSONUtil(); + util.setReader(new StrutsJSONReader()); + + Object result = util.deserializeInput(new StringReader("{\"a\":1, \"b\":\"hello\"}"), 1024); + + assertTrue("Expected a parsed JSON object", result instanceof Map); + assertEquals(1L, ((Map) result).get("a")); + assertEquals("hello", ((Map) result).get("b")); + } + + @Test + public void inputSpanningMultipleLinesIsParsed() throws JSONException { + JSONUtil util = new JSONUtil(); + util.setReader(new StrutsJSONReader()); + + // Line terminators between tokens are insignificant whitespace to the reader. + Object result = util.deserializeInput(new StringReader("{\n\"a\":1,\n\"b\":2\n}"), 1024); + + assertTrue("Expected a parsed JSON object", result instanceof Map); + assertEquals(1L, ((Map) result).get("a")); + assertEquals(2L, ((Map) result).get("b")); + } +} From 2c8243678da4d2b4933f9f50f2b68945ec27fffe Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 29 Jul 2026 08:42:48 +0200 Subject: [PATCH 2/2] WW-5666 fix(core): bound the CSP report body read and make the limit configurable The report body was read without an upper bound. It is now read up to a limit defaulting to 8192 characters and configurable via setMaxReportSize. A body above the limit is discarded with a warning instead of being processed. The whole body up to the limit is now passed to processReport rather than only its first line, and an empty body is passed as an empty string rather than null. --- .../struts2/action/CspReportAction.java | 50 +++++- .../action/CspReportActionReportSizeTest.java | 145 ++++++++++++++++++ 2 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 core/src/test/java/org/apache/struts2/action/CspReportActionReportSizeTest.java diff --git a/core/src/main/java/org/apache/struts2/action/CspReportAction.java b/core/src/main/java/org/apache/struts2/action/CspReportAction.java index c8b6b7bc51..158cfd3bcc 100644 --- a/core/src/main/java/org/apache/struts2/action/CspReportAction.java +++ b/core/src/main/java/org/apache/struts2/action/CspReportAction.java @@ -18,12 +18,14 @@ */ package org.apache.struts2.action; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.struts2.ActionSupport; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import java.io.BufferedReader; import java.io.IOException; +import java.io.Reader; import static org.apache.struts2.interceptor.csp.CspSettings.CSP_REPORT_TYPE; @@ -51,7 +53,27 @@ * @see DefaultCspReportAction */ public abstract class CspReportAction extends ActionSupport implements ServletRequestAware, ServletResponseAware { + + private static final Logger LOG = LogManager.getLogger(CspReportAction.class); + + /** + * Default upper bound, in characters, on the report body accepted by {@link #withServletRequest}. + * CSP violation reports are small JSON documents; anything larger is not treated as a report. + */ + public static final int DEFAULT_MAX_REPORT_SIZE = 8192; + private HttpServletRequest request; + private int maxReportSize = DEFAULT_MAX_REPORT_SIZE; + + /** + * Sets the upper bound, in characters, on an accepted report body. A body exceeding this size is + * discarded and not passed to {@link #processReport(String)}. + * + * @param maxReportSize maximum accepted report size in characters + */ + public void setMaxReportSize(int maxReportSize) { + this.maxReportSize = maxReportSize; + } @Override public void withServletRequest(HttpServletRequest request) { @@ -60,13 +82,35 @@ public void withServletRequest(HttpServletRequest request) { } try { - BufferedReader reader = request.getReader(); - String cspReport = reader.readLine(); + String cspReport = readReport(request.getReader()); + if (cspReport == null) { + LOG.warn("Discarding CSP report larger than the configured limit of {} characters", maxReportSize); + return; + } processReport(cspReport); } catch (IOException ignored) { } } + /** + * Reads at most {@link #maxReportSize} characters from the report body. + * + * @param reader reader over the report body + * @return the report body, or {@code null} if it exceeds the configured limit + */ + private String readReport(Reader reader) throws IOException { + char[] buffer = new char[maxReportSize]; + int total = 0; + int read; + while (total < buffer.length && (read = reader.read(buffer, total, buffer.length - total)) != -1) { + total += read; + } + if (total == buffer.length && reader.read() != -1) { + return null; + } + return new String(buffer, 0, total); + } + private boolean isCspReportRequest(HttpServletRequest request) { if (!"POST".equals(request.getMethod()) || request.getContentLength() <= 0){ return false; diff --git a/core/src/test/java/org/apache/struts2/action/CspReportActionReportSizeTest.java b/core/src/test/java/org/apache/struts2/action/CspReportActionReportSizeTest.java new file mode 100644 index 0000000000..46f54aeaee --- /dev/null +++ b/core/src/test/java/org/apache/struts2/action/CspReportActionReportSizeTest.java @@ -0,0 +1,145 @@ +/* + * 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.action; + +import org.apache.struts2.XWorkTestCase; +import org.apache.struts2.interceptor.csp.CspSettings; +import org.springframework.mock.web.MockHttpServletRequest; + +import java.io.BufferedReader; +import java.io.Reader; +import java.util.concurrent.atomic.AtomicLong; + +/** + * Verifies that {@link CspReportAction} applies an upper bound to the report body it accepts, and + * that the bound is configurable. + */ +public class CspReportActionReportSizeTest extends XWorkTestCase { + + /** + * The reader supplied by the container buffers ahead, so consumption is bounded by the limit + * plus one buffer rather than by the limit exactly. That overshoot is fixed, not proportional + * to the size of the body. + */ + private static final long READ_AHEAD_ALLOWANCE = 8192L; + + /** + * Produces {@code total} characters without buffering them, and records how many the caller + * actually consumed. + */ + private static final class CountingReader extends Reader { + private final long total; + private final AtomicLong consumed; + private long produced = 0; + + CountingReader(long total, AtomicLong consumed) { + this.total = total; + this.consumed = consumed; + } + + @Override + public int read(char[] cbuf, int off, int len) { + if (produced >= total) { + return -1; + } + int count = (int) Math.min(len, total - produced); + for (int i = 0; i < count; i++) { + cbuf[off + i] = 'a'; + } + produced += count; + consumed.addAndGet(count); + return count; + } + + @Override + public void close() { + // characters are generated on demand, so there is nothing to release + } + } + + private static final class CapturingCspReportAction extends CspReportAction { + String captured; + int reports; + + @Override + void processReport(String jsonCspReport) { + captured = jsonCspReport; + reports++; + } + } + + /** + * A request that both declares and delivers {@code size} characters, matching what a client can + * actually send: the declared length and the delivered body agree. + */ + private MockHttpServletRequest requestOfSize(final long size, final AtomicLong consumed) { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/csp-reports") { + @Override + public int getContentLength() { + return (int) Math.min(size, Integer.MAX_VALUE); + } + + @Override + public BufferedReader getReader() { + return new BufferedReader(new CountingReader(size, consumed)); + } + }; + request.setContentType(CspSettings.CSP_REPORT_TYPE); + return request; + } + + public void testReportAboveLimitIsNotProcessed() { + AtomicLong consumed = new AtomicLong(); + MockHttpServletRequest request = requestOfSize(64L * 1024 * 1024, consumed); + + CapturingCspReportAction action = new CapturingCspReportAction(); + action.withServletRequest(request); + + assertEquals("A report above the limit should not be processed", 0, action.reports); + assertTrue("Consumed " + consumed.get() + " characters for a limit of " + + CspReportAction.DEFAULT_MAX_REPORT_SIZE, + consumed.get() <= CspReportAction.DEFAULT_MAX_REPORT_SIZE + READ_AHEAD_ALLOWANCE); + } + + public void testReportWithinLimitIsProcessed() { + String sampleReport = "{\"csp-report\":{\"document-uri\":\"https://example.test/\"}}"; + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/csp-reports"); + request.setContent(sampleReport.getBytes()); + request.setContentType(CspSettings.CSP_REPORT_TYPE); + + CapturingCspReportAction action = new CapturingCspReportAction(); + action.withServletRequest(request); + + assertEquals("A report within the limit should be processed", 1, action.reports); + assertEquals("The report should be passed through unchanged", sampleReport, action.captured); + } + + public void testConfiguredLimitIsApplied() { + AtomicLong consumed = new AtomicLong(); + MockHttpServletRequest request = requestOfSize(4096, consumed); + + CapturingCspReportAction action = new CapturingCspReportAction(); + action.setMaxReportSize(1024); + action.withServletRequest(request); + + assertEquals("A report above the configured limit should not be processed", 0, action.reports); + assertTrue("Consumed " + consumed.get() + " characters for a configured limit of 1024", + consumed.get() <= 1024L + READ_AHEAD_ALLOWANCE); + } +}