This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new 1218c4922 WW-5666 Apply input length limits consistently when reading
request bodies (#1819)
1218c4922 is described below
commit 1218c492244ea872d38aa50b6d4fcf2b81964325
Author: Lukasz Lenart <[email protected]>
AuthorDate: Fri Jul 31 11:05:33 2026 +0200
WW-5666 Apply input length limits consistently when reading request bodies
(#1819)
* 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.
* WW-5666 fix(core): bound the CSP report body read and make the limit
configurable
CspReportAction read the submitted report body with a single readLine() and
had
no limit of its own. Read it up to a limit instead, defaulting to 8192
characters and configurable through struts.csp.report.maxSize. A body above
the
limit is discarded with a warning rather than processed.
The limit is injected when the action is built, before the interceptor stack
runs, because withServletRequest is invoked by the servletConfig interceptor
ahead of staticParams and params. Values that are not usable as a buffer
size
are ignored with a warning.
---
.../java/org/apache/struts2/StrutsConstants.java | 7 +
.../org/apache/struts2/action/CspReportAction.java | 84 +++++++++-
.../org/apache/struts2/default.properties | 4 +
.../action/CspReportActionReportSizeTest.java | 185 +++++++++++++++++++++
.../java/org/apache/struts2/json/JSONUtil.java | 13 +-
.../struts2/json/JSONUtilInputLimitTest.java | 114 +++++++++++++
6 files changed, 400 insertions(+), 7 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java
b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index 4b243bcf4..5a09cfa78 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -795,4 +795,11 @@ public final class StrutsConstants {
*/
public static final String STRUTS_CSP_NONCE_READER =
"struts.csp.nonce.reader";
public static final String STRUTS_CSP_NONCE_SOURCE =
"struts.csp.nonce.source";
+
+ /**
+ * See {@link org.apache.struts2.action.CspReportAction}
+ *
+ * @since 7.3.0
+ */
+ public static final String STRUTS_CSP_REPORT_MAX_SIZE =
"struts.csp.report.maxSize";
}
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 c8b6b7bc5..100c96af3 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,17 @@
*/
package org.apache.struts2.action;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
import org.apache.struts2.ActionSupport;
+import org.apache.struts2.StrutsConstants;
+import org.apache.struts2.inject.Inject;
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 +56,58 @@ import static
org.apache.struts2.interceptor.csp.CspSettings.CSP_REPORT_TYPE;
* @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;
+
+ /**
+ * Largest value accepted for {@code struts.csp.report.maxSize}. A
configured value above this is
+ * ignored, so that a mistyped setting cannot size a per-request buffer
large enough to exhaust
+ * memory.
+ */
+ private static final int MAX_REPORT_SIZE_LIMIT = 1024 * 1024;
+
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)}.
+ * <p>
+ * The value is injected from {@code struts.csp.report.maxSize} when the
action is built, which is
+ * before the interceptor stack runs. It is deliberately not an action
property: the report body is
+ * read by {@link #withServletRequest(HttpServletRequest)}, which the
{@code servletConfig}
+ * interceptor invokes ahead of {@code staticParams} and {@code params},
so a value applied by
+ * either of those would arrive too late to have any effect.
+ *
+ * @param maxReportSize maximum accepted report size in characters
+ * @since 7.3.0
+ */
+ @Inject(value = StrutsConstants.STRUTS_CSP_REPORT_MAX_SIZE, required =
false)
+ public void setMaxReportSize(String maxReportSize) {
+ if (StringUtils.isBlank(maxReportSize)) {
+ return;
+ }
+ int size;
+ try {
+ size = Integer.parseInt(maxReportSize.trim());
+ } catch (NumberFormatException e) {
+ LOG.warn("Ignoring non-numeric {} value: {}, keeping {}",
+ StrutsConstants.STRUTS_CSP_REPORT_MAX_SIZE, maxReportSize,
this.maxReportSize);
+ return;
+ }
+ if (size < 1 || size > MAX_REPORT_SIZE_LIMIT) {
+ LOG.warn("Ignoring out-of-range {} value: {}, expected 1..{},
keeping {}",
+ StrutsConstants.STRUTS_CSP_REPORT_MAX_SIZE, size,
MAX_REPORT_SIZE_LIMIT, this.maxReportSize);
+ return;
+ }
+ this.maxReportSize = size;
+ }
@Override
public void withServletRequest(HttpServletRequest request) {
@@ -60,13 +116,35 @@ public abstract class CspReportAction extends
ActionSupport implements ServletRe
}
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/main/resources/org/apache/struts2/default.properties
b/core/src/main/resources/org/apache/struts2/default.properties
index 9206817fa..51c613a95 100644
--- a/core/src/main/resources/org/apache/struts2/default.properties
+++ b/core/src/main/resources/org/apache/struts2/default.properties
@@ -351,6 +351,10 @@ struts.url.decoder=strutsUrlDecoder
### Defines source to read nonce value from, possible values are: request,
session
struts.csp.nonceSource=session
+### Maximum size, in characters, of a CSP violation report accepted by
CspReportAction
+### Reports larger than this are discarded. Values outside 1..1048576 are
ignored.
+struts.csp.report.maxSize=8192
+
### Checkbox hidden field prefix
### Default prefix for backward compatibility. Change to "struts_checkbox_"
for HTML5 validation.
struts.ui.checkbox.hiddenPrefix=__checkbox_
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 000000000..12a4e9476
--- /dev/null
+++
b/core/src/test/java/org/apache/struts2/action/CspReportActionReportSizeTest.java
@@ -0,0 +1,185 @@
+/*
+ * 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.StrutsConstants;
+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.IOException;
+import java.io.InputStream;
+import java.io.Reader;
+import java.util.Properties;
+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);
+ }
+
+ /**
+ * The key named by {@link StrutsConstants#STRUTS_CSP_REPORT_MAX_SIZE}
must exist in
+ * default.properties under exactly that name. If the two drift apart the
value is silently never
+ * injected, leaving the limit hard-coded and the documented setting inert.
+ */
+ public void testLimitKeyIsDefinedInDefaultProperties() throws IOException {
+ Properties defaults = new Properties();
+ try (InputStream in = getClass().getClassLoader()
+ .getResourceAsStream("org/apache/struts2/default.properties"))
{
+ assertNotNull("default.properties should be on the classpath", in);
+ defaults.load(in);
+ }
+
+ assertEquals(StrutsConstants.STRUTS_CSP_REPORT_MAX_SIZE + " should be
defined in default.properties",
+ String.valueOf(CspReportAction.DEFAULT_MAX_REPORT_SIZE),
+
defaults.getProperty(StrutsConstants.STRUTS_CSP_REPORT_MAX_SIZE));
+ }
+
+ public void testUnusableConfiguredValuesAreIgnored() {
+ String[] unusable = {"", " ", "not-a-number", "0", "-1",
"2147483647"};
+
+ for (String value : unusable) {
+ AtomicLong consumed = new AtomicLong();
+ MockHttpServletRequest request = requestOfSize(64L * 1024 * 1024,
consumed);
+
+ CapturingCspReportAction action = new CapturingCspReportAction();
+ action.setMaxReportSize(value);
+ action.withServletRequest(request);
+
+ assertEquals("A report above the default limit should not be
processed for value '"
+ + value + "'", 0, action.reports);
+ assertTrue("Consumed " + consumed.get() + " characters for value
'" + value + "'",
+ consumed.get() <= CspReportAction.DEFAULT_MAX_REPORT_SIZE
+ READ_AHEAD_ALLOWANCE);
+ }
+ }
+}
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 78bceaacf..e1c20f7b1 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 class JSONUtil {
* @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 000000000..4dbe70b25
--- /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"));
+ }
+}
