Skip to content

Apply number-length validator on streaming integer path of async parser#1611

Merged
cowtowncoder merged 5 commits into
FasterXML:2.18from
tonghuaroot:fix/async-number-length-consistency
May 20, 2026
Merged

Apply number-length validator on streaming integer path of async parser#1611
cowtowncoder merged 5 commits into
FasterXML:2.18from
tonghuaroot:fix/async-number-length-consistency

Conversation

@tonghuaroot
Copy link
Copy Markdown
Contributor

Aligns the async non-blocking parser's integer-path NOT_AVAILABLE exits with the length-validation idiom already used by the sync paths and the async fraction path, so maxNumberLength is enforced consistently across sync and async parsers.

The change reuses the existing _setIntLength helper at each of the integer-path streaming-suspension points in NonBlockingUtf8JsonParserBase (_startPositiveNumber(int), _startNegativeNumber(), _startPositiveNumber() no-arg, _finishNumberIntegralPart()).

Tests

  • New AsyncNumberLengthConsistencyTest pins both the integer- and fraction-path streaming behavior under digit-only chunks.
  • Full mvn test: 1437 tests run, 0 failures, 0 errors, 2 skipped (pre-existing).

Happy to reword / split / amend per maintainer preference.

The non-blocking parser's NOT_AVAILABLE exits in the integer parsing
branch did not invoke the same length validator that the sync parser
and the async fraction path already use. This patch applies the
existing _setIntLength helper at those exits so maxNumberLength is
enforced consistently across sync and async parsers.

Signed-off-by: tonghuaroot <tonghuaroot@gmail.com>
@tonghuaroot tonghuaroot mentioned this pull request May 20, 2026
Closed
@cowtowncoder cowtowncoder changed the title async parser: apply number-length validator on streaming integer path Apply number-length validator on streaming integer path of async parser May 20, 2026
@cowtowncoder cowtowncoder added the cla-needed PR looks good (although may also require code review), but CLA needed from submitter label May 20, 2026
@cowtowncoder
Copy link
Copy Markdown
Member

@tonghuaroot Looks good so far.

One thing before I can merge: CLA from https://github.com/FasterXML/jackson/blob/main/CLA-jackson-2026.pdf (unless you have sent one already; only need to be sent once, good for all future prs).

The usual way is to print it, fill & sign, scan/photo, email to cla at fasterxml dot com.

@tonghuaroot
Copy link
Copy Markdown
Contributor Author

@cowtowncoder Signed CLA emailed to cla@fasterxml.com just now. Thanks!

@cowtowncoder cowtowncoder added cla-received PR already covered by CLA (optional label) and removed cla-needed PR looks good (although may also require code review), but CLA needed from submitter labels May 20, 2026
@cowtowncoder cowtowncoder added this to the 2.18.8 milestone May 20, 2026
@cowtowncoder cowtowncoder added 2.18 Issues planned at earliest for 2.18 2.21 Issues planned (at earliest) for 2.21 3.1 labels May 20, 2026
@cowtowncoder cowtowncoder merged commit 4cdd529 into FasterXML:2.18 May 20, 2026
6 checks passed
strategy:
fail-fast: false
matrix:
java_version: ['8', '11', '17', '21', '25']
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why drop java 11?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not an LTS.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Java 11 was an LTS, just as much as Java 8 was an LTS but both are no longer supported

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

@cowtowncoder cowtowncoder May 20, 2026

Choose a reason for hiding this comment

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

Oh. I stand corrected. Will put it back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.18 Issues planned at earliest for 2.18 2.21 Issues planned (at earliest) for 2.21 3.1 cla-received PR already covered by CLA (optional label)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants