Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 2fe88d2

Browse filesBrowse files
joyeecheungtargos
authored andcommitted
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 <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Backport-PR-URL: #21172
1 parent fc2956d commit 2fe88d2
Copy full SHA for 2fe88d2

File tree

Expand file treeCollapse file tree

12 files changed

+313
-116
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

12 files changed

+313
-116
lines changed
Open diff view settings
Collapse file

‎lib/fs.js‎

Copy file name to clipboardExpand all lines: lib/fs.js
+31-32Lines 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);
Collapse file

‎lib/internal/fs/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/promises.js
+6-13Lines 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,
@@ -33,6 +31,7 @@ const {
3331
validatePath
3432
} = require('internal/fs/utils');
3533
const {
34+
validateAndMaskMode,
3635
validateInteger,
3736
validateUint32
3837
} = require('internal/validators');
@@ -190,10 +189,9 @@ async function copyFile(src, dest, flags) {
190189
// Note that unlike fs.open() which uses numeric file descriptors,
191190
// fsPromises.open() uses the fs.FileHandle class.
192191
async function open(path, flags, mode) {
193-
mode = modeNum(mode, 0o666);
194192
path = getPathFromURL(path);
195193
validatePath(path);
196-
validateUint32(mode, 'mode');
194+
mode = validateAndMaskMode(mode, 'mode', 0o666);
197195
return new FileHandle(
198196
await binding.openFileHandle(pathModule.toNamespacedPath(path),
199197
stringToFlags(flags),
@@ -286,10 +284,9 @@ async function fsync(handle) {
286284
}
287285

288286
async function mkdir(path, mode) {
289-
mode = modeNum(mode, 0o777);
290287
path = getPathFromURL(path);
291288
validatePath(path);
292-
validateUint32(mode, 'mode');
289+
mode = validateAndMaskMode(mode, 'mode', 0o777);
293290
return binding.mkdir(pathModule.toNamespacedPath(path), mode, kUsePromises);
294291
}
295292

@@ -360,19 +357,15 @@ async function unlink(path) {
360357
}
361358

362359
async function fchmod(handle, mode) {
363-
mode = modeNum(mode);
364360
validateFileHandle(handle);
365-
validateUint32(mode, 'mode');
366-
if (mode > 0o777)
367-
throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
361+
mode = validateAndMaskMode(mode, 'mode');
368362
return binding.fchmod(handle.fd, mode, kUsePromises);
369363
}
370364

371365
async function chmod(path, mode) {
372366
path = getPathFromURL(path);
373367
validatePath(path);
374-
mode = modeNum(mode);
375-
validateUint32(mode, 'mode');
368+
mode = validateAndMaskMode(mode, 'mode');
376369
return binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises);
377370
}
378371

Collapse file

‎lib/internal/fs/utils.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/utils.js
-16Lines 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'),
Collapse file

‎lib/internal/validators.js‎

Copy file name to clipboardExpand all lines: lib/internal/validators.js
+36Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.