Fix: Improve/refactor metadata handling & fix tests

gulpjs · Nov 30, 2017 · b7cb0f0 · b7cb0f0
1 parent 559c2d5
commit b7cb0f0
Show file tree

Hide file tree

Showing 7 changed files with 134 additions and 122 deletions.
diff --git a/lib/dest/writeContents/index.js b/lib/dest/writeContents/index.js
@@ -1,64 +1,22 @@
 'use strict';
 
-var fs = require('fs');
+var fs = require('graceful-fs');
+var assign = require('object-assign');
 
 var writeDir = require('./writeDir');
 var writeStream = require('./writeStream');
 var writeBuffer = require('./writeBuffer');
 var writeSymbolicLink = require('./writeSymbolicLink');
 
-
-function getMode(oldStat, newStat) {
-  if (typeof newStat.mode !== 'number') {
-    return;
-  }
-
-  var currentMode = (oldStat.mode & parseInt('0777', 8));
-  var expectedMode = (newStat.mode & parseInt('0777', 8));
-  if (currentMode !== expectedMode) {
-    return expectedMode;
-  }
-}
-
-function getUtimes(oldStat, newStat) {
-  // `futimes` only works if we own the file
-  if (oldStat.uid !== process.getuid()) {
-    return;
-  }
-
-  // Given `mtime` is not valid, do nothing
-  var mtime = newStat.mtime;
-  if (!validDate(mtime)) {
-    return;
-  }
-
-  // Given `atime` is not valid, assign a new one with current time
-  var atime = newStat.atime;
-  if (!validDate(atime)) {
-    atime = new Date();
-  }
-
-  return { atime: atime, mtime: mtime };
-}
+// TODO include sticky/setuid/setgid, i.e. 7777?
+var MASK_MODE = parseInt('0777', 8);
 
 
 // http://stackoverflow.com/a/10589791/586382
 function validDate(date) {
   return date instanceof Date && !isNaN(date.valueOf());
 }
 
-function futimes(fd, oldStat, newStat, cb) {
-  setTimeout(function() {
-    var stat = getUtimes(oldStat, newStat);
-    if (!stat) {
-      return cb();
-    }
-
-    // Set file `atime` and `mtime` fields
-    fs.futimes(fd, stat.atime, stat.mtime, cb);
-  }, 0);
-}
-
 
 function writeContents(writePath, file, cb) {
   // If directory then mkdirp it
@@ -83,77 +41,124 @@ function writeContents(writePath, file, cb) {
 
   // If no contents then do nothing
   if (file.isNull()) {
-    return complete();
+    return finish();
   }
 
-  function complete(err) {
-    cb(err, file);
-  }
 
-  function stat(fd) {
-    function complete2(err1) {
-      fs.close(fd, function(err2) {
-        complete(err1 || err2);
-      });
+  // This is invoked by the various writeXxx modules when they've finished
+  // writing the contents.
+  // The third argument, if present, should be invoked to indicate we're done
+  // with the file descriptor.
+  function written(err, fd, close) {
+    close = close || function(err, cb) {
+      cb(err);
+    };
+
+    if (err && !(err.code === 'EEXIST' && file.flag === 'wx')) {
+      return close(err, finish);
     }
 
-    fs.fstat(fd, function(err, st) {
-      if (err) {
-        return complete2(err);
-      }
+    // TODO handle symlinks properly
+    if (!file.stat || file.symlink) {
+      return close(null, finish);
+    }
 
-      var stat = file.stat;
-      var mode = getMode(st, stat);
-      if (mode == null) {
-        return futimes(fd, st, stat, complete2);
-      }
+    if (typeof fd === 'number') {
+      return stat(fd, close);
+    }
 
-      fs.fchmod(fd, mode, function(error) {
-        if (error) {
-          return complete2(error);
-        }
+    // No file descriptor given (writeDir or writeSymbolicLink) so create one.
+    fs.open(writePath, 'r', function(err, fd) {
+      if (err) {
+        return close(err, finish);
+      }
 
-        futimes(fd, st, stat, complete2);
+      stat(fd, function(err1) {
+        fs.close(fd, function(err2) {
+          finish(err1 || err2);
+        });
       });
     });
   }
 
 
-  function written(err, fd) {
-    if (isErrorFatal(err)) {
-      return complete(err);
-    }
+  // Set mode and/or atime, mtime.
+  function stat(fd, close) {
+    fs.fstat(fd, function(err, stat) {
+      if (err) {
+        return close(err, finish);
+      }
 
-    if (!file.stat) {
-      return complete();
-    }
+      // Check if mode needs to be updated
+      var modeDiff = 0;
+      if (typeof file.stat.mode === 'number') {
+        modeDiff = (file.stat.mode ^ stat.mode) & MASK_MODE;
+      }
 
-    if (typeof fd === 'number') {
-      return stat(fd);
-    }
+      // Check if atime/mtime need to be updated
+      var timesDiff = null;
+      if (validDate(file.stat.mtime)) {
+        timesDiff = {
+          mtime: file.stat.mtime,
+          atime: validDate(file.stat.atime) ? file.stat.atime : stat.atime,
+        };
+      }
 
-    fs.open(writePath, 'r', function(err, fd) {
-      if (err) {
-        return complete(err);
+      // Set file.stat to the reflect current state on disk
+      assign(file.stat, stat);
+
+      // Nothing to do
+      if (!modeDiff && !timesDiff) {
+        return close(null, finish);
       }
 
-      stat(fd);
+      // Check access, `futimes` and `fchmod` only work if we own the file,
+      // or if we are effectively root.
+      var uid = process.geteuid ? process.geteuid() : process.getuid();
+      if (stat.uid !== uid && uid !== 0) {
+        return close(null, finish);
+      }
+
+      if (modeDiff) {
+        return mode();
+      }
+      times();
+
+      function mode() {
+        var mode = stat.mode ^ modeDiff;
+        fs.fchmod(fd, mode, function(err) {
+          if (!err) {
+            file.stat.mode = mode;
+            file.stat.ctime.setTime(Date.now());
+          }
+          if (timesDiff) {
+            return times(err);
+          }
+          close(err, finish);
+        });
+      }
+
+      function times(err1) {
+        fs.futimes(fd, timesDiff.atime, timesDiff.mtime, function(err2) {
+          if (!err2) {
+            file.stat.atime = timesDiff.atime;
+            file.stat.mtime = timesDiff.mtime;
+            file.stat.ctime.setTime(Date.now());
+          }
+          close(err1 || err2, finish);
+        });
+      }
     });
   }
 
-  function isErrorFatal(err) {
-    if (!err) {
-      return false;
-    }
 
-    // Handle scenario for file overwrite failures.
-    if (err.code === 'EEXIST' && file.flag === 'wx') {
-      return false;   // "These aren't the droids you're looking for"
-    }
-
-    // Otherwise, this is a fatal error
-    return true;
+  // This is invoked by the close callbacks; we're all done.
+  function finish(err) {
+    cb(err, file);
   }
+
+
+
 }
 
 module.exports = writeContents;
diff --git a/lib/dest/writeContents/writeBuffer.js b/lib/dest/writeContents/writeBuffer.js
@@ -2,16 +2,20 @@
 
 var fs = require('graceful-fs');
 
-function writeBuffer(writePath, file, cb) {
+function writeBuffer(writePath, file, written) {
   var stat = file.stat;
 
   fs.open(writePath, file.flag, stat.mode, function(err, fd) {
     if (err) {
-      return cb(err);
+      return written(err);
     }
 
     fs.write(fd, file.contents, 0, file.contents.length, 0, function(err) {
-      cb(err, fd);
+      written(err, fd, function(err1, finish) {
+        fs.close(fd, function(err2) {
+          finish(err1 || err2);
+        });
+      });
     });
   });
 }

diff --git a/lib/dest/writeContents/writeDir.js b/lib/dest/writeContents/writeDir.js
@@ -2,8 +2,8 @@
 
 var mkdirp = require('mkdirp');
 
-function writeDir(writePath, file, cb) {
-  mkdirp(writePath, file.stat.mode, cb);
+function writeDir(writePath, file, written) {
+  mkdirp(writePath, file.stat.mode, written);
 }
 
 module.exports = writeDir;
diff --git a/lib/dest/writeContents/writeStream.js b/lib/dest/writeContents/writeStream.js
@@ -4,7 +4,7 @@ var fs = require('graceful-fs');
 
 var streamFile = require('../../src/getContents/streamFile');
 
-function writeStream(writePath, file, cb) {
+function writeStream(writePath, file, written) {
   var stat = file.stat;
   var mode = stat.mode;
 
@@ -13,7 +13,7 @@ function writeStream(writePath, file, cb) {
 
   fs.open(writePath, 'w', mode, function(err, fd) {
     if (err) {
-      cb(err);
+      written(err);
     }
 
     var opt = {
@@ -43,8 +43,7 @@ function writeStream(writePath, file, cb) {
         return complete(error);
       }
 
-      // All finished with WriteStream, close and clean up
-      outStream.end();
+      complete();
     });
   }
 
@@ -56,7 +55,11 @@ function writeStream(writePath, file, cb) {
       outStream.removeListener('error', complete);
       outStream.removeListener('finish', complete);
     }
-    cb(err, outFD);
+    written(err, outFD, function(err1, finish) {
+      outStream.end(function(err2) {
+        finish(err1 || err2);
+      });
+    });
   }
 }
 

diff --git a/lib/dest/writeContents/writeSymbolicLink.js b/lib/dest/writeContents/writeSymbolicLink.js
@@ -2,13 +2,13 @@
 
 var fs = require('graceful-fs');
 
-function writeSymbolicLink(writePath, file, cb) {
+function writeSymbolicLink(writePath, file, written) {
   fs.symlink(file.symlink, writePath, function(err) {
     if (err && err.code !== 'EEXIST') {
       return cb(err);
     }
 
-    cb();
+    written();
   });
 }