CsvHelpers: validate MM/DD/YYYY dates in UTC#97
Open
arpitjain099 wants to merge 1 commit into
Open
Conversation
isValidDate compared the parsed date against getUTC* values, but only the ISO YYYY-MM-DD branch was actually parsed as UTC. The MM/DD/YYYY slash form is parsed in the runtime's local time zone, so in positive-offset zones the parsed instant rolled back to the previous calendar day and valid dates were rejected. Build the date with Date.UTC from the captured components so validation no longer depends on the runtime time zone. The impossible-date guard (e.g. Feb 31) and the ISO branch are unaffected. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
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.
isValidDateinsrc/validators/CsvHelpers.tschecks the parsed date withgetUTCFullYear()/getUTCMonth()/getUTCDate(). That's right for the ISOYYYY-MM-DDbranch, where date-only strings parse as UTC, but wrong for theMM/DD/YYYY(andM/D/YYYY) branch: the slash form parses in the local time zone and then gets read back with UTC getters.In any positive UTC offset (a contributor in
Asia/Tokyo, or a CI runner not pinned to UTC), local midnight lands on the previous calendar day in UTC, so the getters report the day before, the equality check fails, and a validlast_updated_onlike05/01/2024is flagged asInvalidDateError. The repo's own tests show it:TZ=UTC npx vitest runpasses, butTZ=Asia/Tokyo npx vitest runfails the MM/DD/YYYY and M/D/YYYYlast_updated_oncases for v2.0.0, v2.1.0, and v2.2.0. The suite already encodes the right behavior and only passes because CI runs in UTC.The fix builds the date in UTC from the components already parsed out of the string instead of re-parsing the raw value:
The impossible-date guard (Feb 31, month 13) still works since
Date.UTCnormalizes the same way and the equality check rejects them. The ISO branch is left alone, and I fixed the now-wrong comment that claimed the slash form parsed as UTC.No new tests needed; the existing suite covers it, it just had to run outside UTC. After the change
TZ=Asia/Tokyo,TZ=UTC, andTZ=America/Los_Angelesall pass 344/344, and prettier and eslint are clean on the file.