Skip to content

feat(http): add node.allowNumericFieldAlias switch for JSON aliases - #11

Closed
waynercheung wants to merge 17 commits into
release_v4.8.3from
feature/numeric_field_alias_switch
Closed

waynercheung wants to merge 17 commits into
release_v4.8.3from
feature/numeric_field_alias_switch

Conversation

@waynercheung

@waynercheung waynercheung commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

What does this PR do?

Adds a node-level switch, node.allowNumericFieldAlias (default true), that controls whether JsonFormat treats 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 to false disables 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 is owner, and within Permission field 4 is threshold):

Input true (default) false
{"2":"2003"} sets owner.threshold to 3 ignores the numeric key; owner is unset
{"owner":{"4":3}} sets owner.threshold to 3 parses owner, ignores "4"; threshold remains 0
{"owner":{"threshold":3}} sets owner.threshold to 3 same

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, 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:

  • Unit Tests
    • JsonFormatNumericAliasSwitchTest (8): default binding; explicit true/false binding via NodeConfig.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).
    • JsonFormat regression tests and UtilTest pass; checkstyleMain passes.
  • Manual Testing: not performed; behaviour is covered by the unit tests above.

Follow up

None.

Extra details

This branch also moves the per-request GET setup in RateLimiterServlet inside an outer try/finally so 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: true preserves the existing JsonFormat parsing behaviour; operators opt out by setting node.allowNumericFieldAlias to false.

@waynercheung
waynercheung force-pushed the feature/numeric_field_alias_switch branch from 3330739 to 91671c2 Compare October 10, 2026 15:28
@waynercheung waynercheung changed the title feat(http): add node.allowNumericFieldAlias switch for JSON field-number aliases feat(http): add node.allowNumericFieldAlias switch for JSON aliases Oct 10, 2026
@waynercheung
waynercheung force-pushed the feature/numeric_field_alias_switch branch from 91671c2 to e60a306 Compare October 10, 2026 15:35
Federico2014 and others added 12 commits October 11, 2026 21:08
* 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
waynercheung force-pushed the feature/numeric_field_alias_switch branch from e60a306 to 995777f Compare October 13, 2026 07:36
@waynercheung
waynercheung force-pushed the feature/numeric_field_alias_switch branch from 995777f to a975a3d Compare October 13, 2026 07:49
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants