feat(http): add SizeLimitHandler to enforce request body size limit#6658
feat(http): add SizeLimitHandler to enforce request body size limit#6658bladehan1 wants to merge 6 commits intotronprotocol:developfrom
Conversation
…imit Add SizeLimitHandler at the Jetty server level to reject oversized request bodies before they are fully buffered into memory. This prevents OOM attacks via arbitrarily large HTTP payloads that bypass the existing application-level Util.checkBodySize() check (which reads the entire body first) and the JSON-RPC interface (which had no size validation).
Introduce node.http.maxMessageSize and node.jsonrpc.maxMessageSize to allow HTTP and JSON-RPC services to enforce separate request body size limits via Jetty SizeLimitHandler, decoupled from gRPC config. - Default: 4 * GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE (16 MB) - Validation: reject <= 0 with TronError(PARAMETER_INIT) at startup - Each HttpService subclass sets its own maxRequestSize in constructor - SizeLimitHandlerTest covers independent limits, boundary, UTF-8 bytes
bladehan1
requested review from
0xbigapple,
317787106,
Sunny6889,
kuny0707,
lvs0075,
lxcmyf,
waynercheung and
xxo1shine
as code owners
|
@bladehan1 One observation on the config validation in |
|
@xxo1shine |
| } | ||
| } | ||
|
|
||
| @Deprecated |
There was a problem hiding this comment.
Marking this helper as deprecated does not make the new HTTP limit effective yet. PostParams.getPostParams() and many servlets still call Util.checkBodySize(), and that method is still enforcing parameter.getMaxMessageSize() (the gRPC limit), not httpMaxMessageSize. So any request whose body is > node.rpc.maxMessageSize but <= node.http.maxMessageSize will pass Jetty and then still be rejected in the servlet layer, which means the new independent HTTP setting is not actually honored for a large part of the API surface.
| ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); | ||
| context.setContextPath(this.contextPath); | ||
| this.apiServer.setHandler(context); | ||
| SizeLimitHandler sizeLimitHandler = new SizeLimitHandler(this.maxRequestSize, -1); |
There was a problem hiding this comment.
This moves oversized-request handling in front of every servlet, so those requests never reach RateLimiterServlet.service() / Util.processError(). Today the HTTP APIs consistently set application/json and serialize failures through Util.printErrorMsg(...); after this change an over-limit body gets Jetty's default 413 response instead. That is a client-visible behavior change for existing callers, and the new test only checks status codes so it would not catch the response-format regression.
3 participants
What does this PR do?
Add Jetty
SizeLimitHandlerat the server handler level to enforce request body size limits for all HTTP and JSON-RPC endpoints. Oversized requests are rejected with HTTP 413 before the body is fully buffered into memory.node.http.maxMessageSizeandnode.jsonrpc.maxMessageSizeas independent, configurable size limitsGrpcUtil.DEFAULT_MAX_MESSAGE_SIZE(4 MB), consistent with gRPC defaultsSizeLimitHandlerintoHttpService.initContextHandler()as the outermost handlerHttpServicesubclass (4 HTTP + 3 JSON-RPC) setsmaxRequestSizefrom the corresponding config getterUtil.checkBodySize()— retained as fallback for backward compatibilityWhy 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:
Closes #6604
This PR has been tested by:
SizeLimitHandlerTest: boundary, independent limits, UTF-8 byte counting)ArgsTest: default value alignment)Follow up
Util.checkBodySize()callers in a follow-up PR once this is stableExtra details
Files changed: 14 (+253 / -2)
HttpServicemaxRequestSizefield, wireSizeLimitHandlerininitContextHandler()Args/ConfigKey/CommonParameternode.http.maxMessageSizeandnode.jsonrpc.maxMessageSizemaxRequestSizefrom protocol-specific getter in constructorUtil.checkBodySize()@DeprecatedSizeLimitHandlerTestArgsTest