fix(jsonrpc): make the JSON-RPC output more compliant with specification#6763
Open
317787106 wants to merge 1 commit into
Open
fix(jsonrpc): make the JSON-RPC output more compliant with specification#6763317787106 wants to merge 1 commit into
317787106 wants to merge 1 commit into
Conversation
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
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.
What does this PR do?
Fixes three JSON-RPC spec-compliance issues and one parser-hardening gap in
JsonRpcServlet:1.
Content-Typeresponse header — remove spurious; charset=utf-8The JSON-RPC 1.0/2.0 spec and Ethereum JSON-RPC both use
application/jsonorapplication/json-rpcwithout a charset suffix. The extra; charset=utf-8suffix was non-standard and has been removed.2. Non-object/non-array root node →
Invalid RequestSending a JSON primitive as the request body (e.g.
null,true,123) was silently forwarded tojsonrpc4j, which produced a malformed response with"id":"null"(string) instead of"id":null(JSON null). Now these cases are caught before dispatch and return a proper-32600 Invalid Requestresponse with"id":null.Before:
After:
3. Non-object element inside a batch →
Invalid RequestA batch containing a non-object element (e.g.
[[]]) was forwarded per-element tojsonrpc4j, which echoed the inner array back unchanged, producing[[]]instead of an error. Non-object sub-requests are now rejected with-32600 Invalid Requestbefore dispatch.Before:
After (Ethereum-compatible):
4. Apply
StreamReadConstraintsto the JSON-RPC parserJsonRpcServletused a barenew ObjectMapper(), so thenode.http.maxNestingDepthandnode.http.maxTokenCountsettings introduced in #6701 had no effect on JSON-RPC requests. The mapper is now built with aJsonFactorythat reads those two limits fromCommonParameter, consistent with howJSON.javaconstructs its mapper.Without this fix, a ~4 MB request body such as
[0,0,...,0](~2 M tokens) could bypass the configuredmaxTokenCount(default 100,000) and force Jackson to allocate ~2 MJsonNodeobjects, causing GC pressure (Eden/Survivor saturation → premature promotion → Full GC). With the fix, the parser throws after the configured token limit is reached, well before the 4 MB body-size cap.Why are these changes required?
node.http.maxNestingDepth/node.http.maxTokenCountconfig knobs actually effective for the JSON-RPC endpoint, closing a token-density amplification vector.This PR has been tested by
Extra details
Issue 4 shares the same root cause identified in the review comment on this PR: the
MAPPERinJsonRpcServletwas not wired to theStreamReadConstraintsconfigured vianode.http.*.