Skip to content

Commit

Permalink
feat!: Stop using process.umask() & fallback to node default mode (#6)
Browse files Browse the repository at this point in the history 
ref nodejs/node#32321

Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
  • Loading branch information
@coreyfarrell @phated
coreyfarrell and phated committed Jan 10, 2022
1 parent 872eb2a commit f78d60b
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 44 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/dev.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ jobs:

- name: Run tests
run: npm test
env:
VERBOSE: true

- name: Coveralls
uses: coverallsapp/github-action@v1.1.0
Expand Down
19 changes: 10 additions & 9 deletions mkdirp.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,17 @@ var path = require('path');
var fs = require('graceful-fs');

var MASK_MODE = parseInt('7777', 8);
var DEFAULT_DIR_MODE = parseInt('0777', 8);

function mkdirp(dirpath, customMode, callback) {
if (typeof customMode === 'function') {
callback = customMode;
customMode = undefined;
function mkdirp(dirpath, mode, callback) {
if (typeof mode === 'function') {
callback = mode;
mode = undefined;
}

if (typeof mode === 'string') {
mode = parseInt(mode, 8);
}

var mode = customMode || DEFAULT_DIR_MODE & ~process.umask();
dirpath = path.resolve(dirpath);

fs.mkdir(dirpath, mode, onMkdir);
Expand Down Expand Up @@ -46,12 +48,11 @@ function mkdirp(dirpath, customMode, callback) {
return callback(mkdirErr);
}

// TODO: Is it proper to mask like this?
if ((stats.mode & MASK_MODE) === mode) {
if (!mode) {
return callback();
}

if (!customMode) {
if ((stats.mode & MASK_MODE) === mode) {
return callback();
}

Expand Down
7 changes: 6 additions & 1 deletion test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ describe('mkdirpStream', function () {
mode = parseInt(mode, 8);
}

return mode & ~process.umask();
// Set to use to "get" it
var current = process.umask(0);
// Then set it back for the next test
process.umask(current);

return mode & ~current;
}

beforeEach(cleanup);
Expand Down
144 changes: 110 additions & 34 deletions test/mkdirp.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,20 @@ var rimraf = require('rimraf');

var mkdirp = require('../mkdirp');

describe('mkdirp', function () {
var log = {
expected: function(expected) {
if (process.env.VERBOSE) {
console.log('Expected mode:', expected.toString(8));
}
},
found: function(found) {
if (process.env.VERBOSE) {
console.log('Found mode', found.toString(8));
}
}
}

function suite () {
var MASK_MODE = parseInt('7777', 8);
var DEFAULT_DIR_MODE = parseInt('0777', 8);
var isWindows = os.platform() === 'win32';
Expand All @@ -34,16 +47,30 @@ describe('mkdirp', function () {
return mode & MASK_MODE;
}

function statMode(outputPath) {
return masked(fs.lstatSync(outputPath).mode);
function createdMode(outputPath) {
var mode = masked(fs.lstatSync(outputPath).mode);
log.found(mode);
return mode;
}

function applyUmask(mode) {
function expectedMode(mode) {
if (typeof mode !== 'number') {
mode = parseInt(mode, 8);
}

return mode & ~process.umask();
log.expected(mode);
return mode;
}

function expectedDefaultMode() {
// Set to use to "get" it
var current = process.umask(0);
// Then set it back for the next test
process.umask(current);

var mode = (DEFAULT_DIR_MODE & ~current);
log.expected(mode);
return mode;
}

beforeEach(cleanup);
Expand All @@ -64,7 +91,7 @@ describe('mkdirp', function () {
it('makes a single directory', function (done) {
mkdirp(outputDirpath, function (err) {
expect(err).toBeFalsy();
expect(statMode(outputDirpath)).toBeDefined();
expect(createdMode(outputDirpath)).toBeDefined();

done();
});
Expand All @@ -76,11 +103,9 @@ describe('mkdirp', function () {
return;
}

var defaultMode = applyUmask(DEFAULT_DIR_MODE);

mkdirp(outputDirpath, function (err) {
expect(err).toBeFalsy();
expect(statMode(outputDirpath)).toEqual(defaultMode);
expect(createdMode(outputDirpath)).toEqual(expectedDefaultMode());

done();
});
Expand All @@ -89,7 +114,7 @@ describe('mkdirp', function () {
it('makes multiple directories', function (done) {
mkdirp(outputNestedDirpath, function (err) {
expect(err).toBeFalsy();
expect(statMode(outputNestedDirpath)).toBeDefined();
expect(createdMode(outputNestedDirpath)).toBeDefined();

done();
});
Expand All @@ -101,27 +126,57 @@ describe('mkdirp', function () {
return;
}

var defaultMode = applyUmask(DEFAULT_DIR_MODE);

mkdirp(outputNestedDirpath, function (err) {
expect(err).toBeFalsy();
expect(statMode(outputNestedDirpath)).toEqual(defaultMode);
expect(createdMode(outputNestedDirpath)).toEqual(expectedDefaultMode());

done();
});
});

it('makes directory with custom mode as string', function (done) {
if (isWindows) {
this.skip();
return;
}

var mode = '777';

mkdirp(outputDirpath, mode, function (err) {
expect(err).toBeFalsy();
expect(createdMode(outputDirpath)).toEqual(expectedMode(mode));

done();
});
});

it('makes directory with custom mode', function (done) {
it('makes directory with custom mode as octal', function (done) {
if (isWindows) {
this.skip();
return;
}

var mode = applyUmask('700');
var mode = parseInt('777', 8);

mkdirp(outputDirpath, mode, function (err) {
expect(err).toBeFalsy();
expect(statMode(outputDirpath)).toEqual(mode);
expect(createdMode(outputDirpath)).toEqual(expectedMode(mode));

done();
});
});

it('does not mask a custom mode', function (done) {
if (isWindows) {
this.skip();
return;
}

var mode = parseInt('777', 8);

mkdirp(outputDirpath, mode, function (err) {
expect(err).toBeFalsy();
expect(createdMode(outputDirpath)).toEqual(mode);

done();
});
Expand All @@ -133,11 +188,11 @@ describe('mkdirp', function () {
return;
}

var mode = applyUmask('2700');
var mode = '2700';

mkdirp(outputDirpath, mode, function (err) {
expect(err).toBeFalsy();
expect(statMode(outputDirpath)).toEqual(mode);
expect(createdMode(outputDirpath)).toEqual(expectedMode(mode));

done();
});
Expand All @@ -149,14 +204,14 @@ describe('mkdirp', function () {
return;
}

var mode = applyUmask('700');
var mode = '777';

mkdirp(outputDirpath, mode, function (err) {
expect(err).toBeFalsy();

mkdirp(outputDirpath, function (err2) {
expect(err2).toBeFalsy();
expect(statMode(outputDirpath)).toEqual(mode);
expect(createdMode(outputDirpath)).toEqual(expectedMode(mode));

done();
});
Expand All @@ -169,11 +224,11 @@ describe('mkdirp', function () {
return;
}

var mode = applyUmask('700');
var mode = '777';

mkdirp(outputNestedDirpath, mode, function (err) {
expect(err).toBeFalsy();
expect(statMode(outputNestedDirpath)).toEqual(mode);
expect(createdMode(outputNestedDirpath)).toEqual(expectedMode(mode));

done();
});
Expand All @@ -186,13 +241,13 @@ describe('mkdirp', function () {
}

var intermediateDirpath = path.dirname(outputNestedDirpath);
var mode = applyUmask('700');
var defaultMode = applyUmask(DEFAULT_DIR_MODE);
var mode = '777';

mkdirp(outputNestedDirpath, mode, function (err) {
expect(err).toBeFalsy();
expect(statMode(outputDirpath)).toEqual(defaultMode);
expect(statMode(intermediateDirpath)).toEqual(defaultMode);
expect(createdMode(outputDirpath)).toEqual(expectedDefaultMode());
expect(createdMode(intermediateDirpath)).toEqual(expectedDefaultMode());
expect(createdMode(outputNestedDirpath)).toEqual(expectedMode(mode));

done();
});
Expand All @@ -204,16 +259,15 @@ describe('mkdirp', function () {
return;
}

var mode = applyUmask('700');
var defaultMode = applyUmask(DEFAULT_DIR_MODE);
var mode = '777';

mkdirp(outputDirpath, function (err) {
expect(err).toBeFalsy();
expect(statMode(outputDirpath)).toEqual(defaultMode);
expect(createdMode(outputDirpath)).toEqual(expectedDefaultMode());

mkdirp(outputDirpath, mode, function (err2) {
expect(err2).toBeFalsy();
expect(statMode(outputDirpath)).toEqual(mode);
expect(createdMode(outputDirpath)).toEqual(expectedMode(mode));

done();
});
Expand Down Expand Up @@ -243,19 +297,20 @@ describe('mkdirp', function () {
return;
}

var mode = applyUmask('700');
var mode = '777';

mkdirp(outputDirpath, function (err) {
expect(err).toBeFalsy();

fs.writeFile(outputNestedPath, contents, function (err2) {
expect(err2).toBeFalsy();

var expectedMode = statMode(outputNestedPath);
var existingMode = createdMode(outputNestedPath);
expect(existingMode).not.toEqual(mode);

mkdirp(outputNestedPath, mode, function (err3) {
expect(err3).toBeDefined();
expect(statMode(outputNestedPath)).toEqual(expectedMode);
expect(createdMode(outputNestedPath)).toEqual(existingMode);

done();
});
Expand Down Expand Up @@ -300,7 +355,7 @@ describe('mkdirp', function () {
return;
}

var mode = applyUmask('700');
var mode = '777';

mkdirp(outputDirpath, mode, function (err) {
expect(err).toBeFalsy();
Expand All @@ -315,4 +370,25 @@ describe('mkdirp', function () {
});
});
});
}

describe('mkdirp', suite);

describe('mkdirp with umask', function() {

var startingUmask;
before(function(done) {
startingUmask = process.umask(parseInt('066', 8));

done();
});

after(function(done) {
process.umask(startingUmask);

done();
})

// Initialize the normal suite
suite();
});

0 comments on commit f78d60b

Please sign in to comment.