Repository navigation
feat(http): add node.allowNumericFieldAlias switch for JSON aliases - #11
Closed
waynercheung wants to merge 17 commits into
Closed
waynercheung wants to merge 17 commits into
waynercheung wants to merge 17 commits into
Conversation
waynercheung
force-pushed
the
feature/numeric_field_alias_switch
branch
from
October 10, 2026 15:28
3330739 to
91671c2
Compare
waynercheung
force-pushed
the
feature/numeric_field_alias_switch
branch
from
October 10, 2026 15:35
91671c2 to
e60a306
Compare
* fix(jsonrpc): handle null parameters consistently Validate nullable JSON-RPC arguments before they are dereferenced. Return stable protocol errors instead of jsonrpc4j's unhandled -32001 fallback, which exposes Java exception types. Validate eth_call's required transaction argument before its block selector. The double-null case now returns -32602 instead of -32600. Align eth_uninstallFilter lookup misses with geth: null, unknown and already removed IDs return false. Empty and non-hexadecimal strings also return false when no filter matches, without new format checks. Non-null lookup misses now return false instead of -32000. Use the value returned by remove to report whether a filter was removed and avoid the race between checking and removing the same entry. Remove the obsolete uninstall ItemNotFoundException declaration and error mapping. Java binaries remain compatible, but source callers catching that checked exception, or implementations still declaring it, may need adjustment when recompiled. Keep getFilterChanges and getFilterLogs on their existing lookup-miss errors, and preserve request-source precedence and filter isolation. Treat null block-detail flags as false and retain DTO defaults for explicitly null optional fields. Preserve filter wildcard semantics. Add binding, service, wire and VM execution regressions for null arguments, error contracts, filter removal, source isolation and concurrent uninstall results. * fix(jsonrpc): reject null required positional parameters Return invalid params for an explicit null filter ID in eth_uninstallFilter, eth_getFilterChanges and eth_getFilterLogs, and for a null fullTransactionObjects flag in eth_getBlockByHash and eth_getBlockByNumber. The block hash or selector is still validated first, and the flag is checked before any block lookup, so the result does not depend on whether the block exists. Unknown non-null filter IDs keep their existing results. This replaces the earlier null-as-false handling of the flag and the filter ID. go-ethereum v1.17.6 rejects null for these required arguments; earlier releases decoded it as a zero value. * refactor(jsonrpc): share the null check for required parameters Route the null checks for the ten required JSON-RPC parameter positions through one requireParam helper instead of repeating the same if/throw block at each site. The "invalid params" and "invalid filter request" messages are unchanged and are now both private constants in TronJsonRpcImpl. Remove the null check from the LogFilter constructor. In production code, LogFilter is only built through LogFilterWrapper from eth_getLogs and eth_newFilter, and both reject a null filter request before reaching it, so the constructor check and its unit test are dropped. Null filter requests remain covered at the service and wire level. Extract getBlockByHash(byte[]) and use it from ethGetBlockByHash and getBlockByJsonHash, so ethGetBlockByHash no longer inlines the block lookup. The validation order is unchanged. No behavior change. * test(jsonrpc): use the three-argument TronJsonRpcImpl constructor Adapt the tests added for null parameter handling to the constructor that now takes the Manager, and drop the removed setManager call. Tests built only from mocks pass a mocked Manager, and tests backed by the shared database pass dbManager, matching the existing tests.
…col#6985) Replace jsonrpc4j's unhandled -32001 fallback with a fixed -32603 response so unmapped exception types and raw messages no longer reach clients. Keep the documented -32001 contract for net_version and eth_chainId through explicit mappings with fixed message and data. Propagate VirtualMachineError, ThreadDeath, LinkageError, and TronError through the JSON-RPC boundary. Scan complete cause chains without allocating a visited set, detecting cycles without missing fatal causes. Classify Throwables escaping dispatch before logging or mapping them. Before rethrowing an Error, make a best-effort attempt to commit an empty HTTP 500 so the container does not render its default error page. Preserve the original error if cleanup fails, and leave an already committed response untouched. Route single requests through handleRequest because the servlet handle API catches Throwable. Catch RuntimeException, IOException, and Error at both dispatch boundaries, then propagate a classified fatal cause or return -32603 when a response is appropriate. This also covers non-fatal Errors from interceptor hooks. Discard partial dispatch output on failure. Recover each failed batch element without discarding earlier results or skipping later requests. Share internal-error construction across serialization, dispatch, and response-parsing failures. Count only the replacement when parsing fails, retaining existing overflow rules and the current single/batch handling of requests without an id. Log unmapped exceptions, chain-identity failures, and asynchronous log-query failures and interruptions at DEBUG with the Throwable, classifying fatal causes before logging the failures. Log batch request-serialization and response-parsing recoveries at DEBUG with the element index and the Throwable. Log the first escaped dispatch failure in each batch at ERROR with the Throwable; repeats use DEBUG with only the index and exception type. Restore interrupted status for asynchronous log queries. Reject Boolean, object, and array request IDs before dispatch with -32600 and id:null. Reject non-null scalar params with -32600, preserve valid IDs, and isolate invalid batch elements so their siblings run. Missing, null, array, and object params keep their existing semantics. Remove the obsolete request replay wrapper and HTTP status provider. Add resolver, servlet, embedded-Jetty, chain-identity, asynchronous failure, dispatch-contract, and request-envelope regression coverage. Exercise the real HTTP interceptor chain and observe commitment without performing it on the servlet's behalf. Test interceptor Errors with a real server and preserve the existing notification response-suppression rules with characterization tests. Add a package-private server injection seam for servlet tests without changing production initialization.
waynercheung
force-pushed
the
feature/numeric_field_alias_switch
branch
from
October 13, 2026 07:36
e60a306 to
995777f
Compare
waynercheung
force-pushed
the
feature/numeric_field_alias_switch
branch
from
October 13, 2026 07:49
995777f to
a975a3d
Compare
JsonFormat treats a single-digit JSON key as an alias for the protobuf field
with that number (e.g. {"2": ...} maps to field 2). Add a node-level switch,
node.allowNumericFieldAlias, to control this extension. It is loaded at startup
and defaults to true, so the default JSON parsing behaviour is unchanged.
Operators that do not want the extension can set it to false, after which a
digit key is no longer resolved to a field number and is handled by the
existing unknown-field rules instead.
The switch applies to JSON-to-protobuf parsing in the HTTP API and to embedded
ABI JSON parsing in JSON-RPC. Binary protobuf parsing in gRPC, broadcasthex and
P2P is unaffected.
Also move the GET request setup inside an outer try/finally so the existing
cleanup runs when that setup throws, preserving setup exception propagation and
the handler's existing exception handling.
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?
Adds a node-level switch,
node.allowNumericFieldAlias(defaulttrue), that controls whetherJsonFormattreats a single-digit JSON key as an alias for the protobuf field with that number (for example{"2": ...}maps to field 2). It is loaded at startup; a restart is required after changing it.The default (
true) keeps the current JSON parsing behaviour unchanged. Setting it tofalsedisables the extension: digit keys are no longer resolved to field numbers and fall through to the existing unknown-field handling.Examples (parsing into
AccountPermissionUpdateContract; field 2 isowner, and withinPermissionfield 4 isthreshold):true(default)false{"2":"2003"}owner.thresholdto 3owneris unset{"owner":{"4":3}}owner.thresholdto 3owner, ignores"4";thresholdremains 0{"owner":{"threshold":3}}owner.thresholdto 3The switch applies to JSON-to-protobuf parsing in the HTTP API and to embedded ABI JSON parsing in JSON-RPC. Binary protobuf parsing in gRPC,
broadcasthexand P2P is unaffected, and it has no effect on consensus validation of already-constructed transactions.Why are these changes required?
Numeric field aliases are a java-tron extension to standard JSON. This adds a configuration option to disable the extension without a code change, while the default preserves full backward compatibility.
This PR has been tested by:
JsonFormatNumericAliasSwitchTest(8): default binding; explicit true/false binding viaNodeConfig.fromConfig; ON behaviour unchanged; OFF unknown-field handling; named field still parsed while an inner numeric alias is ignored.RateLimiterServletTest(14, including 2 added regressions).UtilTestpass;checkstyleMainpasses.Follow up
None.
Extra details
This branch also moves the per-request GET setup in
RateLimiterServletinside an outertry/finallyso the existing rate-limiter cleanup runs if that setup throws; setup exception propagation and the handler's existing exception handling are preserved.Backward-compatible by default:
truepreserves the existing JsonFormat parsing behaviour; operators opt out by settingnode.allowNumericFieldAliastofalse.