Feature/body size filter - #139
Merged
Merged
Conversation
Add support for rejecting too large bodies with the proper HTTP code
add getters for body size config
HTTP method checking implement Content-Length validation in BodySizeFilter Reject requests with missing, invalid, or oversized Content-Length headers with HTTP 413 add HTTP method checking to BodySizeFilter Only validate body size for POST/PUT/PATCH requests. Allow all others to pass.
Enable request body size limiting with 10MB default maximum.
Run code formatter to comply with project style guidelines. All tests passing.
📝 WalkthroughWalkthroughAdds request-body-size configuration and enforcement: new BodySizeConfig and BodySizeFilter, config keys in ConfigLoader and application-properties, new HTTP statuses (411, 413), and unit tests validating filter behavior. Changes
Sequence DiagramsequenceDiagram
participant Client
participant BodySizeFilter
participant ConfigLoader
participant FilterChain
participant HttpResponse
Client->>BodySizeFilter: HTTP request (POST/PUT/PATCH)
BodySizeFilter->>ConfigLoader: isBodySizeEnabled()
ConfigLoader-->>BodySizeFilter: enabled
alt Disabled
BodySizeFilter->>FilterChain: forward request
else Enabled
BodySizeFilter->>BodySizeFilter: read Content-Length header
alt Missing or non-numeric or negative
BodySizeFilter->>HttpResponse: set 411/400/413 with body
HttpResponse-->>Client: error response
else Present
BodySizeFilter->>ConfigLoader: getMaxBodySizeMb()
ConfigLoader-->>BodySizeFilter: maxSizeMb
alt Content-Length > maxSizeBytes
BodySizeFilter->>HttpResponse: set 413 Payload Too Large
HttpResponse-->>Client: 413 response
else Within limit
BodySizeFilter->>FilterChain: forward request
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 3
🧹 Nitpick comments (2)
src/main/java/org/juv25d/filter/BodySizeFilter.java (1)
108-114: PreferHttpStatus.PAYLOAD_TOO_LARGEover hardcoded 413/text.This avoids status drift and keeps response construction aligned with the centralized enum.
Proposed refactor
+import org.juv25d.http.HttpStatus; @@ private void sendPayloadTooLarge(HttpResponse res, String message) { - byte[] body = ("413 Payload Too Large: " + message + "\n") + byte[] body = (HttpStatus.PAYLOAD_TOO_LARGE.getCode() + " " + + HttpStatus.PAYLOAD_TOO_LARGE.getDescription() + ": " + message + "\n") .getBytes(StandardCharsets.UTF_8); - res.setStatusCode(413); - res.setStatusText("Payload Too Large"); + res.setStatusCode(HttpStatus.PAYLOAD_TOO_LARGE.getCode()); + res.setStatusText(HttpStatus.PAYLOAD_TOO_LARGE.getDescription());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/BodySizeFilter.java` around lines 108 - 114, Replace the hardcoded numeric/status text in sendPayloadTooLarge: use the centralized HttpStatus.PAYLOAD_TOO_LARGE enum value to set the response status and reason instead of 413 and "Payload Too Large". Call the appropriate getters on HttpStatus.PAYLOAD_TOO_LARGE to populate res.setStatusCode(...) and res.setStatusText(...) (leaving headers/body as-is) so the handler stays in sync with the shared status enum.src/test/java/org/juv25d/filter/BodySizeFilterTest.java (1)
110-120: Add regression tests for negative and case-variantContent-Length.Given this filter’s core responsibility, it’s worth pinning behavior for
Content-Length: -1and lowercase header key forms to prevent future regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/juv25d/filter/BodySizeFilterTest.java` around lines 110 - 120, Add two regression tests to BodySizeFilterTest: one named shouldBlockRequest_whenContentLengthNegative that constructs BodySizeFilter(10), stubs req.method() to "POST" and req.headers() to Map.of("Content-Length", "-1") (or "content-length" variant if header normalization is expected), calls filter.doFilter(req, res, chain), then asserts verifyNoInteractions(chain) and verify(res).setStatusCode(413); and another named shouldHandleRequest_whenContentLengthHeaderLowercase that stubs req.method() to "POST" and req.headers() to Map.of("content-length", "5"), calls filter.doFilter(...), then asserts the request proceeds via verify(chain).doFilter(req, res) (and does not set 413). Target the existing test class BodySizeFilterTest and the BodySizeFilter.doFilter behavior for locating where to add these tests.
🤖 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/juv25d/filter/BodySizeFilter.java`:
- Around line 60-67: The Content-Length parsing in BodySizeFilter currently
allows negative values; after parsing contentLength into bodySize in the try
block, add a check for bodySize < 0 and treat it as invalid by logging and
returning a 413 like the oversized case (use the same logging helper
logBodySizeExceeded(req, bodySize) or an appropriate negative-value log and call
sendPayloadTooLarge(res, "...") with a message indicating an invalid negative
Content-Length), otherwise keep the existing maxSizeBytes comparison; ensure
this logic sits inside the same try block that parses
Long.parseLong(contentLength) so NumberFormatException still falls through to
existing handling.
In `@src/main/java/org/juv25d/util/ConfigLoader.java`:
- Around line 117-123: ConfigLoader currently accepts non-positive
request-body-size.max-size-mb; update the bodySizeConfig handling (in the block
that sets requestBodySizeEnabled and maxBodySizeMb) to parse the "max-size-mb"
value as a long, verify it is > 0, and if not throw a clear configuration
exception (e.g., IllegalArgumentException) mentioning
"request-body-size.max-size-mb" and the invalid value; keep the existing default
(10L) when missing, but enforce the positive check after parsing and before
assigning to the maxBodySizeMb field.
In `@src/test/java/org/juv25d/filter/BodySizeFilterTest.java`:
- Around line 67-70: The test method name should reflect that the setup
simulates a missing Content-Length header rather than a zero length body: rename
the test method shouldBlockRequest_whenMethodHasBodyButSizeIsZero() to something
like shouldBlockRequest_whenMethodHasBodyButNoContentLength() (or similar) to
match the arrangement using BodySizeFilter, the mocked req.method() returning
"POST", and req.headers() returning an empty map.
---
Nitpick comments:
In `@src/main/java/org/juv25d/filter/BodySizeFilter.java`:
- Around line 108-114: Replace the hardcoded numeric/status text in
sendPayloadTooLarge: use the centralized HttpStatus.PAYLOAD_TOO_LARGE enum value
to set the response status and reason instead of 413 and "Payload Too Large".
Call the appropriate getters on HttpStatus.PAYLOAD_TOO_LARGE to populate
res.setStatusCode(...) and res.setStatusText(...) (leaving headers/body as-is)
so the handler stays in sync with the shared status enum.
In `@src/test/java/org/juv25d/filter/BodySizeFilterTest.java`:
- Around line 110-120: Add two regression tests to BodySizeFilterTest: one named
shouldBlockRequest_whenContentLengthNegative that constructs BodySizeFilter(10),
stubs req.method() to "POST" and req.headers() to Map.of("Content-Length", "-1")
(or "content-length" variant if header normalization is expected), calls
filter.doFilter(req, res, chain), then asserts verifyNoInteractions(chain) and
verify(res).setStatusCode(413); and another named
shouldHandleRequest_whenContentLengthHeaderLowercase that stubs req.method() to
"POST" and req.headers() to Map.of("content-length", "5"), calls
filter.doFilter(...), then asserts the request proceeds via
verify(chain).doFilter(req, res) (and does not set 413). Target the existing
test class BodySizeFilterTest and the BodySizeFilter.doFilter behavior for
locating where to add these tests.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/org/juv25d/config/BodySizeConfig.javasrc/main/java/org/juv25d/filter/BodySizeFilter.javasrc/main/java/org/juv25d/http/HttpStatus.javasrc/main/java/org/juv25d/util/ConfigLoader.javasrc/main/resources/application-properties.ymlsrc/test/java/org/juv25d/filter/BodySizeFilterTest.java
Closed
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/org/juv25d/filter/BodySizeFilterTest.java (1)
134-144: Add a whitespace-value regression test forContent-Length.You already cover lowercase header keys well (Line 135+). Add one case for values like
" 5 "to lock expected behavior around header value normalization.🧪 Proposed test addition
+ `@Test` + void shouldAllowRequest_whenContentLengthHasSurroundingWhitespace() throws IOException { + BodySizeFilter filter = new BodySizeFilter(10); + when(req.method()).thenReturn("POST"); + when(req.headers()).thenReturn(Map.of("Content-Length", " 5 ")); + + filter.doFilter(req, res, chain); + + verify(chain, times(1)).doFilter(req, res); + verifyNoInteractions(res); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/juv25d/filter/BodySizeFilterTest.java` around lines 134 - 144, Add a regression test in BodySizeFilterTest to ensure Content-Length header values with surrounding whitespace are handled correctly: create a test (e.g., shouldAllowRequest_whenContentLengthHeaderValueHasWhitespace) that instantiates BodySizeFilter(10), mocks req.method() to "POST" and req.headers() to return Map.of("content-length", " 5 "), calls filter.doFilter(req, res, chain), and then verifies chain.doFilter(req, res) was called once and that there were no interactions with res; this locks expected normalization/trim behavior in the BodySizeFilter logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/test/java/org/juv25d/filter/BodySizeFilterTest.java`:
- Around line 134-144: Add a regression test in BodySizeFilterTest to ensure
Content-Length header values with surrounding whitespace are handled correctly:
create a test (e.g.,
shouldAllowRequest_whenContentLengthHeaderValueHasWhitespace) that instantiates
BodySizeFilter(10), mocks req.method() to "POST" and req.headers() to return
Map.of("content-length", " 5 "), calls filter.doFilter(req, res, chain), and
then verifies chain.doFilter(req, res) was called once and that there were no
interactions with res; this locks expected normalization/trim behavior in the
BodySizeFilter logic.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/juv25d/filter/BodySizeFilter.javasrc/main/java/org/juv25d/util/ConfigLoader.javasrc/test/java/org/juv25d/filter/BodySizeFilterTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/juv25d/util/ConfigLoader.java
test to verify that Content-Length values with surrounding whitespace are trimmed and handled correctly per HTTP specification.
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/juv25d/filter/BodySizeFilter.java`:
- Around line 34-38: The BodySizeConfig/BodySizeFilter flow must validate
maxSizeMb and prevent MB→bytes overflow: in BodySizeConfig (constructor/getter)
validate that maxSizeMb is non-negative and within a safe upper bound (e.g., <=
Long.MAX_VALUE / (1024*1024)), and in BodySizeFilter constructors (the
BodySizeFilter() and the other constructor around lines 24-29) perform the same
check before assigning maxSizeBytes; if validation fails throw an
IllegalArgumentException so the app fails fast rather than silently overflowing
or producing a negative maxSizeBytes, and compute maxSizeBytes using a checked
multiplication (or cast/compare) to guarantee no overflow when multiplying
getMaxSizeMb() by 1024*1024.
- Around line 54-56: In BodySizeFilter replace the stream predicate that does
e.getKey().equalsIgnoreCase("Content-Length") with a null-safe check so null
header keys don't throw; for example, change the filter to check the key is
non-null before calling equalsIgnoreCase or use
"Content-Length".equalsIgnoreCase(e.getKey()) in the lambda that builds the
contentLength from req.headers().entrySet().stream(), keeping the subsequent
.map(Map.Entry::getValue) logic intact.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/org/juv25d/filter/BodySizeFilter.javasrc/test/java/org/juv25d/filter/BodySizeFilterTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/org/juv25d/filter/BodySizeFilterTest.java
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/org/juv25d/filter/BodySizeFilter.java (1)
34-38:⚠️ Potential issue | 🟡 MinorSkip
toBytes()when filter is disabled to avoid spurious startup failures.If the filter is disabled in configuration, the
maxSizeBytesvalue is never used. CallingtoBytes()unconditionally means a disabled filter with an invalidmaxSizeMbconfig (e.g., 0 or negative) will throw at startup, even though the feature is off.Proposed fix
public BodySizeFilter() { BodySizeConfig config = new BodySizeConfig(); this.enabled = config.isEnabled(); - this.maxSizeBytes = toBytes(config.getMaxSizeMb()); + this.maxSizeBytes = this.enabled ? toBytes(config.getMaxSizeMb()) : 0L; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/BodySizeFilter.java` around lines 34 - 38, The constructor in BodySizeFilter currently calls toBytes(config.getMaxSizeMb()) unconditionally which can throw on invalid config even when the filter is disabled; change the BodySizeFilter() constructor to first read enabled = config.isEnabled() and only call toBytes(config.getMaxSizeMb()) to set maxSizeBytes when enabled is true (otherwise set maxSizeBytes to a safe default like 0 or leave it unset) so disabled filters do not trigger startup failures; update references to BodySizeConfig.getMaxSizeMb(), the enabled field, and the toBytes(...) helper accordingly.
🧹 Nitpick comments (2)
src/main/java/org/juv25d/filter/BodySizeFilter.java (2)
17-17: Minor: Missing space before opening brace.Formatting nit:
implements Filter{should have a space before{.-public class BodySizeFilter implements Filter{ +public class BodySizeFilter implements Filter {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/BodySizeFilter.java` at line 17, Formatting nit: in the class declaration for BodySizeFilter (public class BodySizeFilter implements Filter{) add a space before the opening brace so it reads "implements Filter {", ensuring the class signature follows the project's spacing conventions.
98-101: Consider null-safe method comparison.If
req.method()could return null (e.g., malformed request), callingmethod.equalsIgnoreCase(...)would throw NPE. Using the constant on the left side makes this null-safe.Proposed fix
private boolean shouldCheckBodySize(HttpRequest req) { String method = req.method(); - return method.equalsIgnoreCase("POST") || method.equalsIgnoreCase("PUT") || method.equalsIgnoreCase("PATCH"); + return "POST".equalsIgnoreCase(method) || "PUT".equalsIgnoreCase(method) || "PATCH".equalsIgnoreCase(method); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/BodySizeFilter.java` around lines 98 - 101, The shouldCheckBodySize(HttpRequest req) method calls req.method() and then invokes equalsIgnoreCase on that result which can NPE if req.method() is null; change the comparison to be null-safe by either checking req.method() for null first or by calling equalsIgnoreCase on the constant strings (e.g., "POST".equalsIgnoreCase(req.method())) for each check so that shouldCheckBodySize returns true for POST/PUT/PATCH without risking a NullPointerException.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main/java/org/juv25d/filter/BodySizeFilter.java`:
- Around line 34-38: The constructor in BodySizeFilter currently calls
toBytes(config.getMaxSizeMb()) unconditionally which can throw on invalid config
even when the filter is disabled; change the BodySizeFilter() constructor to
first read enabled = config.isEnabled() and only call
toBytes(config.getMaxSizeMb()) to set maxSizeBytes when enabled is true
(otherwise set maxSizeBytes to a safe default like 0 or leave it unset) so
disabled filters do not trigger startup failures; update references to
BodySizeConfig.getMaxSizeMb(), the enabled field, and the toBytes(...) helper
accordingly.
---
Nitpick comments:
In `@src/main/java/org/juv25d/filter/BodySizeFilter.java`:
- Line 17: Formatting nit: in the class declaration for BodySizeFilter (public
class BodySizeFilter implements Filter{) add a space before the opening brace so
it reads "implements Filter {", ensuring the class signature follows the
project's spacing conventions.
- Around line 98-101: The shouldCheckBodySize(HttpRequest req) method calls
req.method() and then invokes equalsIgnoreCase on that result which can NPE if
req.method() is null; change the comparison to be null-safe by either checking
req.method() for null first or by calling equalsIgnoreCase on the constant
strings (e.g., "POST".equalsIgnoreCase(req.method())) for each check so that
shouldCheckBodySize returns true for POST/PUT/PATCH without risking a
NullPointerException.
|
ser bra ut :) |
EmmaTravljanin
previously approved these changes
Feb 26, 2026
jesperlarsson1910
left a comment
jesperlarsson1910
left a comment
There was a problem hiding this comment.
Line 66 we call the too large method but the if statement is for null or empty, should that call something else?
Refactored BodySizeFilter to return 411 Length Required for missing headers and 400 Bad Request for invalid values, instead of a blanket 413. Expanded HttpStatus enum to include 411.
Updated test expectations in BodySizeFilterTest to align with new error handling. Tests now verify 411 for missing headers and 400 for invalid/negative values while retaining 413 for actual limit violations.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/org/juv25d/http/HttpStatus.java (1)
24-24: Minor: Extra blank line.There's an unnecessary double blank line between the enum constants and the field declarations. Consider removing it for consistency.
🧹 Suggested fix
PAYLOAD_TOO_LARGE(413, "Payload Too Large"), INTERNAL_SERVER_ERROR(500, "Internal Server Error"), NOT_IMPLEMENTED(501, "Not Implemented"), BAD_GATEWAY(502, "Bad Gateway"), GATEWAY_TIMEOUT(504, "Gateway Timeout"), UNKNOWN(-1, "Unknown Status"); - private final int code;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/http/HttpStatus.java` at line 24, In the HttpStatus enum remove the extra blank line between the enum constants and the subsequent field declarations so the constants and fields are directly adjacent; update the HttpStatus enum formatting to have a single blank line (or none) separating constants from fields to match project style and ensure consistent formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/java/org/juv25d/http/HttpStatus.java`:
- Line 24: In the HttpStatus enum remove the extra blank line between the enum
constants and the subsequent field declarations so the constants and fields are
directly adjacent; update the HttpStatus enum formatting to have a single blank
line (or none) separating constants from fields to match project style and
ensure consistent formatting.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/juv25d/filter/BodySizeFilter.javasrc/main/java/org/juv25d/http/HttpStatus.javasrc/test/java/org/juv25d/filter/BodySizeFilterTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/juv25d/filter/BodySizeFilter.java
- src/test/java/org/juv25d/filter/BodySizeFilterTest.java
Author
|
jesperlarsson1910
approved these changes
Feb 26, 2026
jesperlarsson1910
left a comment
jesperlarsson1910
left a comment
There was a problem hiding this comment.
Can't see anything that sticks out now, good job!
EmmaTravljanin
approved these changes
Feb 27, 2026
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
3 participants
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.
Summary
Changes
Testing
Summary by CodeRabbit
New Features
Configuration
Tests