refactor(framework): replace fastjson with jackson#119
refactor(framework): replace fastjson with jackson#119halibobo1205 wants to merge 1 commit intodevelopfrom
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
framework/src/test/java/org/tron/core/services/http/GetAccountResourceServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/GetAccountResourceServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/VoteWitnessAccountServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/UpdateWitnessServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/ProposalApproveServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/main/java/org/tron/common/logsfilter/EventPluginLoader.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/GetAccountServletTest.java
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/GetAccountServletTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb9652c04b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
CodeAnt AI finished reviewing your PR. |
- EventPluginLoader: replace standalone ObjectMapper with JSON.MAPPER
to preserve NON_NULL and other Fastjson-compatible settings for event
payloads sent to PF4J plugins and native queue subscribers
- JSON.MAPPER: enable ALLOW_TRAILING_COMMA for Fastjson-style tolerance
of trailing commas in request bodies (e.g. {"owner_address":"...",})
- GetAccountResourceServletTest: strengthen assertions to verify
freeNetUsed field presence instead of just non-empty response
- GetAccountServletTest: add response-content assertions for error
and address field validation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@CodeAnt-AI: review |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
@codex review |
Sequence DiagramThis PR replaces Fastjson with org.tron.json JSON/JSONObject/JSONArray wrappers backed by a shared Jackson ObjectMapper, preserving existing HTTP and JSON-RPC request/response contracts while centralizing parsing and serialization behavior. sequenceDiagram
participant Client
participant HttpServlet
participant JsonWrapper
participant ObjectMapper
participant WalletBackend
Client->>HttpServlet: Send JSON request body
HttpServlet->>JsonWrapper: parseObject / parseArray(text)
JsonWrapper->>ObjectMapper: Read JSON tree with Fastjson-like settings
ObjectMapper-->>JsonWrapper: Parsed JSON node
JsonWrapper-->>HttpServlet: JSONObject / JSONArray view
HttpServlet->>WalletBackend: Invoke wallet or query operation
WalletBackend-->>HttpServlet: Protobuf result
HttpServlet->>JsonWrapper: toJSONString(result, prettyFlag)
JsonWrapper->>ObjectMapper: Write JSON string (omit null fields)
ObjectMapper-->>JsonWrapper: Serialized JSON text
JsonWrapper-->>HttpServlet: Response JSON
HttpServlet-->>Client: HTTP response with JSON body
Generated by CodeAnt AI |
framework/src/test/java/org/tron/core/services/http/UpdateWitnessServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/MarketCancelOrderServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/VoteWitnessAccountServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/TransferAssetServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/UnFreezeBalanceV2ServletTest.java
Outdated
Show resolved
Hide resolved
| .getOwnerAddress(), ownerAddr) | ||
| && addressEquals(((AssetIssueContractOuterClass.ParticipateAssetIssueContract) c) | ||
| .getToAddress(), toAddr) | ||
| && ((AssetIssueContractOuterClass.ParticipateAssetIssueContract) c) |
There was a problem hiding this comment.
Suggestion: The verification omits asset_name, so this test can still pass even if JSON-to-contract mapping breaks for that field during the Fastjson→Jackson migration. Add an explicit assertion on the parsed contract asset_name to ensure request-to-service field mapping is fully validated. [logic error]
Severity Level: Major ⚠️
- ⚠️ ParticipateAssetIssueServletTest ignores asset_name in contract verification.
- ⚠️ Asset participation servlet asset_name regressions may bypass unit tests.| && ((AssetIssueContractOuterClass.ParticipateAssetIssueContract) c) | |
| && ByteArray.toHexString(((AssetIssueContractOuterClass.ParticipateAssetIssueContract) c) | |
| .getAssetName().toByteArray()).equals("74657374") |
Steps of Reproduction ✅
1. Open `ParticipateAssetIssueServletTest` at
`framework/src/test/java/org/tron/core/services/http/ParticipateAssetIssueServletTest.java:34-41`;
the test `testParticipateAssetIssue` builds a JSON body containing `"owner_address"`,
`"to_address"`, `"asset_name": "74657374"`, and `"amount": 100`.
2. In the same test file, see `setUpMocks()` at lines 24-32 where
`ParticipateAssetIssueServlet` is instantiated and `wallet.createTransactionCapsule(...)`
is stubbed to return a `TransactionCapsule` for any `ParticipateAssetIssueContract` (no
constraints on `asset_name` at stubbing time).
3. Follow the production path in `ParticipateAssetIssueServlet.doPost` at
`framework/src/main/java/org/tron/core/services/http/ParticipateAssetIssueServlet.java:10-20`,
where the request JSON string is merged into a `ParticipateAssetIssueContract.Builder` via
`JsonFormat.merge(contract, build, visible)` and the resulting contract is passed to
`wallet.createTransactionCapsule(build.build(),
ContractType.ParticipateAssetIssueContract)`.
4. Back in the test, inspect the Mockito verification at lines 46-53:
`verify(wallet).createTransactionCapsule(argThat(...),
eq(ContractType.ParticipateAssetIssueContract));` where the `argThat` predicate checks
only `owner_address`, `to_address`, and `amount` but never calls `getAssetName()` on
`ParticipateAssetIssueContract`. This means any mismatch between the input `"asset_name"`
JSON field and the parsed `asset_name` contract field will not cause the test to fail, so
request-to-contract mapping regressions for `asset_name` can pass CI unnoticed.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** framework/src/test/java/org/tron/core/services/http/ParticipateAssetIssueServletTest.java
**Line:** 52:52
**Comment:**
*Logic Error: The verification omits `asset_name`, so this test can still pass even if JSON-to-contract mapping breaks for that field during the Fastjson→Jackson migration. Add an explicit assertion on the parsed contract `asset_name` to ensure request-to-service field mapping is fully validated.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
framework/src/test/java/org/tron/core/services/http/FreezeBalanceV2ServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/DelegateResourceServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/UnDelegateResourceServletTest.java
Outdated
Show resolved
Hide resolved
| argThat(c -> c instanceof MarketContract.MarketSellAssetContract | ||
| && addressEquals(((MarketContract.MarketSellAssetContract) c) | ||
| .getOwnerAddress(), ownerAddr) | ||
| && ((MarketContract.MarketSellAssetContract) c).getSellTokenQuantity() == 100 |
There was a problem hiding this comment.
Suggestion: The verification predicate does not assert sell_token_id and buy_token_id, so this test can still pass even if the servlet maps token IDs incorrectly (or swaps them) while only preserving quantities. Extend the predicate to validate both token ID fields so request-to-contract mapping regressions are actually detected. [logic error]
Severity Level: Major ⚠️
- ⚠️ MarketSellAssetServletTest misses swapped token ID regressions.
- ⚠️ JSON-to-MarketSellAssetContract mapping issues may go unnoticed.
- ⚠️ Market sell asset HTTP API could mis-handle token IDs.| && ((MarketContract.MarketSellAssetContract) c).getSellTokenQuantity() == 100 | |
| && ByteArray.toHexString(((MarketContract.MarketSellAssetContract) c) | |
| .getSellTokenId().toByteArray()).equals("5f") | |
| && ((MarketContract.MarketSellAssetContract) c).getSellTokenQuantity() == 100 | |
| && ByteArray.toHexString(((MarketContract.MarketSellAssetContract) c) | |
| .getBuyTokenId().toByteArray()).equals("60") |
Steps of Reproduction ✅
1. Open
`framework/src/test/java/org/tron/core/services/http/MarketSellAssetServletTest.java` and
locate `testMarketSellAsset` at lines 33–54, which exercises
`MarketSellAssetServlet.doPost`.
2. Observe that `MarketSellAssetServlet.doPost` in
`framework/src/main/java/org/tron/core/services/http/MarketSellAssetServlet.java:26–38`
parses the JSON body into a `MarketSellAssetContract` builder via
`JsonFormat.merge(contract, build, visible)` and then calls
`wallet.createTransactionCapsule(build.build(), ContractType.MarketSellAssetContract)`.
3. In the test's `verify(wallet).createTransactionCapsule(...)` call at lines 47–51,
inspect the `argThat` predicate: it asserts only that the argument is a
`MarketSellAssetContract`, the `owner_address` matches `ownerAddr`, and
`sell_token_quantity` and `buy_token_quantity` equal 100 and 200 respectively; it never
inspects `sell_token_id` or `buy_token_id`.
4. From this code, it follows that any `MarketSellAssetContract` instance with correct
owner address and quantities will satisfy the predicate, regardless of how `sell_token_id`
and `buy_token_id` were populated by `JsonFormat.merge`; thus, a regression that swaps or
mis-encodes token IDs in `MarketSellAssetServlet.doPost` would not cause this test to
fail, meaning request-to-contract token ID mapping regressions are not detected by this
test.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** framework/src/test/java/org/tron/core/services/http/MarketSellAssetServletTest.java
**Line:** 50:50
**Comment:**
*Logic Error: The verification predicate does not assert `sell_token_id` and `buy_token_id`, so this test can still pass even if the servlet maps token IDs incorrectly (or swaps them) while only preserving quantities. Extend the predicate to validate both token ID fields so request-to-contract mapping regressions are actually detected.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc3e80e7c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address PR #119 new review comments from CodeAnt AI: - UpdateWitnessServletTest: validate update_url field - MarketCancelOrderServletTest: validate order_id (32 bytes) - VoteWitnessAccountServletTest: validate vote_address in votes - TransferAssetServletTest: validate asset_name - ParticipateAssetIssueServletTest: validate asset_name Note: resource enum field (BANDWIDTH) not asserted for FreezeBalanceV2/UnFreezeBalanceV2/DelegateResource/UnDelegateResource because JsonFormat.merge() does not parse the resource field — BANDWIDTH is the protobuf default (0), so asserting it would not prove parsing correctness. This is a pre-existing base branch behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@CodeAnt-AI : review |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Sequence DiagramThis PR replaces Fastjson with Jackson-backed JSON wrappers and adds servlet tests to verify that HTTP APIs still parse request bodies and return transaction JSON consistently while delegating business logic to the wallet. sequenceDiagram
participant Client
participant HttpAPI
participant JsonWrapper
participant JacksonMapper
participant Wallet
Client->>HttpAPI: POST transaction request (JSON)
HttpAPI->>JsonWrapper: parseObject(request body)
JsonWrapper->>JacksonMapper: readTree / readValue
JacksonMapper-->>JsonWrapper: Parsed object node
HttpAPI->>Wallet: createTransactionCapsule(parsed contract)
Wallet-->>HttpAPI: TransactionCapsule (protobuf transaction)
HttpAPI->>JsonWrapper: toJSONString(response payload, pretty)
JsonWrapper->>JacksonMapper: writeValueAsString(object)
JacksonMapper-->>JsonWrapper: Serialized JSON string
HttpAPI-->>Client: 200 OK (JSON with txID and raw_data)
Generated by CodeAnt AI |
framework/src/test/java/org/tron/core/services/http/UpdateAssetServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/ExchangeCreateServletTest.java
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/ExchangeWithdrawServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/DeployContractServletTest.java
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/ExchangeInjectServletTest.java
Outdated
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/ExchangeTransactionServletTest.java
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/services/http/FreezeBalanceV2ServletTest.java
Outdated
Show resolved
Hide resolved
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
@CodeAnt-AI: review |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
@CodeAnt-AI: review |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Sequence DiagramThis PR replaces Fastjson with Jackson-backed JSON wrappers while preserving the existing HTTP API flow: servlets parse JSON requests, build protobuf transactions via the wallet, and return JSON responses. sequenceDiagram
participant Client
participant HttpServlet
participant JSONWrapper
participant Wallet
participant Node
Client->>HttpServlet: POST transaction request (JSON body)
HttpServlet->>JSONWrapper: Parse body to JSONObject and read fields
HttpServlet->>Wallet: Create protobuf contract and transaction
Wallet->>Node: Validate and build transaction capsule
Wallet-->>HttpServlet: Built transaction
HttpServlet->>JSONWrapper: Serialize transaction to JSON string
HttpServlet-->>Client: 200 OK with transaction JSON response
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
1bdc631 to
665adfa
Compare
Remove the Fastjson dependency entirely and replace it with Jackson-backed drop-in wrappers (JSON, JSONObject, JSONArray, JSONException) that preserve the same public API surface. Motivation: - Fastjson has a history of critical CVEs and is no longer actively maintained for 1.x - Jackson-databind 2.18.6 addresses CVE GHSA-72hv-8253-57qq Core changes (common module): - Add org.tron.json.{JSON, JSONObject, JSONArray, JSONException} wrappers backed by a shared Jackson ObjectMapper configured to match Fastjson 1.x parsing/serialization defaults: - Unquoted field names and single-quoted strings (lenient parsing) - BigDecimal for floats, case-insensitive property matching - Null fields omitted (matches Fastjson default) - Type-safe accessors: getBoolean/getLong/getDouble/getIntValue/ getLongValue/getBigDecimal throw JSONException on invalid text instead of silently returning 0/false - parseObject(String) guards against ClassCastException on non-object JSON roots; parseArray handles whitespace-only input - parseObject(String, Class) delegates to parseObject/parseArray for wrapper types to avoid silent field loss via ObjectMapper - Upgrade jackson-databind 2.18.3 → 2.18.6 HTTP servlet changes (framework module): - Swap import from com.alibaba.fastjson → org.tron.json across all HTTP API servlets, JSON-RPC layer, and event/log parsers - No changes to request/response JSON structure — existing API contracts are preserved Test changes: - Add BaseHttpTest base class managing Args lifecycle, Wallet mock, MINIMAL_TX constant, and request/response factory methods (postRequest, getRequest, newResponse) - 44 servlet test classes refactored to extend BaseHttpTest, eliminating ~1400 lines of duplicated boilerplate - Strengthen weak assertNotNull checks to content-based assertions: assertTrue(contains("raw_data")) for transaction servlets, assertTrue(contains("blockID")) for block queries, etc. - Add Mockito verify for wallet service calls in query servlets to catch request-to-service mapping regressions - Fix test environment: initialize Args from config-test.conf (maxMessageSize) and use MINIMAL_TX with raw_data to prevent NPE in Util.printCreateTransaction - Add JsonCompatibilityFuzzTest: 500-round fuzz covering round-trip serialization, BigDecimal/BigInteger precision, deep nesting, unicode, and boundary values - Use SecureRandom for fuzz test randomization Build: - Remove fastjson from common/build.gradle dependencies - Update gradle/verification-metadata.xml for jackson 2.18.6 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: codeant-ai[bot] <151821869+codeant-ai[bot]@users.noreply.github.com> Co-Authored-By: codex <codex@openai.com>
665adfa to
42cc80d
Compare
User description
Remove the Fastjson dependency entirely and replace it with Jackson-backed drop-in wrappers (JSON, JSONObject, JSONArray, JSONException) that preserve the same public API surface.
Motivation:
Core changes (common module):
HTTP servlet changes (framework module):
Test changes:
Build:
Follow up
Extra details
CodeAnt-AI Description
Switch JSON handling to the new Jackson-backed wrappers while keeping the same API shape
What Changed
Impact
✅ Safer JSON parsing✅ Clearer invalid input errors✅ Same API responses after JSON library swap💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.