From fac3385da429a88ed0f025d2567c9476b3eb838a Mon Sep 17 00:00:00 2001 From: Stephan van Leeuwen Date: Tue, 5 Jul 2016 14:54:14 +0200 Subject: [PATCH 1/3] Added support for changing uid/gid --- lib/file-operations.js | 67 +++++++++++++- test/dest-owner.js | 99 ++++++++++++++++++++ test/file-operations.js | 201 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 365 insertions(+), 2 deletions(-) create mode 100644 test/dest-owner.js diff --git a/lib/file-operations.js b/lib/file-operations.js index e49c4524..beb49e57 100644 --- a/lib/file-operations.js +++ b/lib/file-operations.js @@ -66,6 +66,44 @@ function getTimesDiff(fsStat, vinylStat) { return timesDiff; } +function getOwnerDiff(fsStat, vinylStat) { + if (typeof vinylStat.uid !== 'number' && + typeof vinylStat.gid !== 'number') { + return; + } + + var uid = fsStat.uid; // Default to current uid. + if (typeof vinylStat.uid === 'number' && + vinylStat.uid >= 0) { + uid = vinylStat.uid; + } + + var gid = fsStat.gid; // Default to current gid. + if (typeof vinylStat.gid === 'number' && + vinylStat.gid >= 0) { + gid = vinylStat.gid; + } + + if (isEqual(uid, fsStat.uid) && + isEqual(gid, fsStat.gid)) { + return; + } + + if (uid < 0) { + uid = undefined; + } + if (gid < 0) { + gid = undefined; + } + + var ownerDiff = { + uid: uid, + gid: gid, + }; + + return ownerDiff; +} + function isOwner(fsStat) { var hasGetuid = (typeof process.getuid === 'function'); var hasGeteuid = (typeof process.geteuid === 'function'); @@ -106,11 +144,14 @@ function updateMetadata(fd, file, callback) { // Check if atime/mtime need to be updated var timesDiff = getTimesDiff(stat, file.stat); + // Check if uid/gid need to be updated + var ownerDiff = getOwnerDiff(stat, file.stat); + // Set file.stat to the reflect current state on disk assign(file.stat, stat); // Nothing to do - if (!modeDiff && !timesDiff) { + if (!modeDiff && !timesDiff && !ownerDiff) { return callback(null); } @@ -123,7 +164,10 @@ function updateMetadata(fd, file, callback) { if (modeDiff) { return mode(); } - times(); + if (timesDiff) { + return times(); + } + owner(); function mode() { var mode = stat.mode ^ modeDiff; @@ -137,6 +181,9 @@ function updateMetadata(fd, file, callback) { if (timesDiff) { return times(fchmodErr); } + if (ownerDiff) { + return owner(fchmodErr); + } callback(fchmodErr); } } @@ -149,9 +196,24 @@ function updateMetadata(fd, file, callback) { file.stat.atime = timesDiff.atime; file.stat.mtime = timesDiff.mtime; } + if (ownerDiff) { + owner(fchmodErr, futimesErr); + } callback(fchmodErr || futimesErr); } } + + function owner(fchmodErr, futimesErr) { + fs.fchown(fd, ownerDiff.uid, ownerDiff.gid, onFchown); + + function onFchown(fchownErr) { + if (!fchownErr) { + file.stat.uid = ownerDiff.uid; + file.stat.gid = ownerDiff.gid; + } + callback(fchmodErr || futimesErr || fchownErr); + } + } } } @@ -259,6 +321,7 @@ module.exports = { closeFd: closeFd, getModeDiff: getModeDiff, getTimesDiff: getTimesDiff, + getOwnerDiff: getOwnerDiff, isOwner: isOwner, updateMetadata: updateMetadata, writeFile: writeFile, diff --git a/test/dest-owner.js b/test/dest-owner.js new file mode 100644 index 00000000..3705c81b --- /dev/null +++ b/test/dest-owner.js @@ -0,0 +1,99 @@ +'use strict'; + +var os = require('os'); +var path = require('path'); + +var fs = require('graceful-fs'); +var del = require('del'); +var File = require('vinyl'); +var expect = require('expect'); + +var vfs = require('../'); + +function wipeOut() { + this.timeout(20000); + + expect.restoreSpies(); + + // Async del to get sort-of-fix for https://github.com/isaacs/rimraf/issues/72 + return del(path.join(__dirname, './out-fixtures/')); +} + +var isWindows = (os.platform() === 'win32'); + +describe('.dest() with custom owner', function() { + beforeEach(wipeOut); + afterEach(wipeOut); + + it('should call fchown when the uid and/or gid are provided on the vinyl stat', function(done) { + if (isWindows) { + this.skip(); + return; + } + + var inputPath = path.join(__dirname, './fixtures/test.coffee'); + var inputBase = path.join(__dirname, './fixtures/'); + var expectedPath = path.join(__dirname, './out-fixtures/test.coffee'); + var expectedContents = fs.readFileSync(inputPath); + + var fchownSpy = expect.spyOn(fs, 'fchown').andCallThrough(); + + var expectedFile = new File({ + base: inputBase, + cwd: __dirname, + path: inputPath, + contents: expectedContents, + stat: { + uid: 1001, + gid: 1001, + }, + }); + + var onEnd = function() { + expect(fchownSpy.calls.length).toEqual(1); + expect(fchownSpy.calls[0].arguments[1]).toEqual(1001); + expect(fchownSpy.calls[0].arguments[2]).toEqual(1001); + done(); + }; + + var stream = vfs.dest('./out-fixtures/', { cwd: __dirname }); + stream.on('end', onEnd); + stream.write(expectedFile); + stream.end(); + }); + + it('should not call fchown when the uid and gid provided on the vinyl stat are invalid', function(done) { + if (isWindows) { + this.skip(); + return; + } + + var inputPath = path.join(__dirname, './fixtures/test.coffee'); + var inputBase = path.join(__dirname, './fixtures/'); + var expectedPath = path.join(__dirname, './out-fixtures/test.coffee'); + var expectedContents = fs.readFileSync(inputPath); + + var fchownSpy = expect.spyOn(fs, 'fchown').andCallThrough(); + + var expectedFile = new File({ + base: inputBase, + cwd: __dirname, + path: inputPath, + contents: expectedContents, + stat: { + uid: -1, + gid: -1, + }, + }); + + var onEnd = function() { + expect(fchownSpy.calls.length).toEqual(0); + done(); + }; + + var stream = vfs.dest('./out-fixtures/', { cwd: __dirname }); + stream.on('end', onEnd); + stream.write(expectedFile); + stream.end(); + }); +}); diff --git a/test/file-operations.js b/test/file-operations.js index 375319f3..89112a38 100644 --- a/test/file-operations.js +++ b/test/file-operations.js @@ -19,6 +19,7 @@ var isOwner = fo.isOwner; var writeFile = fo.writeFile; var getModeDiff = fo.getModeDiff; var getTimesDiff = fo.getTimesDiff; +var getOwnerDiff = fo.getOwnerDiff; var updateMetadata = fo.updateMetadata; var resolution = defaultResolution(); @@ -333,6 +334,206 @@ describe('getTimesDiff', function() { }); }); +describe('getOwnerDiff', function() { + + it('returns undefined if vinyl uid & gid are invalid', function(done) { + var fsStat = { + uid: 1000, + gid: 1000, + }; + var vfsStat = { + uid: undefined, + gid: undefined, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(undefined); + + done(); + }); + + it('returns undefined if vinyl uid & gid are both equal to counterparts', function(done) { + var fsStat = { + uid: 1000, + gid: 1000, + }; + var vfsStat = { + uid: 1000, + gid: 1000, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(undefined); + + done(); + }); + + it('returns a diff object if uid or gid do not match', function(done) { + var fsStat = { + uid: 1000, + gid: 1000, + }; + var vfsStat = { + uid: 1001, + gid: 1000, + }; + var expected = { + uid: 1001, + gid: 1000, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(expected); + + vfsStat = { + uid: 1000, + gid: 1001, + }; + expected = { + uid: 1000, + gid: 1001, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(expected); + + done(); + }); + + it('returns the fs uid if the vinyl uid is invalid', function(done) { + var fsStat = { + uid: 1000, + gid: 1000, + }; + var vfsStat = { + uid: undefined, + gid: 1001, + }; + var expected = { + uid: 1000, + gid: 1001, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(expected); + + var vfsStat = { + uid: -1, + gid: 1001, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(expected); + + done(); + }); + + it('returns the fs gid if the vinyl gid is invalid', function(done) { + var fsStat = { + uid: 1000, + gid: 1000, + }; + var vfsStat = { + uid: 1001, + gid: undefined, + }; + var expected = { + uid: 1001, + gid: 1000, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(expected); + + var vfsStat = { + uid: 1001, + gid: -1, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(expected); + + done(); + }); + + it('makes uid diff undefined if fs and vinyl uid are invalid', function(done) { + var fsStat = { + uid: undefined, + gid: 1000, + }; + var vfsStat = { + uid: undefined, + gid: 1001, + }; + var expected = { + uid: undefined, + gid: 1001, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(expected); + + var fsStat = { + uid: -1, + gid: 1000, + }; + var vfsStat = { + uid: -1, + gid: 1001, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(expected); + + done(); + }); + + it('makes gid diff undefined if fs and vinyl gid are invalid', function(done) { + var fsStat = { + uid: 1000, + gid: undefined, + }; + var vfsStat = { + uid: 1001, + gid: undefined, + }; + var expected = { + uid: 1001, + gid: undefined, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(expected); + + fsStat = { + uid: 1000, + gid: -1, + }; + vfsStat = { + uid: 1001, + gid: -1, + }; + + var result = getOwnerDiff(fsStat, vfsStat); + + expect(result).toEqual(expected); + + done(); + }); + +}); + describe('closeFd', function() { it('calls the callback with propagated error if fd is not a number', function(done) { From e67af4155d283ec1dc4d9f8d156b61fd977f575e Mon Sep 17 00:00:00 2001 From: Stephan van Leeuwen Date: Wed, 6 Jul 2016 10:50:13 +0200 Subject: [PATCH 2/3] Added isValidUnixId method, fixed callback issue --- lib/file-operations.js | 40 ++++++++++++++++++++++------------------ test/file-operations.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/lib/file-operations.js b/lib/file-operations.js index beb49e57..ae507b93 100644 --- a/lib/file-operations.js +++ b/lib/file-operations.js @@ -26,6 +26,18 @@ function closeFd(propagatedErr, fd, callback) { } } +function isValidUnixId(id) { + if (typeof id !== 'number') { + return false; + } + + if (id < 0) { + return false; + } + + return true; +} + function getModeDiff(fsMode, vinylMode) { var modeDiff = 0; @@ -67,20 +79,18 @@ function getTimesDiff(fsStat, vinylStat) { } function getOwnerDiff(fsStat, vinylStat) { - if (typeof vinylStat.uid !== 'number' && - typeof vinylStat.gid !== 'number') { + if (!isValidUnixId(vinylStat.uid) && + !isValidUnixId(vinylStat.gid)) { return; } - var uid = fsStat.uid; // Default to current uid. - if (typeof vinylStat.uid === 'number' && - vinylStat.uid >= 0) { + var uid = isValidUnixId(fsStat.uid) ? fsStat.uid : undefined; // Default to current uid. + if (isValidUnixId(vinylStat.uid)) { uid = vinylStat.uid; } - var gid = fsStat.gid; // Default to current gid. - if (typeof vinylStat.gid === 'number' && - vinylStat.gid >= 0) { + var gid = isValidUnixId(fsStat.gid) ? fsStat.gid : undefined; // Default to current gid. + if (isValidUnixId(vinylStat.gid)) { gid = vinylStat.gid; } @@ -89,13 +99,6 @@ function getOwnerDiff(fsStat, vinylStat) { return; } - if (uid < 0) { - uid = undefined; - } - if (gid < 0) { - gid = undefined; - } - var ownerDiff = { uid: uid, gid: gid, @@ -197,13 +200,13 @@ function updateMetadata(fd, file, callback) { file.stat.mtime = timesDiff.mtime; } if (ownerDiff) { - owner(fchmodErr, futimesErr); + return owner(fchmodErr || futimesErr); } callback(fchmodErr || futimesErr); } } - function owner(fchmodErr, futimesErr) { + function owner(earlierErr) { fs.fchown(fd, ownerDiff.uid, ownerDiff.gid, onFchown); function onFchown(fchownErr) { @@ -211,7 +214,7 @@ function updateMetadata(fd, file, callback) { file.stat.uid = ownerDiff.uid; file.stat.gid = ownerDiff.gid; } - callback(fchmodErr || futimesErr || fchownErr); + callback(earlierErr || fchownErr); } } } @@ -319,6 +322,7 @@ function mkdirp(dirpath, customMode, callback) { module.exports = { closeFd: closeFd, + isValidUnixId: isValidUnixId, getModeDiff: getModeDiff, getTimesDiff: getTimesDiff, getOwnerDiff: getOwnerDiff, diff --git a/test/file-operations.js b/test/file-operations.js index 89112a38..84b2b86e 100644 --- a/test/file-operations.js +++ b/test/file-operations.js @@ -20,6 +20,7 @@ var writeFile = fo.writeFile; var getModeDiff = fo.getModeDiff; var getTimesDiff = fo.getTimesDiff; var getOwnerDiff = fo.getOwnerDiff; +var isValidUnixId = fo.isValidUnixId; var updateMetadata = fo.updateMetadata; var resolution = defaultResolution(); @@ -132,6 +133,33 @@ describe('isOwner', function() { }); }); +describe('isValidUnixId', function() { + + it('returns true if the given id is a valid unix id', function(done) { + var result = isValidUnixId(1000); + + expect(result).toEqual(true); + + done(); + }); + + it('returns false if the given id is not a number', function(done) { + var result = isValidUnixId('root'); + + expect(result).toEqual(false); + + done(); + }); + + it('returns false when the given id is less than 0', function(done) { + var result = isValidUnixId(-1); + + expect(result).toEqual(false); + + done(); + }); +}); + describe('getModeDiff', function() { it('returns 0 if both modes are the same', function(done) { From 7f955e3319d512633348addcc1f9e057997f19d4 Mon Sep 17 00:00:00 2001 From: Stephan van Leeuwen Date: Wed, 6 Jul 2016 20:36:40 +0200 Subject: [PATCH 3/3] Return undefined when fs uid/gid & vfs uid/gid are undefined --- lib/file-operations.js | 9 +++++++-- test/file-operations.js | 20 ++++++-------------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/lib/file-operations.js b/lib/file-operations.js index ae507b93..64a4fa38 100644 --- a/lib/file-operations.js +++ b/lib/file-operations.js @@ -84,12 +84,17 @@ function getOwnerDiff(fsStat, vinylStat) { return; } - var uid = isValidUnixId(fsStat.uid) ? fsStat.uid : undefined; // Default to current uid. + if ((!isValidUnixId(fsStat.uid) && !isValidUnixId(vinylStat.uid)) || + (!isValidUnixId(fsStat.gid) && !isValidUnixId(vinylStat.gid))) { + return; + } + + var uid = fsStat.uid; // Default to current uid. if (isValidUnixId(vinylStat.uid)) { uid = vinylStat.uid; } - var gid = isValidUnixId(fsStat.gid) ? fsStat.gid : undefined; // Default to current gid. + var gid = fsStat.gid; // Default to current gid. if (isValidUnixId(vinylStat.gid)) { gid = vinylStat.gid; } diff --git a/test/file-operations.js b/test/file-operations.js index 84b2b86e..c0fe35f2 100644 --- a/test/file-operations.js +++ b/test/file-operations.js @@ -492,7 +492,7 @@ describe('getOwnerDiff', function() { done(); }); - it('makes uid diff undefined if fs and vinyl uid are invalid', function(done) { + it('returns undefined if fs and vinyl uid are invalid', function(done) { var fsStat = { uid: undefined, gid: 1000, @@ -501,14 +501,10 @@ describe('getOwnerDiff', function() { uid: undefined, gid: 1001, }; - var expected = { - uid: undefined, - gid: 1001, - }; var result = getOwnerDiff(fsStat, vfsStat); - expect(result).toEqual(expected); + expect(result).toEqual(undefined); var fsStat = { uid: -1, @@ -521,12 +517,12 @@ describe('getOwnerDiff', function() { var result = getOwnerDiff(fsStat, vfsStat); - expect(result).toEqual(expected); + expect(result).toEqual(undefined); done(); }); - it('makes gid diff undefined if fs and vinyl gid are invalid', function(done) { + it('returns undefined if fs and vinyl gid are invalid', function(done) { var fsStat = { uid: 1000, gid: undefined, @@ -535,14 +531,10 @@ describe('getOwnerDiff', function() { uid: 1001, gid: undefined, }; - var expected = { - uid: 1001, - gid: undefined, - }; var result = getOwnerDiff(fsStat, vfsStat); - expect(result).toEqual(expected); + expect(result).toEqual(undefined); fsStat = { uid: 1000, @@ -555,7 +547,7 @@ describe('getOwnerDiff', function() { var result = getOwnerDiff(fsStat, vfsStat); - expect(result).toEqual(expected); + expect(result).toEqual(undefined); done(); });