Skip to content

fix(http): bound JsonFormat integer and enum token parsing - #4

Open
waynercheung wants to merge 1 commit into
developfrom
fix/jsonformat-bigint
Open

fix(http): bound JsonFormat integer and enum token parsing#4
waynercheung wants to merge 1 commit into
developfrom
fix/jsonformat-bigint

Conversation

@waynercheung

Copy link
Copy Markdown
Owner

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 to framework/src/main/java/org/tron/core/services/http/JsonFormat.java.

  1. Integer token limit. parseInteger rejects tokens longer than 256 characters as its first operation, before any substring or BigInteger construction. The limit applies to the raw token, so it includes the sign and the radix prefix.
  2. Enum identifier limit. Enum identifier tokens longer than 256 characters (quotes included) are rejected before case normalization and descriptor lookup.
  3. Fixed range messages. Integer out-of-range and unsigned-negative errors now use fixed text and no longer append the input token.
  4. Bounded error detail. The JDK detail retained for integer syntax errors, and the identifier echoed in enum lookup errors, are both truncated. Truncation is UTF-16 surrogate safe.
  5. Float and double errors. 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?

JsonFormat had 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_LENGTH bounds the work performed by parseInteger after tokenization. It rejects oversized tokens before substring, Long.parseLong, or BigInteger parsing. It does not bound request reading, tokenization, generic string and bytes fields, or protocol-wide response handling.
  • MAX_ENUM_TOKEN_LENGTH is 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):

    File New Covers
    JsonFormatIntegerTokenTest 16 raw limit at and over the boundary, radix prefixes and inner signs, exact int64 MIN/MAX and off-by-one, decimal/octal/hex range errors, both parser branches on trailing garbage, Unicode leading zeros, enum and unknown-field routing
    JsonFormatErrorBoundaryTest 5 oversized enum identifier, enum limit including quotes, unknown field names still ignored, large string and bytes fields still accepted
    DeployContractServletTest 2 servlet-level bounded-response tests using mock request and response objects, for an oversized ABI integer and an oversized ABI enum identifier
    JsonFormatTest 2 float error carries no input, truncation does not split a surrogate pair

    Those 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. checkstyleMain and checkstyleTest pass. git diff develop...HEAD --check passes. The full :framework:test suite was not run locally and is left to CI.

  • Manual Testing

    Behavioral differential against the develop baseline. The same probe source was compiled separately against develop's JsonFormat and 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 by DeployContractServlet for call_token_value and token_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 / parseUInt32 unsigned conversion semantics and sign validation. In particular parseUInt64 does not currently accept the upper half of the uint64 range: bigValue.longValueExact() raises an ArithmeticException there rather than the NumberFormatException the callers handle. That behavior predates this PR and is unchanged by it.
  • A unified error model for HTTP, JSON-RPC and gRPC responses and logs (stable error codes, fixed short text, request id). The changes here are local to one parser and do not make that guarantee protocol-wide.
  • Backport. The patch applies to master and release_v4.8.2 without 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:

Integer tokens longer than 256 characters are now rejected before numeric parsing. The limit includes signs and radix prefixes, and therefore also rejects overlong zero-padded values that earlier versions could parse successfully. Accepted integer values whose tokens are at most 256 characters retain their existing values and exception types.

Enum identifier tokens are limited to 256 characters including quotes. Integer range, unsigned-negative, enum lookup, and floating-point parse error messages have changed and no longer include arbitrary-length input. Large string and bytes fields, as well as ignored unknown field names, are not subject to these integer and enum limits. The limits are fixed parser contracts and are not runtime-configurable.

Headroom for the 256-character limit. parseInteger serves 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 of framework, common, actuator, chainbase and protocol sources (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 integer still matches; callers parsing the full message, the trailing colon, or the echoed input suffix do not.

@waynercheung waynercheung changed the title fix(http): bound JsonFormat integer and enum token parsing fix(http): bound JsonFormat integer and enum token parsing Aug 28, 2026
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
waynercheung force-pushed the fix/jsonformat-bigint branch from 5fa5d56 to 2433198 Compare August 28, 2026 14:46
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.

2 participants