WW-5723 Bound the request body read in the REST plugin - #1912
Merged
Conversation
…ptor 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
lukaszlenart
force-pushed
the
WW-5723-rest-body-limit
branch
from
September 11, 2026 12:11
6f40627 to
5236bd9
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes WW-5723
ContentTypeInterceptorhandedrequest.getInputStream()to the content-type handler with no length limit, while the JSON plugin bounds the same read withstruts.json.maxLengthandCspReportActionwithstruts.csp.report.maxSize. This applies the same limit to the REST plugin.What changes
struts.rest.content.maxLength, default2097152(matching the JSON plugin), declared in the plugin'sstruts-plugin.xmland injected intoContentTypeInterceptorvia@Inject(required = false). Blank, non-numeric or sub-1 values are ignored with a warning and the default kept, asCspReportActiondoes.BoundedReader— aFilterReaderthat counts characters and fails once the limit is passed. On overflow the interceptor throws the newRequestBodyTooLargeException(aStrutsException), and the action is never invoked.applyRequestBodyand the two authorization paths take aReaderinstead of anInputStreamReader;ContentTypeHandler.toObjectalready declaredReader.getContentLength() > 0gate is unchanged.Design notes
Bound the read, not the header. The limit is enforced on characters actually consumed, so it holds regardless of the declared
Content-Length.Lazy rather than buffered. An earlier shape read the body into a buffer before calling the handler. That drained the stream even for handlers that never read it (
HtmlHandler,FormUrlEncodedHandler,MultipartFormDataHandler), which would have broken an action reading the raw body itself behind one of them. Wrapping the reader instead means those handlers leave the body untouched exactly as before, and Jackson keeps streaming rather than parsing from a buffer.One exception type regardless of handler. Handlers wrap the reader's
IOExceptionin their own types — Jackson passes it through, XStream wraps inStreamException, Juneau inParseException. Rather than depend on what propagates,intercept()consults the reader's flag after the call and throwsRequestBodyTooLargeExceptioneither way. A handler that swallows the failure still fails closed: the flag is checked on the normal return path too. The dedicated type lets an application map this to 413 via<exception-mapping>without catching everyStrutsException.Framework constant, not an action property. Matches both siblings and behaves identically on 6.x, where interceptor ordering differs.
No upper cap on the configured value.
CspReportActioncaps because it pre-allocates a buffer of that size; nothing is pre-allocated here, so a large value costs nothing until a body that size arrives.Authorization context. The read now happens inside
applyWithAuthorizationContext's bind/unbind window.ParameterAuthorizationContext.unbind()removes all three thread-locals unconditionally infinally, and the Jackson handlers clear their dynamic-key scope in their ownfinally, so an abort mid-parse leaves nothing on the thread. Properties bound before the limit trips have each passed authorization individually — the outcome is the same as a malformed body truncated at that offset, andinvoke()does not run.Tests
Six new tests in
ContentTypeInterceptorTest: over-limit body rejected before the action runs, body exactly at the limit passed in full, over-limit body not read to the end, non-numeric and sub-1 configuration keep the default, and a handler that ignores the reader leaves the body unread.Two existing tests asserted the handler received an
InputStreamReaderand read its encoding from it, i.e. an implementation type. They now assert the decoded content, and the ASCII case becomes ISO-8859-1 so the assertion discriminates between honouring the request charset and ignoring it — an ASCII body decodes the same under any charset.A second commit adds four more for the branches the first set left unreached — blank configuration, a handler that swallows the reader's failure, a handler failure under the limit, and
skip().REST plugin suite: 159 tests, 0 failures.
Follow-ups
struts.rest.content.maxLengthon the REST plugin page in struts-site.🤖 Generated with Claude Code