feat(http): add SizeLimitHandler to enforce request body size limit

[bladehan1]

What does this PR do?

Add Jetty SizeLimitHandler at the server handler level to enforce request body size limits for all HTTP and JSON-RPC endpoints, preventing OOM denial-of-service from oversized payloads.

Changes

• Introduce node.http.maxMessageSize and node.jsonrpc.maxMessageSize as independent, configurable size limits
• Default: 4 MB, consistent with gRPC defaults
• Support human-readable size values (e.g. 4m, 128MB) via HOCON getMemorySize() for all three size configs
• Wire SizeLimitHandler into HttpService.initContextHandler() as the outermost handler
• Each HttpService subclass (4 HTTP + 3 JSON-RPC) sets maxRequestSize from the corresponding config getter
• Initialize maxRequestSize with a safe 4MB default to prevent silent reject-all if a future subclass omits the assignment
• Deprecate Util.checkBodySize() — updated to use httpMaxMessageSize, retained as fallback for backward compatibility
• Existing node.rpc.maxMessageSize now also supports human-readable sizes (backward compatible — bare integers still treated as bytes)

Why are these changes required?

Previously, HTTP request body size was only validated at the application layer (Util.checkBodySize()), which reads the entire body into memory before checking. The JSON-RPC interface had no size validation at all. This allows an attacker to send arbitrarily large payloads, causing OOM and denial of service.

Moving the limit to the Jetty handler chain provides:

1. OOM protection — oversized payloads are never fully buffered into memory
2. Streaming enforcement — limits are enforced during read, not after full buffering
3. Unified coverage — all HTTP and JSON-RPC endpoints are protected by a single mechanism
4. Independent tuning — operators can configure HTTP and JSON-RPC limits separately

Scope and known limitations

This PR's primary goal is OOM protection, not uniform HTTP 413 responses.

Chunked transfer behavior differs by servlet type due to a pre-existing exception handling design in the servlet chain:

Request type	HTTP Servlets (132)	JSON-RPC Servlet
Content-Length exceeds limit	413 (Jetty rejects before dispatch)	413 (same)
Chunked / no Content-Length, exceeds limit	200 + error JSON {"Error":"BadMessageException"}	200 + empty body

Root cause: SizeLimitHandler truncates body read at the limit and throws BadMessageException (RuntimeException) during streaming.

• HTTP servlets have their own catch(Exception) → Util.processError() writes error JSON to the response body, so the client sees 200 + {"Error":"..."}.
• JsonRpcServlet has no try/catch in doPost() → exception escapes to RateLimiterServlet.service() → catch(Exception unexpected) only logs, does not set status or write body → client sees 200 + empty body.

This is a pre-existing design issue in RateLimiterServlet (line 119: silent catch(Exception unexpected)), not introduced by this PR. Any RuntimeException escaping from JsonRpcServlet.doPost() would produce the same 200 empty body result, with or without SizeLimitHandler. OOM protection is effective in all cases — the body read is truncated regardless of the response status code.

Closes #6604

This PR has been tested by

• SizeLimitHandlerTest (10 tests): boundary, independent limits, UTF-8 byte counting, chunked transfer, zero-limit, checkBodySize consistency
• JsonrpcServiceTest.testJsonRpcSizeLimitIntegration: real JSON-RPC integration test covering normal passthrough, Content-Length oversized (413), and chunked oversized (200 empty body)
• ArgsTest (5 new tests): human-readable sizes (KB/MB/GB × binary/SI), raw integer backward compatibility, zero-value, error paths (exceeds int max, negative, invalid unit, non-numeric)
• UtilTest: checkBodySize uses httpMaxMessageSize

Follow-up

• Fix RateLimiterServlet.catch(Exception unexpected) to return 500 + error JSON instead of silent 200 empty body (pre-existing issue, affects all RuntimeExceptions from any servlet, not specific to SizeLimitHandler)
• Remove Util.checkBodySize() callers once SizeLimitHandler is stable

Changed files

16 files changed, +617 / -8

Component	Changes
HttpService	Add maxRequestSize field (default 4MB), wire SizeLimitHandler in initContextHandler()
Args / ConfigKey / CommonParameter	Parse node.http.maxMessageSize and node.jsonrpc.maxMessageSize; refactor all three to use getMemorySize()
7 service subclasses	Set maxRequestSize from protocol-specific config getter
Util.checkBodySize()	Mark @Deprecated, switch to httpMaxMessageSize
config.conf	Add documented config examples for HTTP, RPC, JSON-RPC size limits with zero-value behavior
SizeLimitHandlerTest	New: 10 tests covering HTTP limits, boundary, UTF-8, chunked, independence, zero-limit, checkBodySize consistency
JsonrpcServiceTest	New: testJsonRpcSizeLimitIntegration — real JSON-RPC integration test
ArgsTest	New: 5 tests for human-readable parsing (KB/MB/GB), error paths
UtilTest	New: checkBodySize consistency test

[xxo1shine]

@bladehan1 One observation on the config validation in Args.java: the guard currently rejects <= 0 values with TronError, but there is no upper-bound check. An operator who accidentally sets node.http.maxMessageSize = 2147483647 would silently run with a 2 GB limit. Adding a reasonable maximum (e.g. 128 MB) with an explicit warn-or-throw would make misconfiguration more visible.

[bladehan1]

@xxo1shine
Thanks for the review. I think that when users manually configure values, it's unnecessary to set all boundaries except for error-prone values. Especially for values ​​that are obviously likely to have problems, setting boundaries is unnecessary.

[waynercheung]

[SHOULD] With the updated PR scope, I no longer see the chunked 413 gap itself as a contract mismatch, since the description now explicitly scopes this PR to OOM protection rather than uniform HTTP status behavior. The remaining gap is test realism: SizeLimitHandlerTest still does not exercise the production JSON-RPC stack, because the /jsonrpc path is backed by a synthetic BroadCatchServlet, not the real JsonRpcServlet -> jsonrpc4j chain. Since the PR still claims protection for JSON-RPC endpoints, I strongly recommend either adding one real JSON-RPC integration case or making the test naming/comments explicit that this is validating generic HttpInput interception rather than end-to-end JSON-RPC behavior.

[waynercheung]

[NIT] SizeLimitHandlerTest relies heavily on banner-style ruler comments such as // -- ... -------- throughout the file. This style adds visual noise without structural value, and is harder to maintain - new tests may not fit neatly into an existing section, and the dashes need manual alignment. The usual best practice here is to rely on descriptive test names, minimal section comments, and helper methods / smaller test classes for grouping, rather than heavy visual separators.

Note also that one comment has already drifted from the implementation: the Javadoc on testChunkedBodyWithinLimit still reads "succeed (EchoServlet)", but the servlet under test is now BroadCatchServlet. Please update that reference while cleaning up the comments.

[bladehan1]

banner-style ruler comments / stale EchoServlet Javadoc

Done. Removed all banner-style ruler comments and fixed the stale EchoServlet reference in testChunkedBodyWithinLimit Javadoc. See latest push.

[bladehan1]

JSON-RPC test realism / BroadCatchServlet does not exercise the real jsonrpc4j chain

Agreed — the PR inserts SizeLimitHandler on the JSON-RPC path, so we should verify no regression on the real stack.

Added testJsonRpcSizeLimitIntegration() in JsonrpcServiceTest, using the Spring-injected FullNodeJsonRpcHttpService (real JsonRpcServlet + jsonrpc4j). Covers three scenarios with explicit expected results:

1. Normal JSON-RPC request through SizeLimitHandler → 200 with valid "result" (no regression)
2. Content-Length oversized → 413 before servlet dispatch
3. Chunked oversized → 200 with empty body

Scenario 3 deserves explanation: BadMessageException (RuntimeException) thrown during body read propagates through jsonrpc4j → out of JsonRpcServlet.doPost() (no try/catch) → caught by RateLimiterServlet.catch(Exception unexpected) which logs but does not write a response. Jetty defaults to 200 empty body. OOM protection is still effective — the body read is truncated at the limit.

The silent swallowing in RateLimiterServlet is a pre-existing design issue (affects all RuntimeExceptions from any servlet, not just BadMessageException). Fixing it impacts 100+ servlets and belongs in a dedicated PR.

Also cleaned up SizeLimitHandlerTest: removed the 3 redundant testJsonRpcBody* tests (BroadCatchServlet cannot represent the real jsonrpc4j chain), renamed TestJsonRpcService → SecondHttpService, kept only testTwoServicesHaveIndependentLimits for validating per-service limit isolation.

See latest push.

[waynercheung]

Re-checking the current PR head (6f0a0f0), I still do not see the changes described in the reply on this branch yet.

Specifically:

• SizeLimitHandlerTest still contains the banner-style section comments, and the stale EchoServlet reference is still present.
• I do not see the new testJsonRpcSizeLimitIntegration() in JsonrpcServiceTest.
• The synthetic /jsonrpc tests in SizeLimitHandlerTest are still present.
• Util.checkBodySize() is still using body.getBytes() with the platform default charset.

If there is a newer push on another branch or commit, please point me to it. Otherwise I think these threads should stay open until the code is actually updated in this PR.

[bladehan1]

@waynercheung

The push was a little late, but you can view it now.
As mentioned above, Util.checkBodySize() no longer adds body.getBytes.
Another issue has been updated.

[waynercheung]

[NIT] After tracing the actual path more closely, this comment is not just stronger than what the test proves - the mechanism is also inaccurate. For chunked / unknown-length requests, the over-limit exception is raised while jsonrpc4j is reading the request body, and JsonRpcServer.handle(...) catches it internally before it can propagate back to RateLimiterServlet. Since JsonRpcServlet configures the JSON-RPC HttpStatusCodeProvider to always return 200, the current client-visible behavior becomes 200 + empty body. Please reword this comment/assertion accordingly.