fix(http): bound JsonFormat integer and enum token parsing - #4
Open
waynercheung wants to merge 1 commit into
Open
fix(http): bound JsonFormat integer and enum token parsing#4waynercheung wants to merge 1 commit into
waynercheung wants to merge 1 commit into
Conversation
Reject integer tokens longer than 256 characters as the first operation in parseInteger, before substring creation and BigInteger parsing. Pass the original token to BigInteger without normalization and retain the existing exact range checks. Reject enum identifier tokens longer than 256 characters, quotes included, before case normalization and descriptor lookup. Abbreviate identifiers echoed in enum errors to 64 characters, without splitting a UTF-16 surrogate pair. Use fixed messages for integer range errors and unsigned negative values, without including the input token. Bound the JDK detail retained for integer syntax errors. Replace float and double parse error details with a message containing only the token length. Leave string and bytes fields, and ignored unknown field names, outside these limits. Add parser and servlet tests covering integer token limits, both integer parser branches, exact signed int64 boundaries, enum identifiers, bounded error messages, and existing string, bytes and unknown-field handling. BREAKING CHANGE: integer tokens longer than 256 characters are now rejected, including zero-padded values that previously parsed successfully. Enum identifier tokens are limited to 256 characters. Integer range, unsigned-negative, enum lookup, and floating-point parse error messages have changed.
waynercheung
force-pushed
the
fix/jsonformat-bigint
branch
from
August 28, 2026 14:46
5fa5d56 to
2433198
Compare
halibobo1205
approved these changes
Sep 3, 2026
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 fixed parser limits and bounded error messages to
JsonFormat, the parser behind the HTTP API's JSON-to-protobuf conversion. Production changes are confined toframework/src/main/java/org/tron/core/services/http/JsonFormat.java.parseIntegerrejects tokens longer than 256 characters as its first operation, before anysubstringorBigIntegerconstruction. The limit applies to the raw token, so it includes the sign and the radix prefix.Couldn't parse number:now reports only the token length instead of forwarding the JDK message. No length limit is introduced for float and double tokens.For integer and enum tokens within the new limits, accepted values, exception types and parse results are unchanged. String and bytes fields, and ignored unknown field names, are explicitly not subject to these limits.
Why are these changes required?
JsonFormathad no explicit length contract for integer and enum identifier tokens, and several of its parse errors copied the caller-supplied token into the message. Both the processed token length and the size of the resulting error text were therefore decided by the request rather than by the parser. This PR makes both explicit: a fixed limit for each of the two token kinds, and bounded-size error text.The two limits have different roles, and the code comments state this so the distinction is not lost later:
MAX_INTEGER_TOKEN_LENGTHbounds the work performed byparseIntegerafter tokenization. It rejects oversized tokens beforesubstring,Long.parseLong, orBigIntegerparsing. It does not bound request reading, tokenization, generic string and bytes fields, or protocol-wide response handling.MAX_ENUM_TOKEN_LENGTHis an input contract and an early-failure check. Enum error text is kept short independently by identifier truncation, so removing this limit would not change how large those messages can become.Both are fixed parser contracts and deliberately not runtime-configurable, so that the HTTP API presents the same input contract across node deployments. This parser is not part of consensus.
This PR has been tested by:
Unit Tests
25 new targeted tests (JDK 17, all passing):
JsonFormatIntegerTokenTestint64MIN/MAX and off-by-one, decimal/octal/hex range errors, both parser branches on trailing garbage, Unicode leading zeros, enum and unknown-field routingJsonFormatErrorBoundaryTestDeployContractServletTestJsonFormatTestThose four files contain 54 tests in total; all pass.
Package
org.tron.core.services.http, using only branch-tracked test sources: 81 test classes, 269 tests, 0 failures, 0 errors, 0 skipped.checkstyleMainandcheckstyleTestpass.git diff develop...HEAD --checkpasses. The full:framework:testsuite was not run locally and is left to CI.Manual Testing
Behavioral differential against the
developbaseline. The same probe source was compiled separately againstdevelop'sJsonFormatand this branch's, then run over 32 token shapes (radix prefixes, inner signs, leading zeros, boundary values, Unicode digits, trailing garbage). Compiling two revisions, rather than embedding a copy of the algorithm in a test, avoids the copy-drift problem a self-contained differential has.Result: 20 identical, 10 message-only changes, and 2 outcome changes. Both outcome changes are tokens whose raw length exceeds 256 because of zero padding, which is exactly the documented compatibility change.
Follow up
Deliberately out of scope here, tracked separately:
Util.getJsonLongValue/TypeUtils.castToBigDecimal, used byDeployContractServletforcall_token_valueandtoken_id, is a different conversion path with its own existing length limit. The servlet tests added here carry a comment stating they scope only the ABI parse path.parseUInt64/parseUInt32unsigned conversion semantics and sign validation. In particularparseUInt64does not currently accept the upper half of theuint64range:bigValue.longValueExact()raises anArithmeticExceptionthere rather than theNumberFormatExceptionthe callers handle. That behavior predates this PR and is unchanged by it.masterandrelease_v4.8.2without rejects. That is an apply check only; each target branch still needs its own compile and full test run before a backport PR is opened.Extra details
Compatibility. This text should also be carried into the release note, since the repository has no CHANGELOG file:
Headroom for the 256-character limit.
parseIntegerserves protobuf 32-bit and 64-bit integer fields. Under the decimal, octal and hexadecimal syntax it accepts, a canonical token for those types runs to at most a few tens of characters, so a 256-character limit leaves a wide compatibility margin. A scan offramework,common,actuator,chainbaseandprotocolsources (main and test,*.java*.json*.conf) found no numeric-looking token longer than 88 characters. That is repository evidence, not production client telemetry.Error-text compatibility. Callers that relied on the previous message text are affected. The type-semantic prefix was preserved where possible, so a caller matching on
Number out of range for 64-bit signed integerstill matches; callers parsing the full message, the trailing colon, or the echoed input suffix do not.