Refetch atime after copy when using preserveTimestamps option

jprichardson · Sep 9, 2019 · f928144 · f928144
1 parent fb89a29
commit f928144
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 10 deletions.
diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js
@@ -74,7 +74,7 @@ function copyFile (srcStat, src, dest, opts) {
       // (through utimes call)
       fs.chmodSync(dest, srcStat.mode | 0o200)
     }
-    return setDestTimestampsAndMode(srcStat, dest, opts)
+    return setDestTimestampsAndMode(srcStat, src, dest, opts)
   }
   return copyFileFallback(srcStat, src, dest, opts)
 }
@@ -93,17 +93,20 @@ function copyFileFallback (srcStat, src, dest, opts) {
     pos += bytesRead
   }
 
-  setDestTimestampsAndMode(srcStat, fdw, opts)
+  setDestTimestampsAndMode(srcStat, src, fdw, opts)
 
   fs.closeSync(fdr)
   fs.closeSync(fdw)
 }
 
-function setDestTimestampsAndMode (srcStat, dest, opts) {
+function setDestTimestampsAndMode (srcStat, src, dest, opts) {
   const utimesSync = typeof dest === 'string' ? utimesMillisSync : fs.futimesSync
   const chmodSync = typeof dest === 'string' ? fs.chmodSync : fs.fchmodSync
   if (opts.preserveTimestamps) {
-    utimesSync(dest, srcStat.atime, srcStat.mtime)
+    // The initial srcStat.atime cannot be trusted because it is modified by the read(2) system call
+    // (See https://nodejs.org/api/fs.html#fs_stat_time_values)
+    const updatedSrcStat = fs.statSync(src)
+    utimesSync(dest, updatedSrcStat.atime, srcStat.mtime)
   }
   chmodSync(dest, srcStat.mode)
 }

diff --git a/lib/copy/copy.js b/lib/copy/copy.js
@@ -103,10 +103,10 @@ function copyFile (srcStat, src, dest, opts, cb) {
         // (through utimes call)
         return fs.chmod(dest, srcStat.mode | 0o200, (err) => {
           if (err) return cb(err)
-          return setDestTimestampsAndMode(srcStat, dest, opts, cb)
+          return setDestTimestampsAndMode(srcStat, src, dest, opts, cb)
         })
       }
-      return setDestTimestampsAndMode(srcStat, dest, opts, cb)
+      return setDestTimestampsAndMode(srcStat, src, dest, opts, cb)
     })
   }
   return copyFileFallback(srcStat, src, dest, opts, cb)
@@ -119,17 +119,22 @@ function copyFileFallback (srcStat, src, dest, opts, cb) {
     const ws = fs.createWriteStream(dest)
     ws.on('error', err => cb(err))
       .on('open', () => rs.pipe(ws))
-      .once('close', () => setDestTimestampsAndMode(srcStat, dest, opts, cb))
+      .once('close', () => setDestTimestampsAndMode(srcStat, src, dest, opts, cb))
   })
 }
 
-function setDestTimestampsAndMode (srcStat, dest, opts, cb) {
+function setDestTimestampsAndMode (srcStat, src, dest, opts, cb) {
   const utimes = typeof dest === 'string' ? utimesMillis : fs.futimes
   const chmod = typeof dest === 'string' ? fs.chmod : fs.fchmod
   if (opts.preserveTimestamps) {
-    return utimes(dest, srcStat.atime, srcStat.mtime, (err) => {
+    // The initial srcStat.atime cannot be trusted because it is modified by the read(2) system call
+    // (See https://nodejs.org/api/fs.html#fs_stat_time_values)
+    return fs.stat(src, (err, updatedSrcStat) => {
       if (err) return cb(err)
-      return chmod(dest, srcStat.mode, cb)
+      return utimes(dest, updatedSrcStat.atime, srcStat.mtime, (err2) => {
+        if (err2) return cb(err2)
+        return chmod(dest, srcStat.mode, cb)
+      })
     })
   }
   return chmod(dest, srcStat.mode, cb)