Feature/issue99 request body size limit filter#114
Open
MartinStenhagen wants to merge 11 commits intomainfrom
Open
Feature/issue99 request body size limit filter#114MartinStenhagen wants to merge 11 commits intomainfrom
MartinStenhagen wants to merge 11 commits intomainfrom
Conversation
…ry(SC_PAYLOAD_TOO_LARGE, "Payload Too Large") too HttpResponseBuilder
Removed unnecessary .toUpperCase in getHeaderAsLong
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a configurable maximum request body size feature: new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ConnectionHandler
participant MaxRequestBodySizeFilter
participant FilterChain
participant HttpResponseBuilder
Client->>ConnectionHandler: send HTTP request (headers + optional body)
ConnectionHandler->>MaxRequestBodySizeFilter: invoke doFilter(request, response, chain)
MaxRequestBodySizeFilter->>MaxRequestBodySizeFilter: check method mayHaveBody()
alt Content-Length present
MaxRequestBodySizeFilter->>MaxRequestBodySizeFilter: parse header as long
alt header > maxBytes
MaxRequestBodySizeFilter->>HttpResponseBuilder: set status 413 + body
MaxRequestBodySizeFilter->>ConnectionHandler: short-circuit response
ConnectionHandler->>Client: write/flush 413 response
else header within limit
MaxRequestBodySizeFilter->>FilterChain: chain.doFilter(request, response)
FilterChain->>ConnectionHandler: continue processing
end
else no Content-Length
MaxRequestBodySizeFilter->>MaxRequestBodySizeFilter: measure UTF-8 byte length of body
alt body bytes > maxBytes
MaxRequestBodySizeFilter->>HttpResponseBuilder: set status 413 + body
MaxRequestBodySizeFilter->>ConnectionHandler: short-circuit response
ConnectionHandler->>Client: write/flush 413 response
else
MaxRequestBodySizeFilter->>FilterChain: chain.doFilter(request, response)
FilterChain->>ConnectionHandler: continue processing
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/example/ConnectionHandler.java`:
- Around line 44-47: The MaxRequestBodySizeFilter is never able to enforce
actual byte-size because ConnectionHandler constructs the HttpRequest with an
empty body before filters run; fix by ensuring the raw request body bytes are
available to the filter: either run MaxRequestBodySizeFilter before building the
HttpRequest or construct the HttpRequest with the actual body stream/byte[]
(instead of an empty body) so MaxRequestBodySizeFilter can inspect the bytes;
update the ConnectionHandler code path that creates the HttpRequest and the
filter registration (AppConfig.MaxRequestBodyConfig / MaxRequestBodySizeFilter)
so the filter receives the raw body (or a reusable InputStream/byte supplier)
prior to any parsing or consumption.
In `@src/main/java/org/example/filter/MaxRequestBodySizeFilter.java`:
- Line 95: In MaxRequestBodySizeFilter locate the response.setBody call that
builds the rejection message (the one using contentLength and maxBytes) and
insert a space before the "bytes" token so the message reads "...: X bytes (max
Y)" instead of "...: Xbytes"; update the string concatenation in that
response.setBody invocation to include the space between the size value and
"bytes".
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/org/example/ConnectionHandler.javasrc/main/java/org/example/config/AppConfig.javasrc/main/java/org/example/filter/MaxRequestBodySizeFilter.javasrc/main/java/org/example/http/HttpResponseBuilder.javasrc/main/resources/application.ymlsrc/test/java/org/example/filter/MaxRequestBodySizeTest.java
Comment on lines
+44
to
+47
| AppConfig.MaxRequestBodyConfig maxBodyConfig = config.maxRequestBody(); | ||
| if (Boolean.TRUE.equals(maxBodyConfig.enabled())) { | ||
| list.add(new MaxRequestBodySizeFilter(maxBodyConfig.maxBytes())); | ||
| } |
There was a problem hiding this comment.
Filter integration currently bypasses parsed-body enforcement.
MaxRequestBodySizeFilter is added here, but in this execution path HttpRequest is constructed with an empty body on Line 71. That means the filter’s fallback byte-length check never runs, so enforcement is effectively limited to the declared Content-Length header only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/example/ConnectionHandler.java` around lines 44 - 47, The
MaxRequestBodySizeFilter is never able to enforce actual byte-size because
ConnectionHandler constructs the HttpRequest with an empty body before filters
run; fix by ensuring the raw request body bytes are available to the filter:
either run MaxRequestBodySizeFilter before building the HttpRequest or construct
the HttpRequest with the actual body stream/byte[] (instead of an empty body) so
MaxRequestBodySizeFilter can inspect the bytes; update the ConnectionHandler
code path that creates the HttpRequest and the filter registration
(AppConfig.MaxRequestBodyConfig / MaxRequestBodySizeFilter) so the filter
receives the raw body (or a reusable InputStream/byte supplier) prior to any
parsing or consumption.
Changed body from "" to null in ConnectionHandler to not give impressionthat body exists.
Fixed this line:
response.setBody("Payload too large: " + contentLength + " bytes (max " + maxBytes + ")");
Added
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
1 participant
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.
Resolves #99
Summary by CodeRabbit