Repository navigation
fs: preserve negative utimes timestamps - #66634
Open
guybedford wants to merge 1 commit into
Open
guybedford wants to merge 1 commit into
guybedford wants to merge 1 commit into
Conversation
`toUnixTimestamp()` silently replaced negative numeric timestamps with the current time, while negative strings and Dates were passed through as pre-epoch times. On Windows, stat times were additionally reinterpreted as unsigned so that pre-epoch times read back as post-2038 times. Treat negative times as pre-epoch on all platforms, consistent with POSIX, at the cost of post-2038 times on Windows which libuv is unable to represent. Fixes: nodejs#64597 Refs: nodejs#64627 Assisted-by: OpenCode Signed-off-by: Guy Bedford <guybedford@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #66634 +/- ##
==========================================
- Coverage 92.78% 90.43% -2.35%
==========================================
Files 422 791 +369
Lines 193692 276589 +82897
Branches 29881 53112 +23231
==========================================
+ Hits 179718 250137 +70419
- Misses 13645 16860 +3215
- Partials 329 9592 +9263
🚀 New features to boost your workflow:
|
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.
This is the semver-major alternative to #64627, fixes #64597.
toUnixTimestamp()silently replaced negative numeric timestamps with the current time, while negative strings andDates were passed through as pre-epoch times:Removing the clamp exposed that Windows reinterprets stat
tv_secasunsigned long, so that pre-epoch times read back as post-2038 times (viastatic_cast<unsigned long>inFillStatsArray). This was a Node.js decision from #43714 to work around libuv's 32-bitlong tv_secon Windows, which is what thetest-fs-utimes-y2K38.jsWindowsoverflow_mtimecase covered. As discussed in #64627, this PR treats negative times as pre-epoch on all platforms, consistent with POSIX:toUnixTimestamp()no longer replaces negative numbers with the current time.2038-01-19T03:14:07Zare no longer representable on Windows until libuv gains 64-bit time support.atime/mtimerules and the Windows time range.Test changes:
test-fs-utimes.jsnow computes the expected mtime independently offs._toUnixTimestamp()(previously the oracle was the code under test, so the negative case always passed), uses an absolute difference, and adds negative number and pre-epochDatecases. These are skipped on AIX, which rejects pre-epoch times withEINVAL.test-fs-utimes-y2K38.jsskips the Y2K38 round trip on Windows and drops theoverflow_mtimecase, keeping the Windows precision check.A CITGM run is suggested per the discussion in #64627.