From 5eca3d9b761883f3d2da2067af03aa940303bf93 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 11 Sep 2026 12:56:13 +0200 Subject: [PATCH 1/2] WW-5723 feat(rest): bound the request body read in ContentTypeInterceptor The REST plugin handed request.getInputStream() to the content-type handler with no length limit, while the JSON plugin bounds the same read with struts.json.maxLength and CspReportAction with struts.csp.report.maxSize. Apply the same limit here. Add struts.rest.content.maxLength (default 2097152, matching the JSON plugin) as a framework constant injected into the interceptor, so it is set before the interceptor stack runs and behaves identically on both maintenance lines. Blank, non-numeric or sub-1 values are ignored with a warning and the default kept, as CspReportAction does. No upper cap: unlike CspReportAction nothing is pre-allocated, so a large value costs nothing until a body that size arrives. The bound is enforced on the read itself, not on Content-Length: the handler receives a FilterReader that counts characters and fails once the limit is passed. Reading lazily means handlers that never touch the reader (HTML, form-urlencoded, multipart) leave the body untouched for the action, exactly as before, and Jackson keeps streaming rather than parsing from a buffer. Handlers wrap the reader's failure in their own types (Jackson passes IOException through, XStream wraps in StreamException, Juneau in ParseException), so intercept() consults the reader's flag after the call and throws RequestBodyTooLargeException regardless of what propagated. A handler that swallows the failure still fails closed: the flag is checked on the normal return path too, and the action is never invoked. The dedicated exception type lets an application map it to a 413 via exception-mapping without catching every StrutsException. The getContentLength() > 0 gate is unchanged. Two existing tests asserted the handler received an InputStreamReader and read its encoding from it. They now assert the decoded content instead; the ASCII case becomes ISO-8859-1 so the assertion actually discriminates between honouring the request charset and ignoring it. Co-Authored-By: Claude Opus 5 (1M context) --- .../struts2/rest/ContentTypeInterceptor.java | 110 +++++++- .../rest/RequestBodyTooLargeException.java | 32 +++ .../apache/struts2/rest/RestConstants.java | 1 + .../rest/src/main/resources/struts-plugin.xml | 1 + .../rest/ContentTypeInterceptorTest.java | 253 +++++++++++++++++- 5 files changed, 382 insertions(+), 15 deletions(-) create mode 100644 plugins/rest/src/main/java/org/apache/struts2/rest/RequestBodyTooLargeException.java diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java b/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java index 17ccdf2de2..e814e5dfc0 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java @@ -27,6 +27,7 @@ import org.apache.struts2.ServletActionContext; import org.apache.struts2.rest.handler.ContentTypeHandler; import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -34,8 +35,11 @@ import java.beans.BeanInfo; import java.beans.Introspector; import java.beans.PropertyDescriptor; +import java.io.FilterReader; +import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.io.Reader; import java.lang.reflect.Array; import java.lang.reflect.Method; import java.util.ArrayList; @@ -62,9 +66,12 @@ public class ContentTypeInterceptor extends AbstractInterceptor { private static final Logger LOG = LogManager.getLogger(ContentTypeInterceptor.class); + public static final int DEFAULT_MAX_LENGTH = 2_097_152; + private ContentTypeHandlerManager selector; private ParameterAuthorizer parameterAuthorizer; private boolean requireAnnotations = false; + private int maxLength = DEFAULT_MAX_LENGTH; @Inject public void setContentTypeHandlerSelector(ContentTypeHandlerManager selector) { @@ -81,6 +88,27 @@ public void setRequireAnnotations(String requireAnnotations) { this.requireAnnotations = BooleanUtils.toBoolean(requireAnnotations); } + @Inject(value = RestConstants.REST_CONTENT_MAX_LENGTH, required = false) + public void setMaxLength(String maxLength) { + if (StringUtils.isBlank(maxLength)) { + return; + } + int length; + try { + length = Integer.parseInt(maxLength.trim()); + } catch (NumberFormatException e) { + LOG.warn("Ignoring non-numeric {} value: {}, keeping {}", + RestConstants.REST_CONTENT_MAX_LENGTH, maxLength, this.maxLength); + return; + } + if (length < 1) { + LOG.warn("Ignoring out-of-range {} value: {}, expected 1 or more, keeping {}", + RestConstants.REST_CONTENT_MAX_LENGTH, length, this.maxLength); + return; + } + this.maxLength = length; + } + public String intercept(ActionInvocation invocation) throws Exception { HttpServletRequest request = ServletActionContext.getRequest(); ContentTypeHandler handler = selector.getHandlerForRequest(request); @@ -91,19 +119,35 @@ public String intercept(ActionInvocation invocation) throws Exception { } if (request.getContentLength() > 0) { - applyRequestBody(invocation, handler, target, openBodyReader(request)); + BoundedReader reader = new BoundedReader(openBodyReader(request), maxLength); + try { + applyRequestBody(invocation, handler, target, reader); + } catch (Exception e) { + if (reader.limitExceeded()) { + throw requestBodyTooLarge(); + } + throw e; + } + if (reader.limitExceeded()) { + throw requestBodyTooLarge(); + } } return invocation.invoke(); } - private static InputStreamReader openBodyReader(HttpServletRequest request) throws java.io.IOException { + private RequestBodyTooLargeException requestBodyTooLarge() { + return new RequestBodyTooLargeException("Request body exceeds maximum allowed length (" + + maxLength + "). Use " + RestConstants.REST_CONTENT_MAX_LENGTH + " to increase the limit."); + } + + private static InputStreamReader openBodyReader(HttpServletRequest request) throws IOException { String encoding = request.getCharacterEncoding(); InputStream is = request.getInputStream(); return encoding == null ? new InputStreamReader(is) : new InputStreamReader(is, encoding); } private void applyRequestBody(ActionInvocation invocation, ContentTypeHandler handler, Object target, - InputStreamReader reader) throws Exception { + Reader reader) throws Exception { if (!requireAnnotations) { // Direct deserialization (backward compat when requireAnnotations is not enabled). handler.toObject(invocation, reader, target); @@ -122,7 +166,7 @@ private void applyRequestBody(ActionInvocation invocation, ContentTypeHandler ha * for the call duration. */ private void applyWithAuthorizationContext(ActionInvocation invocation, ContentTypeHandler handler, Object target, - InputStreamReader reader) throws java.io.IOException { + Reader reader) throws IOException { Object action = invocation.getAction(); Object resolvedTarget = parameterAuthorizer.resolveTarget(action); org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext.bind( @@ -141,7 +185,7 @@ private void applyWithAuthorizationContext(ActionInvocation invocation, ContentT * unauthorized property is nulled out, so skipping is the safer choice). */ private void applyTwoPhaseDeserialize(ActionInvocation invocation, ContentTypeHandler handler, Object target, - InputStreamReader reader) throws Exception { + Reader reader) throws Exception { Object freshInstance = createFreshInstance(target.getClass()); if (freshInstance == null) { LOG.warn("REST body rejected: requireAnnotations=true but [{}] has no no-arg constructor; " @@ -378,4 +422,60 @@ private boolean isNestedBeanType(Class clazz) { return true; } + /** + * Stops the handler at {@code struts.rest.content.maxLength} characters. The handler may wrap the + * {@link IOException} thrown here in its own type, so {@link #intercept} consults + * {@link #limitExceeded()} afterwards rather than relying on what propagates. + */ + private static final class BoundedReader extends FilterReader { + + private final int limit; + private long consumed; + private boolean limitExceeded; + + BoundedReader(Reader in, int limit) { + super(in); + this.limit = limit; + } + + @Override + public int read() throws IOException { + int c = super.read(); + if (c != -1) { + consumed(1); + } + return c; + } + + @Override + public int read(char[] buf, int off, int len) throws IOException { + int n = super.read(buf, off, len); + if (n > 0) { + consumed(n); + } + return n; + } + + @Override + public long skip(long n) throws IOException { + long skipped = super.skip(n); + if (skipped > 0) { + consumed(skipped); + } + return skipped; + } + + private void consumed(long n) throws IOException { + consumed += n; + if (consumed > limit) { + limitExceeded = true; + throw new IOException("Request body exceeds " + limit + " characters"); + } + } + + boolean limitExceeded() { + return limitExceeded; + } + } + } diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/RequestBodyTooLargeException.java b/plugins/rest/src/main/java/org/apache/struts2/rest/RequestBodyTooLargeException.java new file mode 100644 index 0000000000..5f1e8ef5c2 --- /dev/null +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/RequestBodyTooLargeException.java @@ -0,0 +1,32 @@ +/* + * 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.rest; + +import org.apache.struts2.StrutsException; + +/** + * Thrown by {@link ContentTypeInterceptor} when a request body exceeds + * {@code struts.rest.content.maxLength}. + */ +public class RequestBodyTooLargeException extends StrutsException { + + public RequestBodyTooLargeException(String message) { + super(message); + } +} diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/RestConstants.java b/plugins/rest/src/main/java/org/apache/struts2/rest/RestConstants.java index d2675ecee0..2fd7721209 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/RestConstants.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/RestConstants.java @@ -36,4 +36,5 @@ public class RestConstants { public static final String REST_MAPPER_PUT_CONTINUE_METHOD_NAME = "struts.mapper.putContinueMethodName"; public static final String STRUTS_REST_NAMESPACE = "struts.rest.namespace"; public static final String REST_VALIDATION_FAILURE_STATUS_CODE = "struts.rest.validationFailureStatusCode"; + public static final String REST_CONTENT_MAX_LENGTH = "struts.rest.content.maxLength"; } diff --git a/plugins/rest/src/main/resources/struts-plugin.xml b/plugins/rest/src/main/resources/struts-plugin.xml index 73ad21146e..c6bb08e0eb 100644 --- a/plugins/rest/src/main/resources/struts-plugin.xml +++ b/plugins/rest/src/main/resources/struts-plugin.xml @@ -41,6 +41,7 @@ + diff --git a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java index a14d407eef..3983ded65b 100644 --- a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java +++ b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java @@ -26,7 +26,11 @@ import org.apache.struts2.ActionSupport; import junit.framework.TestCase; -import java.io.InputStreamReader; +import jakarta.servlet.ReadListener; +import jakarta.servlet.ServletInputStream; +import java.io.IOException; +import java.io.Reader; +import java.util.Arrays; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; @@ -74,8 +78,8 @@ public boolean matches(Object[] args) { mockContentTypeHandler.verify(); } - public void testRequestWithEncodingAscii() throws Exception { - final Charset charset = StandardCharsets.US_ASCII; + public void testRequestWithEncodingLatin1() throws Exception { + final Charset charset = StandardCharsets.ISO_8859_1; ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); interceptor.setParameterAuthorizer((parameterName, target, action) -> true); @@ -86,8 +90,7 @@ public void testRequestWithEncodingAscii() throws Exception { Mock mockContentTypeHandler = new Mock(ContentTypeHandler.class); mockContentTypeHandler.expect("toObject", new AnyConstraintMatcher() { public boolean matches(Object[] args) { - InputStreamReader in = (InputStreamReader) args[1]; - return charset.equals(Charset.forName(in.getEncoding())); + return "caf\u00e9".equals(readFully((Reader) args[1])); } }); mockActionInvocation.expectAndReturn("invoke", Action.SUCCESS); @@ -101,7 +104,7 @@ public boolean matches(Object[] args) { interceptor.setContentTypeHandlerSelector((ContentTypeHandlerManager) mockContentTypeHandlerManager.proxy()); MockHttpServletRequest request = new MockHttpServletRequest(); - request.setContent(new byte[] {1}); + request.setContent("caf\u00e9".getBytes(charset)); request.setCharacterEncoding(charset.name()); ActionContext.of() @@ -115,7 +118,7 @@ public boolean matches(Object[] args) { mockContentTypeHandler.verify(); } - public void testRequestWithEncodingUtf() throws Exception { + public void testRequestWithEncodingUtf8() throws Exception { final Charset charset = StandardCharsets.UTF_8; ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); @@ -127,8 +130,7 @@ public void testRequestWithEncodingUtf() throws Exception { Mock mockContentTypeHandler = new Mock(ContentTypeHandler.class); mockContentTypeHandler.expect("toObject", new AnyConstraintMatcher() { public boolean matches(Object[] args) { - InputStreamReader in = (InputStreamReader) args[1]; - return charset.equals(Charset.forName(in.getEncoding())); + return "caf\u00e9".equals(readFully((Reader) args[1])); } }); mockActionInvocation.expectAndReturn("invoke", Action.SUCCESS); @@ -142,7 +144,7 @@ public boolean matches(Object[] args) { interceptor.setContentTypeHandlerSelector((ContentTypeHandlerManager) mockContentTypeHandlerManager.proxy()); MockHttpServletRequest request = new MockHttpServletRequest(); - request.setContent(new byte[] {1}); + request.setContent("caf\u00e9".getBytes(charset)); request.setCharacterEncoding(charset.name()); ActionContext.of() @@ -233,4 +235,235 @@ public boolean matches(Object[] args) { mockActionInvocation.verify(); mockContentTypeHandler.verify(); } + + public void testBodyOverLimitIsRejectedBeforeActionRuns() throws Exception { + ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); + interceptor.setParameterAuthorizer((parameterName, target, action) -> true); + interceptor.setMaxLength("8"); + + Mock mockActionInvocation = new Mock(ActionInvocation.class); + mockActionInvocation.expectAndReturn("getAction", new ActionSupport()); + interceptor.setContentTypeHandlerSelector(selectorReturning(readingHandler())); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setContent("123456789".getBytes(StandardCharsets.US_ASCII)); + + ActionContext.of() + .withActionMapping(new ActionMapping()) + .withServletRequest(request) + .bind(); + + try { + interceptor.intercept((ActionInvocation) mockActionInvocation.proxy()); + fail("expected " + RequestBodyTooLargeException.class.getSimpleName()); + } catch (RequestBodyTooLargeException expected) { + assertTrue(expected.getMessage().contains(RestConstants.REST_CONTENT_MAX_LENGTH)); + } + mockActionInvocation.verify(); + } + + public void testBodyAtLimitIsPassedToHandlerInFull() throws Exception { + ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); + interceptor.setParameterAuthorizer((parameterName, target, action) -> true); + interceptor.setMaxLength("8"); + + assertEquals("12345678", interceptAndCaptureBody(interceptor, new MockHttpServletRequest(), + "12345678".getBytes(StandardCharsets.US_ASCII))); + } + + public void testBodyOverLimitIsNotReadToTheEnd() throws Exception { + ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); + interceptor.setParameterAuthorizer((parameterName, target, action) -> true); + interceptor.setMaxLength("8"); + + Mock mockActionInvocation = new Mock(ActionInvocation.class); + mockActionInvocation.expectAndReturn("getAction", new ActionSupport()); + interceptor.setContentTypeHandlerSelector(selectorReturning(readingHandler())); + + byte[] body = new byte[1024 * 1024]; + Arrays.fill(body, (byte) 'x'); + CountingRequest request = new CountingRequest(); + request.setContent(body); + + ActionContext.of() + .withActionMapping(new ActionMapping()) + .withServletRequest(request) + .bind(); + + try { + interceptor.intercept((ActionInvocation) mockActionInvocation.proxy()); + fail("expected " + RequestBodyTooLargeException.class.getSimpleName()); + } catch (RequestBodyTooLargeException expected) { + assertTrue("read " + request.bytesRead + " of " + body.length + " bytes", + request.bytesRead < body.length); + } + } + + public void testNonNumericMaxLengthKeepsDefault() throws Exception { + ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); + interceptor.setParameterAuthorizer((parameterName, target, action) -> true); + interceptor.setMaxLength("lots"); + + byte[] body = new byte[64 * 1024]; + Arrays.fill(body, (byte) 'x'); + assertEquals(body.length, interceptAndCaptureBody(interceptor, new MockHttpServletRequest(), body).length()); + } + + public void testMaxLengthBelowOneKeepsDefault() throws Exception { + ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); + interceptor.setParameterAuthorizer((parameterName, target, action) -> true); + interceptor.setMaxLength("0"); + + assertEquals("abc", interceptAndCaptureBody(interceptor, new MockHttpServletRequest(), + "abc".getBytes(StandardCharsets.US_ASCII))); + } + + public void testHandlerThatIgnoresTheReaderLeavesBodyUnread() throws Exception { + ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); + interceptor.setParameterAuthorizer((parameterName, target, action) -> true); + + Mock mockActionInvocation = new Mock(ActionInvocation.class); + mockActionInvocation.expectAndReturn("invoke", Action.SUCCESS); + mockActionInvocation.expectAndReturn("getAction", new ActionSupport()); + Mock mockContentTypeHandler = new Mock(ContentTypeHandler.class); + mockContentTypeHandler.expect("toObject", new AnyConstraintMatcher() { + public boolean matches(Object[] args) { + return true; + } + }); + Mock mockContentTypeHandlerManager = new Mock(ContentTypeHandlerManager.class); + mockContentTypeHandlerManager.expectAndReturn("getHandlerForRequest", new AnyConstraintMatcher() { + public boolean matches(Object[] args) { + return true; + } + }, mockContentTypeHandler.proxy()); + interceptor.setContentTypeHandlerSelector((ContentTypeHandlerManager) mockContentTypeHandlerManager.proxy()); + + CountingRequest request = new CountingRequest(); + request.setContent("raw body the action may want to read itself".getBytes(StandardCharsets.US_ASCII)); + + ActionContext.of() + .withActionMapping(new ActionMapping()) + .withServletRequest(request) + .bind(); + + interceptor.intercept((ActionInvocation) mockActionInvocation.proxy()); + assertEquals(0, request.bytesRead); + mockActionInvocation.verify(); + } + + /** + * A handler that reads the body the way the real ones do, and surfaces the reader's failure in its + * own exception type as Jackson, XStream and Juneau each do. + */ + private static ContentTypeHandler readingHandler() { + Mock handler = new Mock(ContentTypeHandler.class); + handler.expect("toObject", new AnyConstraintMatcher() { + public boolean matches(Object[] args) { + readFully((Reader) args[1]); + return true; + } + }); + return (ContentTypeHandler) handler.proxy(); + } + + private static ContentTypeHandlerManager selectorReturning(ContentTypeHandler handler) { + Mock selector = new Mock(ContentTypeHandlerManager.class); + selector.expectAndReturn("getHandlerForRequest", new AnyConstraintMatcher() { + public boolean matches(Object[] args) { + return true; + } + }, handler); + return (ContentTypeHandlerManager) selector.proxy(); + } + + private static String interceptAndCaptureBody(ContentTypeInterceptor interceptor, MockHttpServletRequest request, + byte[] body) throws Exception { + String[] captured = new String[1]; + Mock mockActionInvocation = new Mock(ActionInvocation.class); + mockActionInvocation.expectAndReturn("invoke", Action.SUCCESS); + mockActionInvocation.expectAndReturn("getAction", new ActionSupport()); + Mock mockContentTypeHandler = new Mock(ContentTypeHandler.class); + mockContentTypeHandler.expect("toObject", new AnyConstraintMatcher() { + public boolean matches(Object[] args) { + captured[0] = readFully((Reader) args[1]); + return true; + } + }); + Mock mockContentTypeHandlerManager = new Mock(ContentTypeHandlerManager.class); + mockContentTypeHandlerManager.expectAndReturn("getHandlerForRequest", new AnyConstraintMatcher() { + public boolean matches(Object[] args) { + return true; + } + }, mockContentTypeHandler.proxy()); + interceptor.setContentTypeHandlerSelector((ContentTypeHandlerManager) mockContentTypeHandlerManager.proxy()); + + request.setContent(body); + ActionContext.of() + .withActionMapping(new ActionMapping()) + .withServletRequest(request) + .bind(); + + interceptor.intercept((ActionInvocation) mockActionInvocation.proxy()); + mockContentTypeHandler.verify(); + mockActionInvocation.verify(); + return captured[0]; + } + + /** Counts the bytes the interceptor actually pulls from the request stream. */ + private static final class CountingRequest extends MockHttpServletRequest { + long bytesRead; + + @Override + public ServletInputStream getInputStream() { + ServletInputStream delegate = super.getInputStream(); + return new ServletInputStream() { + @Override + public int read() throws IOException { + int b = delegate.read(); + if (b != -1) { + bytesRead++; + } + return b; + } + + @Override + public int read(byte[] buf, int off, int len) throws IOException { + int n = delegate.read(buf, off, len); + if (n > 0) { + bytesRead += n; + } + return n; + } + + @Override + public boolean isFinished() { + return delegate.isFinished(); + } + + @Override + public boolean isReady() { + return delegate.isReady(); + } + + @Override + public void setReadListener(ReadListener readListener) { + delegate.setReadListener(readListener); + } + }; + } + } + + private static String readFully(Reader reader) { + try { + StringBuilder out = new StringBuilder(); + int c; + while ((c = reader.read()) != -1) { + out.append((char) c); + } + return out.toString(); + } catch (IOException e) { + throw new IllegalStateException(e); + } + } } From 5236bd94023e78f8972d1839328ec0d1713cc7b4 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 11 Sep 2026 14:11:08 +0200 Subject: [PATCH 2/2] WW-5723 test(rest): cover the remaining ContentTypeInterceptor branches Sonar reported 79.4% coverage on new code against an 80% gate. The unit tests reached the limit through single-character reads only, so four branches were untested: a blank configured value keeping the default, a handler that swallows the reader's failure still being rejected on the normal-return check, a handler failure under the limit propagating as the same object, and skipped input counting against the limit. Co-Authored-By: Claude Opus 5 (1M context) --- .../rest/ContentTypeInterceptorTest.java | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java index 3983ded65b..8ed42d4c19 100644 --- a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java +++ b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java @@ -352,6 +352,119 @@ public boolean matches(Object[] args) { mockActionInvocation.verify(); } + public void testBlankMaxLengthKeepsDefault() throws Exception { + ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); + interceptor.setParameterAuthorizer((parameterName, target, action) -> true); + interceptor.setMaxLength(" "); + + byte[] body = new byte[64 * 1024]; + Arrays.fill(body, (byte) 'x'); + assertEquals(body.length, interceptAndCaptureBody(interceptor, new MockHttpServletRequest(), body).length()); + } + + public void testHandlerThatSwallowsTheLimitIsStillRejected() throws Exception { + ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); + interceptor.setParameterAuthorizer((parameterName, target, action) -> true); + interceptor.setMaxLength("8"); + + Mock mockActionInvocation = new Mock(ActionInvocation.class); + mockActionInvocation.expectAndReturn("getAction", new ActionSupport()); + Mock swallowingHandler = new Mock(ContentTypeHandler.class); + swallowingHandler.expect("toObject", new AnyConstraintMatcher() { + public boolean matches(Object[] args) { + try { + readFully((Reader) args[1]); + } catch (RuntimeException swallowed) { + // a handler that hides the reader's failure must not let the action run + } + return true; + } + }); + interceptor.setContentTypeHandlerSelector(selectorReturning((ContentTypeHandler) swallowingHandler.proxy())); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setContent("123456789".getBytes(StandardCharsets.US_ASCII)); + ActionContext.of() + .withActionMapping(new ActionMapping()) + .withServletRequest(request) + .bind(); + + try { + interceptor.intercept((ActionInvocation) mockActionInvocation.proxy()); + fail("expected " + RequestBodyTooLargeException.class.getSimpleName()); + } catch (RequestBodyTooLargeException expected) { + // action never invoked: no "invoke" expectation was set + } + mockActionInvocation.verify(); + } + + public void testHandlerFailureUnderTheLimitPropagatesUnchanged() throws Exception { + ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); + interceptor.setParameterAuthorizer((parameterName, target, action) -> true); + interceptor.setMaxLength("8"); + + Mock mockActionInvocation = new Mock(ActionInvocation.class); + mockActionInvocation.expectAndReturn("getAction", new ActionSupport()); + IllegalStateException handlerFailure = new IllegalStateException("malformed"); + Mock failingHandler = new Mock(ContentTypeHandler.class); + failingHandler.expect("toObject", new AnyConstraintMatcher() { + public boolean matches(Object[] args) { + throw handlerFailure; + } + }); + interceptor.setContentTypeHandlerSelector(selectorReturning((ContentTypeHandler) failingHandler.proxy())); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setContent("abc".getBytes(StandardCharsets.US_ASCII)); + ActionContext.of() + .withActionMapping(new ActionMapping()) + .withServletRequest(request) + .bind(); + + try { + interceptor.intercept((ActionInvocation) mockActionInvocation.proxy()); + fail("expected the handler's own exception"); + } catch (IllegalStateException e) { + assertSame(handlerFailure, e); + } + } + + public void testSkippingPastTheLimitIsRejected() throws Exception { + ContentTypeInterceptor interceptor = new ContentTypeInterceptor(); + interceptor.setParameterAuthorizer((parameterName, target, action) -> true); + interceptor.setMaxLength("8"); + + Mock mockActionInvocation = new Mock(ActionInvocation.class); + mockActionInvocation.expectAndReturn("getAction", new ActionSupport()); + Mock skippingHandler = new Mock(ContentTypeHandler.class); + skippingHandler.expect("toObject", new AnyConstraintMatcher() { + public boolean matches(Object[] args) { + try { + ((Reader) args[1]).skip(Long.MAX_VALUE); + } catch (IOException e) { + throw new IllegalStateException(e); + } + return true; + } + }); + interceptor.setContentTypeHandlerSelector(selectorReturning((ContentTypeHandler) skippingHandler.proxy())); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setContent("123456789".getBytes(StandardCharsets.US_ASCII)); + ActionContext.of() + .withActionMapping(new ActionMapping()) + .withServletRequest(request) + .bind(); + + try { + interceptor.intercept((ActionInvocation) mockActionInvocation.proxy()); + fail("expected " + RequestBodyTooLargeException.class.getSimpleName()); + } catch (RequestBodyTooLargeException expected) { + // skipped input counts against the limit like read input + } + mockActionInvocation.verify(); + } + /** * A handler that reads the body the way the real ones do, and surfaces the reader's failure in its * own exception type as Jackson, XStream and Juneau each do.