#3567 add test return HTTP 413 with JSON error when request body exceeds size l…

[robfrank]

…imit (#3567)

Undertow's built-in MAX_ENTITY_SIZE silently returned 400 with an empty body. Replace with a handler-level check that returns a proper 413 with a JSON error describing the limit and the config key to adjust it.

What does this PR do?

A brief description of the change being made with this pull request.

Motivation

What inspired you to submit this pull request?

Related issues

A list of issues either fixed, containing architectural discussions, otherwise relevant
for this Pull Request.

Additional Notes

Anything else we should know when reviewing?

Checklist

• I have run the build using mvn clean package command
• My unit tests cover both failure and success scenarios

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

The pull request enhances the ArcadeDB HTTP server's robustness by improving how it handles requests with excessively large bodies. Previously, such requests would result in a vague HTTP 400 error without specific details. This change introduces a dedicated handler that intercepts oversized requests, responds with an appropriate HTTP 413 status code, and provides a clear JSON error message. This message informs the client about the exceeded limit and guides them on how to configure the 'arcadedb.server.httpBodyContentMaxSize' setting, significantly improving the developer experience and API clarity.

Highlights

• Improved Error Handling for Oversized Request Bodies: Implemented a custom HTTP handler to enforce request body size limits, replacing Undertow's default behavior which previously resulted in a silent HTTP 400 error.
• Explicit HTTP 413 Response: Changed the server's response for oversized requests from a generic HTTP 400 to an explicit HTTP 413 (Request Entity Too Large).
• Detailed JSON Error Message: Provided a detailed JSON error message in the 413 response, including the exceeded limit, the actual request size, and the configuration key (arcadedb.server.httpBodyContentMaxSize) for adjustment.
• New Test Coverage: Added a new test suite (HttpBodySizeLimitTest) to validate the correct handling of both oversized and valid request bodies, ensuring the new error mechanism functions as expected.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• server/src/main/java/com/arcadedb/server/http/HttpServer.java
  • Imported necessary Undertow classes (HttpHandler, HttpServerExchange, Headers, StatusCodes) for custom HTTP handling.
  • Modified the buildUndertowServer method to set UndertowOptions.MAX_ENTITY_SIZE to Long.MAX_VALUE, effectively disabling Undertow's built-in body size check.
  • Integrated a new createBodySizeLimitHandler into the server's handler chain to manage body size limits.
  • Added the createBodySizeLimitHandler method, which checks the request's Content-Length against GlobalConfiguration.SERVER_HTTP_BODY_CONTENT_MAX_SIZE and sends a 413 response with a JSON error if the limit is exceeded.
• server/src/test/java/com/arcadedb/server/http/HttpBodySizeLimitTest.java
  • Created a new test class HttpBodySizeLimitTest to cover HTTP body size limit scenarios.
  • Implemented onServerConfiguration to set a small SERVER_HTTP_BODY_CONTENT_MAX_SIZE (1KB) for controlled testing.
  • Added requestExceedingBodyLimitReturns413WithJsonError to verify that requests exceeding the configured limit receive an HTTP 413 status and a descriptive JSON error body.
  • Included requestWithinBodyLimitSucceeds to confirm that requests with body sizes within the configured limit are processed successfully with an HTTP 200 status.
  • Provided a helper method readErrorStream to facilitate extracting error messages from HTTP connections during testing.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[claude]

Code Review

Good approach overall. A few issues worth addressing.

Critical: Chunked Transfer Encoding Bypass

The check relies solely on the Content-Length header. When a client uses chunked transfer encoding (Transfer-Encoding: chunked), getRequestContentLength() returns -1 and the check is silently skipped. The size limit is therefore trivially bypassed, removing DoS protection for chunked requests — arguably worse than before since Undertow MAX_ENTITY_SIZE handled chunked bodies correctly at the I/O layer. Either reject requests without Content-Length when maxEntitySize > 0, or wrap the exchange input stream to count actual bytes read. At minimum, document this as a known limitation.

Style: Hardcoded Config Key in Error Message

The error message hardcodes the string arcadedb.server.httpBodyContentMaxSize. Use GlobalConfiguration.SERVER_HTTP_BODY_CONTENT_MAX_SIZE.getKey() so the message stays correct if the key is ever renamed.

Style: JSON via String Concatenation

Per CLAUDE.md, JSON should be built with com.arcadedb.serializer.json.JSONObject rather than string concatenation. While long values cannot cause JSON injection here, this violates the project convention.

Minor: Condition Semantics for maxEntitySize

The config doc says -1 means unlimited, but the guard skips enforcement for any value <= 0. A value of 0 therefore silently disables the limit. Worth documenting or aligning with the config description.

Minor: Redundant connect() in Test

In requestWithinBodyLimitSucceeds(), connection.connect() is called after writing to the output stream. Writing already triggers the connection implicitly.

Test: Missing Content-Type Assertion

The 413 test verifies the response body but does not assert Content-Type: application/json even though the handler explicitly sets that header.

Summary: the chunked-encoding bypass is the main concern; the rest are polish. Once addressed this should be in good shape.

[gemini-code-assist]

Code Review

This pull request introduces a custom Undertow handler to provide a more informative error (HTTP 413 with a JSON body) when a request body exceeds the configured size limit. This is a good improvement over Undertow's default behavior. The added tests correctly verify the new behavior for requests with a Content-Length header. However, I have identified a potential security issue related to requests that use chunked encoding, which bypass the new size check. I've also included a minor suggestion to improve code readability.

[gemini-code-assist]

This handler only checks for requests with a Content-Length header. Requests using chunked encoding (Transfer-Encoding: chunked) will bypass this check because exchange.getRequestContentLength() returns -1 for them. Since UndertowOptions.MAX_ENTITY_SIZE is now set to Long.MAX_VALUE, this could allow an attacker to send an arbitrarily large chunked request, potentially leading to an OutOfMemoryError and a Denial of Service. This is a regression from the previous implementation that used UndertowOptions.MAX_ENTITY_SIZE, which correctly handles all request types. A more robust solution is needed to handle chunked requests as well.

[gemini-code-assist]

Manual JSON string concatenation using + is fragile and can be hard to read. Using String.format() would make this code cleaner, more readable, and less prone to concatenation errors.

          exchange.getResponseSender().send(String.format("{\"error\":\"Request body too large\",\"detail\":\"Request body size (%d bytes) exceeds the maximum allowed size of %d bytes. Configure 'arcadedb.server.httpBodyContentMaxSize' to increase the limit.\"}", contentLength, maxEntitySize));

[mergify]

🧪 CI Insights

Here's what we observed from your CI run for 8dc0266.

🟢 All jobs passed!

But CI Insights is watching 👀

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for 6f3e9ef1	✅ 100.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (6f3e9ef)	Report Missing	Report Missing	Report Missing
Head commit (499d465)	140644	91775	65.25%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#3669)	13	13	100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Footnotes

1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

[claude]

test

[claude]

test2

[claude]

Code Review - Issue 1: Size limit bypass via chunked encoding. Issue 2: Silent IOException in readErrorStream. Issue 3: Redundant connection.connect(). Issue 4: Loose assertion contains error. Issue 5: No response body validation. See detailed review in follow-up comments.

[claude]

Removing placeholder comments above - full review below.

[claude]

Code Review

[claude]

This PR adds a regression test (HttpBodySizeLimitTest) for the HTTP 413 response behaviour introduced by createBodySizeLimitHandler in HttpServer.java. The implementation itself is already on main - the PR is test-only.

[claude]

Issue 1 - Size limit bypass via chunked encoding: createBodySizeLimitHandler only checks exchange.getRequestContentLength(), which returns -1 when the client omits Content-Length and uses Transfer-Encoding: chunked. A client doing so bypasses the limit entirely. Consider adding a test that sends a chunked request over the limit to document the known limitation or verify a future fix.

[claude]

Issue 2 - Silent catch in readErrorStream masks assertion failures: If an IOException occurs reading the error stream, the method returns an empty string and the subsequent assertion assertThat(errorBody).contains('error') fails with a misleading 'expected to contain error but was empty' message rather than revealing the true cause. The exception should at minimum be rethrown as an unchecked exception so the test failure is diagnosable.

[claude]

Issue 3 - Redundant connection.connect() in requestWithinBodyLimitSucceeds: getOutputStream() already implicitly opens the connection, so the explicit connection.connect() call after writing the body is a no-op. Remove it to avoid confusion.

[claude]

Issue 4 - Test assertions are too loose: assertThat(errorBody).contains('error') would pass for any JSON that mentions the word 'error' (e.g. an unrelated server-side exception). A tighter assertion against the specific message is better - e.g. assertThat(errorBody).contains('Request body too large') - to make the test actually validate the right code path.

[claude]

Issue 5 - requestWithinBodyLimitSucceeds does not validate the response body: The success test only checks for HTTP 200 but ignores the response body. A lightweight check that the body is well-formed JSON with a 'result' key would make it more meaningful and catch regressions where the request goes through but the query itself fails silently.

[claude]

Minor / Nits: The test sends raw bytes (x repeated) with Content-Type: application/json - acceptable since the size check fires before body parsing, but a brief comment explaining the intentionally invalid body content would help readers. The PR checklist items ('I have run the build...' and 'My unit tests cover both failure and success scenarios') are unchecked - please verify both before merge.

[claude]

Summary: The test covers the primary happy and sad paths for the Content-Length-based limit check and is a valuable regression guard. The two items most worth addressing before merge are: (a) the readErrorStream silent-catch which makes test failures hard to diagnose, and (b) the loose contains('error') assertion. The chunked-encoding bypass is a pre-existing limitation in the implementation, but is worth at least documenting with a comment or a skipped test.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -9.47%	✅ ∅

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (21ba9b7)	109700	81957	74.71%
Head commit (8dc0266)	140644 (+30944)	91750 (+9793)	65.24% (-9.47%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#3669)	0	0	∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.82%. Comparing base (21ba9b7) to head (8dc0266).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3669      +/-   ##
==========================================
- Coverage   65.83%   65.82%   -0.01%     
==========================================
  Files        1550     1550              
  Lines      109700   109700              
  Branches    22878    22878              
==========================================
- Hits        72218    72210       -8     
- Misses      27768    27804      +36     
+ Partials     9714     9686      -28     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.