fix(core): parse JSON numbers in exponent notation#8225
Closed
MohMaya wants to merge 2 commits into
Closed
Conversation
4 tasks
Author
|
Duplicate of PR #8226 |
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.
Issue
Fixes #8224
Description
jsonReviverthrows on any JSON number written in exponent notation, e.g.1.784316439424E9:Exponent notation is valid JSON, and AWS services emit it — ECS serializes epoch-second timestamps that way, so every
ecs:DescribeServicesresponse currently fails to deserialize.The reviver decides a token is precision-losing by comparing the raw source text against
String(value):An exponent-notation token always fails that string comparison (
"1.784316439424E9" !== "1784316439.424"), so it is treated as precision-losing even when thedoubleround-trips exactly. Both branches then reject it:NumericValue, whose format is/^-?\d*(\.\d+)?$/and forbidsEBigInt("1E9")throwsSyntaxError: Cannot convert 1E9 to a BigIntThis change normalizes exponent notation to plain decimal before the classification, so the existing logic sees a form it understands. Values that round-trip (the common case, including ECS timestamps) now fall through to a plain
number; values that genuinely exceeddoubleprecision are still preserved asBigInt/NumericValue.Expansion is bounded by
MAX_EXPANDED_DIGITS(1024) so an extreme exponent cannot cause a large string allocation; past that bound the reviver returns the parsed double rather than throwing.Behavior notes for reviewers
Two consequences worth a look, both currently exceptions rather than values:
1e1000/1e-1000wereSyntaxError/NumericValuethrows; they are now preserved exactly (BigIntandNumericValuerespectively) instead of collapsing toInfinity/0. This follows the reviver's stated purpose, but it is a behavior choice — happy to return the double instead if you'd prefer the narrower change.MAX_EXPANDED_DIGITSthe parsed double is returned (Infinity/0), matching no-reviver behavior. The threshold is arbitrary; say the word if you want a different bound.I did not widen
NumericValue's format regex insmithy-typescript, since fixing the classification here keeps the change in one repo. If you'd rather accept the full JSON number grammar there, that would also solve it and I'm glad to follow up.Testing
Added cases to
jsonReviver.spec.ts, gated oncontextSourceAvailablelike the existing tests:1.784316439424E9, integral1E9, lowercase1e9, negative-2.5E3, and fractional1.23E11.5E-10→NumericValue("0.00000000015"),1.2345678901234567890E30→BigIntThe existing tests in that file (small ints/decimals, 37-digit
BigInt, 41-digitbigDecimal) are unchanged and still pass — those inputs contain no exponent, so they take the original path untouched.Verified end to end by patching the compiled fix into
@aws-sdk/core@3.977.0in a local install:describeServicesgoes from the error above to returningcreatedAtas aDate, both against a stubbed response and against the live ECS API.tsc --strict --noEmitis clean on the changed file.Additional context
Regression range:
@aws-sdk/core@3.976.0is fine,3.977.0fails. Suspect #8214, which enabled the reviver more broadly vianeedsReviver/parseJsonBody.botocoreparses these identical bodies without complaint, so the divergence is specific to this reviver.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.