WW-5666 Apply input length limits consistently when reading request bodies

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;

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);
+    }
+}

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<Pattern> 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.");

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"));
+    }
+}