Skip to content

Commit d4eeaae

Browse files
coreyfarrellphated
coreyfarrell
and
phated
authored
Breaking: Stop using process.umask() & fallback to node default mode (#6)
ref nodejs/node#32321 Co-authored-by: Blaine Bublitz <[email protected]>
1 parent b6fd81a commit d4eeaae

File tree

4 files changed

+128
-44
lines changed

4 files changed

+128
-44
lines changed

‎.github/workflows/dev.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ jobs:
5353

5454
- name: Run tests
5555
run: npm test
56+
env:
57+
VERBOSE: true
5658

5759
- name: Coveralls
5860
uses: coverallsapp/[email protected]

‎mkdirp.js

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,17 @@ var path = require('path');
55
var fs = require('graceful-fs');
66

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

10-
function mkdirp(dirpath, customMode, callback) {
11-
if (typeof customMode === 'function') {
12-
callback = customMode;
13-
customMode = undefined;
9+
function mkdirp(dirpath, mode, callback) {
10+
if (typeof mode === 'function') {
11+
callback = mode;
12+
mode = undefined;
13+
}
14+
15+
if (typeof mode === 'string') {
16+
mode = parseInt(mode, 8);
1417
}
1518

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

1921
fs.mkdir(dirpath, mode, onMkdir);
@@ -46,12 +48,11 @@ function mkdirp(dirpath, customMode, callback) {
4648
return callback(mkdirErr);
4749
}
4850

49-
// TODO: Is it proper to mask like this?
50-
if ((stats.mode & MASK_MODE) === mode) {
51+
if (!mode) {
5152
return callback();
5253
}
5354

54-
if (!customMode) {
55+
if ((stats.mode & MASK_MODE) === mode) {
5556
return callback();
5657
}
5758

‎test/index.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,12 @@ describe('mkdirpStream', function () {
4444
mode = parseInt(mode, 8);
4545
}
4646

47-
return mode & ~process.umask();
47+
// Set to use to "get" it
48+
var current = process.umask(0);
49+
// Then set it back for the next test
50+
process.umask(current);
51+
52+
return mode & ~current;
4853
}
4954

5055
beforeEach(cleanup);

‎test/mkdirp.js

Lines changed: 110 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,20 @@ var rimraf = require('rimraf');
1010

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

13-
describe('mkdirp', function () {
13+
var log = {
14+
expected: function(expected) {
15+
if (process.env.VERBOSE) {
16+
console.log('Expected mode:', expected.toString(8));
17+
}
18+
},
19+
found: function(found) {
20+
if (process.env.VERBOSE) {
21+
console.log('Found mode', found.toString(8));
22+
}
23+
}
24+
}
25+
26+
function suite () {
1427
var MASK_MODE = parseInt('7777', 8);
1528
var DEFAULT_DIR_MODE = parseInt('0777', 8);
1629
var isWindows = os.platform() === 'win32';
@@ -34,16 +47,30 @@ describe('mkdirp', function () {
3447
return mode & MASK_MODE;
3548
}
3649

37-
function statMode(outputPath) {
38-
return masked(fs.lstatSync(outputPath).mode);
50+
function createdMode(outputPath) {
51+
var mode = masked(fs.lstatSync(outputPath).mode);
52+
log.found(mode);
53+
return mode;
3954
}
4055

41-
function applyUmask(mode) {
56+
function expectedMode(mode) {
4257
if (typeof mode !== 'number') {
4358
mode = parseInt(mode, 8);
4459
}
4560

46-
return mode & ~process.umask();
61+
log.expected(mode);
62+
return mode;
63+
}
64+
65+
function expectedDefaultMode() {
66+
// Set to use to "get" it
67+
var current = process.umask(0);
68+
// Then set it back for the next test
69+
process.umask(current);
70+
71+
var mode = (DEFAULT_DIR_MODE & ~current);
72+
log.expected(mode);
73+
return mode;
4774
}
4875

4976
beforeEach(cleanup);
@@ -64,7 +91,7 @@ describe('mkdirp', function () {
6491
it('makes a single directory', function (done) {
6592
mkdirp(outputDirpath, function (err) {
6693
expect(err).toBeFalsy();
67-
expect(statMode(outputDirpath)).toBeDefined();
94+
expect(createdMode(outputDirpath)).toBeDefined();
6895

6996
done();
7097
});
@@ -76,11 +103,9 @@ describe('mkdirp', function () {
76103
return;
77104
}
78105

79-
var defaultMode = applyUmask(DEFAULT_DIR_MODE);
80-
81106
mkdirp(outputDirpath, function (err) {
82107
expect(err).toBeFalsy();
83-
expect(statMode(outputDirpath)).toEqual(defaultMode);
108+
expect(createdMode(outputDirpath)).toEqual(expectedDefaultMode());
84109

85110
done();
86111
});
@@ -89,7 +114,7 @@ describe('mkdirp', function () {
89114
it('makes multiple directories', function (done) {
90115
mkdirp(outputNestedDirpath, function (err) {
91116
expect(err).toBeFalsy();
92-
expect(statMode(outputNestedDirpath)).toBeDefined();
117+
expect(createdMode(outputNestedDirpath)).toBeDefined();
93118

94119
done();
95120
});
@@ -101,27 +126,57 @@ describe('mkdirp', function () {
101126
return;
102127
}
103128

104-
var defaultMode = applyUmask(DEFAULT_DIR_MODE);
105-
106129
mkdirp(outputNestedDirpath, function (err) {
107130
expect(err).toBeFalsy();
108-
expect(statMode(outputNestedDirpath)).toEqual(defaultMode);
131+
expect(createdMode(outputNestedDirpath)).toEqual(expectedDefaultMode());
132+
133+
done();
134+
});
135+
});
136+
137+
it('makes directory with custom mode as string', function (done) {
138+
if (isWindows) {
139+
this.skip();
140+
return;
141+
}
142+
143+
var mode = '777';
144+
145+
mkdirp(outputDirpath, mode, function (err) {
146+
expect(err).toBeFalsy();
147+
expect(createdMode(outputDirpath)).toEqual(expectedMode(mode));
109148

110149
done();
111150
});
112151
});
113152

114-
it('makes directory with custom mode', function (done) {
153+
it('makes directory with custom mode as octal', function (done) {
115154
if (isWindows) {
116155
this.skip();
117156
return;
118157
}
119158

120-
var mode = applyUmask('700');
159+
var mode = parseInt('777', 8);
121160

122161
mkdirp(outputDirpath, mode, function (err) {
123162
expect(err).toBeFalsy();
124-
expect(statMode(outputDirpath)).toEqual(mode);
163+
expect(createdMode(outputDirpath)).toEqual(expectedMode(mode));
164+
165+
done();
166+
});
167+
});
168+
169+
it('does not mask a custom mode', function (done) {
170+
if (isWindows) {
171+
this.skip();
172+
return;
173+
}
174+
175+
var mode = parseInt('777', 8);
176+
177+
mkdirp(outputDirpath, mode, function (err) {
178+
expect(err).toBeFalsy();
179+
expect(createdMode(outputDirpath)).toEqual(mode);
125180

126181
done();
127182
});
@@ -133,11 +188,11 @@ describe('mkdirp', function () {
133188
return;
134189
}
135190

136-
var mode = applyUmask('2700');
191+
var mode = '2700';
137192

138193
mkdirp(outputDirpath, mode, function (err) {
139194
expect(err).toBeFalsy();
140-
expect(statMode(outputDirpath)).toEqual(mode);
195+
expect(createdMode(outputDirpath)).toEqual(expectedMode(mode));
141196

142197
done();
143198
});
@@ -149,14 +204,14 @@ describe('mkdirp', function () {
149204
return;
150205
}
151206

152-
var mode = applyUmask('700');
207+
var mode = '777';
153208

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

157212
mkdirp(outputDirpath, function (err2) {
158213
expect(err2).toBeFalsy();
159-
expect(statMode(outputDirpath)).toEqual(mode);
214+
expect(createdMode(outputDirpath)).toEqual(expectedMode(mode));
160215

161216
done();
162217
});
@@ -169,11 +224,11 @@ describe('mkdirp', function () {
169224
return;
170225
}
171226

172-
var mode = applyUmask('700');
227+
var mode = '777';
173228

174229
mkdirp(outputNestedDirpath, mode, function (err) {
175230
expect(err).toBeFalsy();
176-
expect(statMode(outputNestedDirpath)).toEqual(mode);
231+
expect(createdMode(outputNestedDirpath)).toEqual(expectedMode(mode));
177232

178233
done();
179234
});
@@ -186,13 +241,13 @@ describe('mkdirp', function () {
186241
}
187242

188243
var intermediateDirpath = path.dirname(outputNestedDirpath);
189-
var mode = applyUmask('700');
190-
var defaultMode = applyUmask(DEFAULT_DIR_MODE);
244+
var mode = '777';
191245

192246
mkdirp(outputNestedDirpath, mode, function (err) {
193247
expect(err).toBeFalsy();
194-
expect(statMode(outputDirpath)).toEqual(defaultMode);
195-
expect(statMode(intermediateDirpath)).toEqual(defaultMode);
248+
expect(createdMode(outputDirpath)).toEqual(expectedDefaultMode());
249+
expect(createdMode(intermediateDirpath)).toEqual(expectedDefaultMode());
250+
expect(createdMode(outputNestedDirpath)).toEqual(expectedMode(mode));
196251

197252
done();
198253
});
@@ -204,16 +259,15 @@ describe('mkdirp', function () {
204259
return;
205260
}
206261

207-
var mode = applyUmask('700');
208-
var defaultMode = applyUmask(DEFAULT_DIR_MODE);
262+
var mode = '777';
209263

210264
mkdirp(outputDirpath, function (err) {
211265
expect(err).toBeFalsy();
212-
expect(statMode(outputDirpath)).toEqual(defaultMode);
266+
expect(createdMode(outputDirpath)).toEqual(expectedDefaultMode());
213267

214268
mkdirp(outputDirpath, mode, function (err2) {
215269
expect(err2).toBeFalsy();
216-
expect(statMode(outputDirpath)).toEqual(mode);
270+
expect(createdMode(outputDirpath)).toEqual(expectedMode(mode));
217271

218272
done();
219273
});
@@ -243,19 +297,20 @@ describe('mkdirp', function () {
243297
return;
244298
}
245299

246-
var mode = applyUmask('700');
300+
var mode = '777';
247301

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

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

254-
var expectedMode = statMode(outputNestedPath);
308+
var existingMode = createdMode(outputNestedPath);
309+
expect(existingMode).not.toEqual(mode);
255310

256311
mkdirp(outputNestedPath, mode, function (err3) {
257312
expect(err3).toBeDefined();
258-
expect(statMode(outputNestedPath)).toEqual(expectedMode);
313+
expect(createdMode(outputNestedPath)).toEqual(existingMode);
259314

260315
done();
261316
});
@@ -300,7 +355,7 @@ describe('mkdirp', function () {
300355
return;
301356
}
302357

303-
var mode = applyUmask('700');
358+
var mode = '777';
304359

305360
mkdirp(outputDirpath, mode, function (err) {
306361
expect(err).toBeFalsy();
@@ -315,4 +370,25 @@ describe('mkdirp', function () {
315370
});
316371
});
317372
});
373+
}
374+
375+
describe('mkdirp', suite);
376+
377+
describe('mkdirp with umask', function() {
378+
379+
var startingUmask;
380+
before(function(done) {
381+
startingUmask = process.umask(parseInt('066', 8));
382+
383+
done();
384+
});
385+
386+
after(function(done) {
387+
process.umask(startingUmask);
388+
389+
done();
390+
})
391+
392+
// Initialize the normal suite
393+
suite();
318394
});

0 commit comments

Comments
 (0)