Skip to content

Commit 7073282

Browse files
joyeecheungtargos
joyeecheung
authored and
targos
committed
lib: mask mode_t type of arguments with 0o777
- Introduce the `validateAndMaskMode` validator that validates `mode_t` arguments and mask them with 0o777 if they are 32-bit unsigned integer or octal string to be more consistent with POSIX APIs. - Use the validator in fs APIs and process.umask for consistency. - Add tests for 32-bit unsigned modes larger than 0o777. PR-URL: #20636 Fixes: #20498 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Backport-PR-URL: #21172
1 parent 0d4aa10 commit 7073282

File tree

12 files changed

+313
-116
lines changed

12 files changed

+313
-116
lines changed

‎lib/fs.js‎

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ const internalUtil = require('internal/util');
6565
const {
6666
copyObject,
6767
getOptions,
68-
modeNum,
6968
nullCheck,
7069
preprocessSymlinkDestination,
7170
Stats,
@@ -85,6 +84,7 @@ const {
8584
} = require('internal/constants');
8685
const {
8786
isUint32,
87+
validateAndMaskMode,
8888
validateInteger,
8989
validateUint32
9090
} = require('internal/validators');
@@ -549,32 +549,36 @@ fs.closeSync = function(fd) {
549549
handleErrorFromBinding(ctx);
550550
};
551551

552-
fs.open = function(path, flags, mode, callback_) {
553-
var callback = makeCallback(arguments[arguments.length - 1]);
554-
mode = modeNum(mode, 0o666);
555-
552+
fs.open = function(path, flags, mode, callback) {
556553
path = getPathFromURL(path);
557554
validatePath(path);
558-
validateUint32(mode, 'mode');
555+
const flagsNumber = stringToFlags(flags);
556+
if (arguments.length < 4) {
557+
callback = makeCallback(mode);
558+
mode = 0o666;
559+
} else {
560+
mode = validateAndMaskMode(mode, 'mode', 0o666);
561+
callback = makeCallback(callback);
562+
}
559563

560564
const req = new FSReqWrap();
561565
req.oncomplete = callback;
562566

563567
binding.open(pathModule.toNamespacedPath(path),
564-
stringToFlags(flags),
568+
flagsNumber,
565569
mode,
566570
req);
567571
};
568572

569573
fs.openSync = function(path, flags, mode) {
570-
mode = modeNum(mode, 0o666);
571574
path = getPathFromURL(path);
572575
validatePath(path);
573-
validateUint32(mode, 'mode');
576+
const flagsNumber = stringToFlags(flags);
577+
mode = validateAndMaskMode(mode, 'mode', 0o666);
574578

575579
const ctx = { path };
576580
const result = binding.open(pathModule.toNamespacedPath(path),
577-
stringToFlags(flags), mode,
581+
flagsNumber, mode,
578582
undefined, ctx);
579583
handleErrorFromBinding(ctx);
580584
return result;
@@ -849,12 +853,16 @@ fs.fsyncSync = function(fd) {
849853
};
850854

851855
fs.mkdir = function(path, mode, callback) {
852-
if (typeof mode === 'function') callback = mode;
853-
callback = makeCallback(callback);
854856
path = getPathFromURL(path);
855857
validatePath(path);
856-
mode = modeNum(mode, 0o777);
857-
validateUint32(mode, 'mode');
858+
859+
if (arguments.length < 3) {
860+
callback = makeCallback(mode);
861+
mode = 0o777;
862+
} else {
863+
callback = makeCallback(callback);
864+
mode = validateAndMaskMode(mode, 'mode', 0o777);
865+
}
858866

859867
const req = new FSReqWrap();
860868
req.oncomplete = callback;
@@ -864,8 +872,7 @@ fs.mkdir = function(path, mode, callback) {
864872
fs.mkdirSync = function(path, mode) {
865873
path = getPathFromURL(path);
866874
validatePath(path);
867-
mode = modeNum(mode, 0o777);
868-
validateUint32(mode, 'mode');
875+
mode = validateAndMaskMode(mode, 'mode', 0o777);
869876
const ctx = { path };
870877
binding.mkdir(pathModule.toNamespacedPath(path), mode, undefined, ctx);
871878
handleErrorFromBinding(ctx);
@@ -1047,25 +1054,18 @@ fs.unlinkSync = function(path) {
10471054
};
10481055

10491056
fs.fchmod = function(fd, mode, callback) {
1050-
mode = modeNum(mode);
10511057
validateUint32(fd, 'fd');
1052-
validateUint32(mode, 'mode');
1053-
// Values for mode < 0 are already checked via the validateUint32 function
1054-
if (mode > 0o777)
1055-
throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
1058+
mode = validateAndMaskMode(mode, 'mode');
1059+
callback = makeCallback(callback);
10561060

10571061
const req = new FSReqWrap();
1058-
req.oncomplete = makeCallback(callback);
1062+
req.oncomplete = callback;
10591063
binding.fchmod(fd, mode, req);
10601064
};
10611065

10621066
fs.fchmodSync = function(fd, mode) {
1063-
mode = modeNum(mode);
10641067
validateUint32(fd, 'fd');
1065-
validateUint32(mode, 'mode');
1066-
// Values for mode < 0 are already checked via the validateUint32 function
1067-
if (mode > 0o777)
1068-
throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
1068+
mode = validateAndMaskMode(mode, 'mode');
10691069
const ctx = {};
10701070
binding.fchmod(fd, mode, undefined, ctx);
10711071
handleErrorFromBinding(ctx);
@@ -1106,11 +1106,10 @@ if (O_SYMLINK !== undefined) {
11061106

11071107

11081108
fs.chmod = function(path, mode, callback) {
1109-
callback = makeCallback(callback);
11101109
path = getPathFromURL(path);
11111110
validatePath(path);
1112-
mode = modeNum(mode);
1113-
validateUint32(mode, 'mode');
1111+
mode = validateAndMaskMode(mode, 'mode');
1112+
callback = makeCallback(callback);
11141113

11151114
const req = new FSReqWrap();
11161115
req.oncomplete = callback;
@@ -1120,8 +1119,8 @@ fs.chmod = function(path, mode, callback) {
11201119
fs.chmodSync = function(path, mode) {
11211120
path = getPathFromURL(path);
11221121
validatePath(path);
1123-
mode = modeNum(mode);
1124-
validateUint32(mode, 'mode');
1122+
mode = validateAndMaskMode(mode, 'mode');
1123+
11251124
const ctx = { path };
11261125
binding.chmod(pathModule.toNamespacedPath(path), mode, undefined, ctx);
11271126
handleErrorFromBinding(ctx);

‎lib/internal/fs/promises.js‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,14 @@ const { Buffer, kMaxLength } = require('buffer');
1212
const {
1313
ERR_FS_FILE_TOO_LARGE,
1414
ERR_INVALID_ARG_TYPE,
15-
ERR_METHOD_NOT_IMPLEMENTED,
16-
ERR_OUT_OF_RANGE
15+
ERR_METHOD_NOT_IMPLEMENTED
1716
} = require('internal/errors').codes;
1817
const { getPathFromURL } = require('internal/url');
1918
const { isUint8Array } = require('internal/util/types');
2019
const {
2120
copyObject,
2221
getOptions,
2322
getStatsFromBinding,
24-
modeNum,
2523
nullCheck,
2624
preprocessSymlinkDestination,
2725
stringToFlags,
@@ -34,6 +32,7 @@ const {
3432
} = require('internal/fs/utils');
3533
const {
3634
isUint32,
35+
validateAndMaskMode,
3736
validateInteger,
3837
validateUint32
3938
} = require('internal/validators');
@@ -191,10 +190,9 @@ async function copyFile(src, dest, flags) {
191190
// Note that unlike fs.open() which uses numeric file descriptors,
192191
// fsPromises.open() uses the fs.FileHandle class.
193192
async function open(path, flags, mode) {
194-
mode = modeNum(mode, 0o666);
195193
path = getPathFromURL(path);
196194
validatePath(path);
197-
validateUint32(mode, 'mode');
195+
mode = validateAndMaskMode(mode, 'mode', 0o666);
198196
return new FileHandle(
199197
await binding.openFileHandle(pathModule.toNamespacedPath(path),
200198
stringToFlags(flags),
@@ -287,10 +285,9 @@ async function fsync(handle) {
287285
}
288286

289287
async function mkdir(path, mode) {
290-
mode = modeNum(mode, 0o777);
291288
path = getPathFromURL(path);
292289
validatePath(path);
293-
validateUint32(mode, 'mode');
290+
mode = validateAndMaskMode(mode, 'mode', 0o777);
294291
return binding.mkdir(pathModule.toNamespacedPath(path), mode, kUsePromises);
295292
}
296293

@@ -361,19 +358,15 @@ async function unlink(path) {
361358
}
362359

363360
async function fchmod(handle, mode) {
364-
mode = modeNum(mode);
365361
validateFileHandle(handle);
366-
validateUint32(mode, 'mode');
367-
if (mode > 0o777)
368-
throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
362+
mode = validateAndMaskMode(mode, 'mode');
369363
return binding.fchmod(handle.fd, mode, kUsePromises);
370364
}
371365

372366
async function chmod(path, mode) {
373367
path = getPathFromURL(path);
374368
validatePath(path);
375-
mode = modeNum(mode);
376-
validateUint32(mode, 'mode');
369+
mode = validateAndMaskMode(mode, 'mode');
377370
return binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises);
378371
}
379372

‎lib/internal/fs/utils.js‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -70,21 +70,6 @@ function getOptions(options, defaultOptions) {
7070
return options;
7171
}
7272

73-
function modeNum(m, def) {
74-
if (typeof m === 'number')
75-
return m;
76-
if (typeof m === 'string') {
77-
const parsed = parseInt(m, 8);
78-
if (Number.isNaN(parsed))
79-
return m;
80-
return parsed;
81-
}
82-
// TODO(BridgeAR): Only return `def` in case `m == null`
83-
if (def !== undefined)
84-
return def;
85-
return m;
86-
}
87-
8873
// Check if the path contains null types if it is a string nor Uint8Array,
8974
// otherwise return silently.
9075
function nullCheck(path, propName, throwError = true) {
@@ -391,7 +376,6 @@ module.exports = {
391376
assertEncoding,
392377
copyObject,
393378
getOptions,
394-
modeNum,
395379
nullCheck,
396380
preprocessSymlinkDestination,
397381
realpathCacheKey: Symbol('realpathCacheKey'),

‎lib/internal/validators.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const {
44
ERR_INVALID_ARG_TYPE,
5+
ERR_INVALID_ARG_VALUE,
56
ERR_OUT_OF_RANGE
67
} = require('internal/errors').codes;
78

@@ -13,6 +14,40 @@ function isUint32(value) {
1314
return value === (value >>> 0);
1415
}
1516

17+
const octalReg = /^[0-7]+$/;
18+
const modeDesc = 'must be a 32-bit unsigned integer or an octal string';
19+
// Validator for mode_t (the S_* constants). Valid numbers or octal strings
20+
// will be masked with 0o777 to be consistent with the behavior in POSIX APIs.
21+
function validateAndMaskMode(value, name, def) {
22+
if (isUint32(value)) {
23+
return value & 0o777;
24+
}
25+
26+
if (typeof value === 'number') {
27+
if (!Number.isInteger(value)) {
28+
throw new ERR_OUT_OF_RANGE(name, 'an integer', value);
29+
} else {
30+
// 2 ** 32 === 4294967296
31+
throw new ERR_OUT_OF_RANGE(name, '>= 0 && < 4294967296', value);
32+
}
33+
}
34+
35+
if (typeof value === 'string') {
36+
if (!octalReg.test(value)) {
37+
throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc);
38+
}
39+
const parsed = parseInt(value, 8);
40+
return parsed & 0o777;
41+
}
42+
43+
// TODO(BridgeAR): Only return `def` in case `value == null`
44+
if (def !== undefined) {
45+
return def;
46+
}
47+
48+
throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc);
49+
}
50+
1651
function validateInteger(value, name) {
1752
let err;
1853

@@ -67,6 +102,7 @@ function validateUint32(value, name, positive) {
67102
module.exports = {
68103
isInt32,
69104
isUint32,
105+
validateAndMaskMode,
70106
validateInteger,
71107
validateInt32,
72108
validateUint32

0 commit comments

Comments
 (0)