fix(utils): guard adjustTime against non-finite deltas (freeze + NaN:NaN)#3584
Open
arham766 wants to merge 1 commit into
Open
fix(utils): guard adjustTime against non-finite deltas (freeze + NaN:NaN)#3584arham766 wants to merge 1 commit into
arham766 wants to merge 1 commit into
Conversation
arham766
requested review from
cixzhang,
ejhammond,
imdreamrunner,
josephfarina,
marie-lucas,
nynexman4464 and
thedjpetersen
as code owners
|
@arham766 is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
meta-cla
Bot
added
the
CLA Signed
arham766
force-pushed
the
fix/adjust-time-nonfinite
branch
from
630a129 to
2a86b37
Compare
deltaMinutes of -Infinity spun the wrap-around while-loop forever (a pure CPU spin that freezes the tab), and NaN skipped the loop and flowed through formatISOTime as the corrupt string "NaN:NaN", which isTimeInRange does not reject. Both are reachable from DateTimeInput's public timeIncrement prop via the ArrowUp/Down handler when a consumer computes the step (undefined arithmetic or division by zero). Return the input unchanged for non-finite deltas, and replace the negative wrap-around loop with a double-modulo so huge finite negative deltas also wrap in constant time. Fixes facebook#3583
arham766
force-pushed
the
fix/adjust-time-nonfinite
branch
from
2a86b37 to
f9c4a69
Compare
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
1 participant
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.
Summary
Fixes #3583.
adjustTime's wrap-around loop never terminates fordeltaMinutes = -Infinity(-Infinity + 1440is still-Infinity) — a pure CPU spin that freezes the tab — andNaNskips the loop and flows throughformatISOTimeas the corrupt string"NaN:NaN", whichisTimeInRangedoesn't reject, so DateTimeInput can commit it into consumer form state. Both are reachable from the publictimeIncrementprop via the ArrowUp/ArrowDown handler whenever a consumer computes the step (60 * undefined→ NaN, division by zero → ±Infinity).The fix: non-finite deltas return the input time unchanged, and the negative wrap-around becomes a double-modulo (
((m % 1440) + 1440) % 1440) — O(1) for huge finite negatives too.Test plan
-Infinity/Infinity/NaNall return the input unchanged (the -Infinity case previously hung — verified via subprocess kill at 8s pre-fix); huge negative finite deltas wrap correctly in constant time