fix: apply strictNumbers when coercing strings to numbers - #2661
Open
amantu-qbit wants to merge 1 commit into
Open
fix: apply strictNumbers when coercing strings to numbers#2661amantu-qbit wants to merge 1 commit into
amantu-qbit wants to merge 1 commit into
Conversation
Type coercion assigned the result of `+data` without repeating the finiteness check that `checkDataType` applies, so with `coerceTypes` enabled the string "Infinity" validated as both `number` and `integer` even under the default `strictNumbers: true`. For `integer` the value also slipped past the fractional-part test, because `Infinity % 1` is NaN and `!(NaN)` is true. "NaN" was rejected only incidentally, by `data == +data` being false, which is why the two behaved differently. The same applies to any string that overflows to Infinity, so `1e999` was accepted as an integer and reached the caller as a non-finite number. Gate the coercion on `isFinite` when `strictNumbers` is set, matching what `checkDataType` already does for values that arrive as numbers. With `strictNumbers: false` the previous behaviour is unchanged.
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
With
coerceTypesenabled, the string"Infinity"validates as bothnumberandintegerunder the defaultstrictNumbers: true.coerceDataassigns the result of+datawithout repeating the finiteness check thatcheckDataTypeapplies, sostrictNumbersis only enforced on the branch for values that are already numbers.For
integerthe value also slips past the fractional-part test, becauseInfinity % 1isNaNand!(NaN)istrue."NaN"was rejected only incidentally —data == +datais false for it — which is why the two behave differently today.The same applies to any string that overflows, so
1e999is accepted as an integer and reaches the caller as a non-finite number. That's the realistic trigger: it's an ordinary-looking number to a client.How
Gate the string coercion on
isFinitewhenstrictNumbersis set, matchingcheckDataType. WithstrictNumbers: false, previous behaviour is unchanged.Tests
Three cases in
spec/coercion.spec.ts: non-finite strings rejected by default for bothnumberandinteger, finite strings (including1e300) still coercing, andstrictNumbers: falsestill coercing to Infinity. Full suite: 7615 passing, 0 failing.Context
This was traced from fastify/fastify#6718, where
?num=Infinityand?num=1e999passtype: integervalidation and reach the handler as non-finite numbers. The Fastify maintainers closed that issue as belonging here.