Skip to content

refactor(http): improve JsonFormat parser robustness#6845

Merged
lvs0075 merged 1 commit into
tronprotocol:release_v4.8.2from
halibobo1205:fix/jsonformat-parser-robustness
Jun 17, 2026
Merged

refactor(http): improve JsonFormat parser robustness#6845
lvs0075 merged 1 commit into
tronprotocol:release_v4.8.2from
halibobo1205:fix/jsonformat-parser-robustness

Conversation

@halibobo1205

@halibobo1205 halibobo1205 commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Improves JsonFormat parsing robustness for protobuf JSON used by HTTP wallet APIs.

  • Reworks field parsing so comma-separated fields are handled iteratively instead of recursive self-calls.
  • Adds nesting-depth checks when parsing object/array structures in JsonFormat.
  • Adds unit coverage for preserved parser behavior, changed parser behavior, trailing comma handling, nesting limits, and large flat JSON bodies.

Why are these changes required?

The previous parser could consume an excessive call stack when processing very field-heavy or deeply nested JSON input. This PR makes the parser fail predictably with ParseException for overly deep input, while keeping normal JSON/protobuf parsing behavior compatible.

This PR has been tested by:

  • Unit Tests
    • ./gradlew :framework:test --tests org.tron.core.services.http.JsonFormatTest --tests org.tron.core.services.http.TriggerConstantContractServletTest :framework:checkstyleTest -x generateGitProperties
    • ./gradlew lint
  • Manual Testing
    • Verified compatibility-sensitive parser behavior before and after the change:
      • Existing unknown-field skipping remains supported.
      • Known repeated fields still reject unsupported nested arrays.
      • Unknown nested object/array fields still reject trailing commas.
      • Valid nesting within Constant.MAX_NESTING_DEPTH remains accepted.

Follow up

None required.

Extra details

Compatibility notes:

  • Direct JsonFormat.merge() callers now reject protobuf JSON nesting beyond Constant.MAX_NESTING_DEPTH with ParseException: Hit recursion limit..

  • HTTP API compatibility risk is low because the normal HTTP JSON path already applies Jackson's MAX_NESTING_DEPTH = 100.

  • Deeply nested invalid input now returns a parser error instead of surfacing as a container-level HTTP 500.

    • JSON-RPC create-contract ABI parsing keeps the previous deeply nested ABI behavior: it still returns invalid params with invalid abi. Other ABI inputs that fail JsonFormat parsing may now also be reported as invalid params instead of internal error; this only affects invalid ABI input.
  • Compatibility notes for trailing commas:

Scenario Example Before this PR After this PR Notes
Top-level object trailing comma {"address":"61646472657373",} Rejected with ParseException Accepted Behavior is relaxed. This applies to top-level fields after the caller loop consumes the trailing comma.
Known nested message trailing comma {"genesisBlockId":{"hash":"00","number":1,}} Rejected with ParseException Accepted Behavior is relaxed for known protobuf message fields.
Known repeated field trailing comma {"approvals":["00","01",]} Accepted Accepted No behavior change; repeated-field array parsing already tolerated a single trailing comma.
Unknown nested object trailing comma {"zzz":{"a":1,}} Rejected with ParseException Rejected with ParseException No behavior change. Unknown fields are only skipped and are not used by business logic.
Unknown nested array trailing comma {"zzz":[1,]} Rejected with ParseException Rejected with ParseException No behavior change.
Repeated commas {"address":"61646472657373",,} Rejected with ParseException Rejected with ParseException No behavior change; only a single trailing comma may be accepted on known/top-level paths.

This is an input-acceptance relaxation for top-level fields and known nested protobuf message fields. It is aligned with the existing org.tron.json.JSON behavior, which already allows a single trailing comma while rejecting repeated commas.

@github-actions github-actions Bot requested review from Sunny6889 and bladehan1 June 16, 2026 07:53
@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.8.2 milestone Jun 16, 2026
@halibobo1205 halibobo1205 added the topic:api rpc/http related issue label Jun 16, 2026
Improve the hand-rolled JsonFormat parser used by HTTP wallet APIs so field-heavy and deeply nested JSON no longer overflows the request thread stack.

mergeField now consumes one field per call and leaves comma iteration to the caller. JsonFormat also enforces Constant.MAX_NESTING_DEPTH for object/array descent and returns ParseException instead of allowing StackOverflowError to escape.

Compatibility notes: direct JsonFormat.merge callers now reject nesting beyond 100 levels; trailing commas on known-field parse paths (top-level fields, known message fields, and known repeated fields) may be accepted, while trailing commas inside unknown nested object/array fields remain rejected; malicious deep-nesting requests now surface as parse errors instead of container-level HTTP 500s.
@halibobo1205 halibobo1205 force-pushed the fix/jsonformat-parser-robustness branch from 529bb2f to 554898b Compare June 16, 2026 08:17

@bladehan1 bladehan1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings: 1 [DISCUSS] (trailing-comma behavior — appears intentional, aligns with the fastjson-compat JSON shim) + 1 [NIT] (boundary test + cosmetic loop shape).

Comment thread framework/src/main/java/org/tron/core/services/http/JsonFormat.java
Comment thread framework/src/main/java/org/tron/core/services/http/JsonFormat.java
@lvs0075 lvs0075 merged commit 0871079 into tronprotocol:release_v4.8.2 Jun 17, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:api rpc/http related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants