feat(framework,actuator,common): replace fastjson with jackson#6701
feat(framework,actuator,common): replace fastjson with jackson#6701halibobo1205 wants to merge 1 commit intotronprotocol:developfrom
Conversation
Replace `com.alibaba:fastjson` with Jackson-backed drop-in
wrappers (`org.tron.json.{JSON, JSONObject, JSONArray, JSONException}`).
No external API changes — all HTTP and JSON-RPC responses remain identical.
Motivation:
- Fastjson 1.2.83 is EOL with 20+ CVEs including critical RCE
- Upgrade jackson-databind 2.18.3 → 2.18.6 (GHSA-72hv-8253-57qq)
- Unify JSON handling (previously split between Jackson and Fastjson)
Core changes (common):
- Add org.tron.json wrappers backed by a shared ObjectMapper
- Remove fastjson from common/build.gradle
HTTP & servlet changes (framework):
- Swap imports from com.alibaba.fastjson → org.tron.json across
all HTTP servlets, JSON-RPC layer, and event/log parsers
Test changes:
- Add BaseHttpTest base class for servlet test lifecycle
Build:
- Update jackson to 2.18.6
- Remove fastjson
close tronprotocol#6607
github-actions
Bot
requested review from
317787106,
lxcmyf,
xxo1shine and
yanghang8612
| * case-insensitive {@code "null"} literal — mirroring Fastjson's lenient | ||
| * treatment of these inputs as JSON {@code null}. | ||
| */ | ||
| static boolean isNullLiteral(String text) { |
There was a problem hiding this comment.
[MUST] Preserve unquoted NULL compatibility: {a:NULL} currently throws, but fastjson 1.2.83 accepted this and treated it as null. Since this PR claims drop-in behavior, this is a breaking compatibility regression for existing clients and should be fixed before merge.
|
|
||
| public String asJsonString(boolean formatted) { | ||
| return serializeFieldsOnly(this, formatted); | ||
| public String asJsonString() { |
There was a problem hiding this comment.
[SHOULD] Avoid breaking ProgramTrace.asJsonString(boolean) API: replacing it with a no-arg method introduces source/binary compatibility risk for external callers and removes compact-vs-pretty output control. Prefer keeping the original signature (or adding an overload) to preserve compatibility.
| // Fastjson tolerates trailing commas (e.g. {"a":1,}) by default | ||
| .enable(JsonReadFeature.ALLOW_TRAILING_COMMA) | ||
| // Fastjson accepts NaN/Infinity as valid tokens | ||
| .enable(JsonReadFeature.ALLOW_NON_NUMERIC_NUMBERS) |
There was a problem hiding this comment.
[MUST] Please remove this flag from the shared request parser.
This is reachable on current wallet HTTP paths, not just a theoretical Fastjson-parity issue. DeployContractServlet and TriggerSmartContractServlet read call_value, call_token_value, token_id, and fee_limit through Util.getJsonLongValue(...).
With this flag enabled, Infinity / -Infinity parse successfully, getBigDecimal(...) returns null, and those non-required fields become 0L instead of rejecting the request. Before this PR, Fastjson rejected those payloads during parse.
| parameter.put("type_url", contract.getParameterOrBuilder().getTypeUrl()); | ||
| JSONObject jsonContract = new JSONObject(); | ||
| jsonContract.put(PARAMETER, parameter); | ||
| jsonContract.put("type", contract.getType()); |
There was a problem hiding this comment.
[SHOULD] This line controls the "type" field in every HTTP transaction JSON payload, but the new tests do not pin the exact serialized value here.
UtilMockTest#testPrintTransactionToJSON() only asserts non-null, and most servlet tests only check "no Error" / key presence. Please add an exact-output assertion that "type" remains the expected string for at least one TransferContract and one TriggerSmartContract response.
|
|
||
| @Getter | ||
| @Setter | ||
| private JSONObject transaction; |
There was a problem hiding this comment.
[QUESTION] TransactionJson.transaction is serialized by jsonrpc4j's own ObjectMapper, not org.tron.json.JSON.MAPPER. @JsonValue on JSONObject.unwrap() should bridge them, but I couldn't find a test that pins the on-wire bytes.
BuildTransactionTest stops at the rebuilt ContractType; JsonrpcServiceTest doesn't cover this serialization path.
Could we add one golden-output assertion for a representative buildTransaction response (transfer + trigger contract)? That's the most direct check on the "responses remain identical" claim for the JSON-RPC layer.
|
|
||
| @Test | ||
| public void testGet() throws Exception { | ||
| MockHttpServletRequest request = |
There was a problem hiding this comment.
[NIT] git diff --check still reports trailing whitespace here.
What about:
MockHttpServletRequest request = getRequest("visible", "true", "value",
"TBxSocpujP6UGKV5ydXNVTDQz7fAgdmoaB");5 participants
Summary
Replace
com.alibaba:fastjsonwith Jackson-backed drop-in wrappers (org.tron.json.{JSON, JSONObject, JSONArray, JSONException}). No external API changes — all HTTP and JSON-RPC responses remain identical.Motivation
Core changes
(common):
(framework): HTTP & servlet changes
com.alibaba.fastjson→org.tron.jsonacross all HTTP servlets, JSON-RPC layer, and event/log parsersBuild:
close #6607