FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: fix flag and mode validation · nodejs/node@da217d0 · GitHub

/ node Public

Commit da217d0

Browse files
jasnell
committed
fs: fix flag and mode validation
The `flag` and `mode` options were not being validated correctly. Signed-off-by: James M Snell <jasnell@gmail.com> Fixes: #37430 PR-URL: #37480 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent 7c63bc6 commit da217d0

9 files changed

Lines changed: 67 additions & 43 deletions

File tree

‎lib/fs.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ function readFile(path, options, callback) {
347347
return;
348348
}
349349

350-
const flagsNumber = stringToFlags(options.flag);
350+
const flagsNumber = stringToFlags(options.flag, 'options.flag');
351351
path = getValidatedPath(path);
352352

353353
const req = new FSReqCallback();
@@ -1284,6 +1284,7 @@ function fchmodSync(fd, mode) {
12841284

12851285
function lchmod(path, mode, callback) {
12861286
callback = maybeCallback(callback);
1287+
mode = parseFileMode(mode, 'mode');
12871288
fs.open(path, O_WRONLY | O_SYMLINK, (err, fd) => {
12881289
if (err) {
12891290
callback(err);

‎lib/internal/fs/utils.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,8 +526,9 @@ function getStatsFromBinding(stats, offset = 0) {
526526
);
527527
}
528528

529-
function stringToFlags(flags) {
529+
function stringToFlags(flags, name = 'flags') {
530530
if (typeof flags === 'number') {
531+
validateInt32(flags, name);
531532
return flags;
532533
}
533534

‎lib/internal/validators.js‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,26 +56,16 @@ const modeDesc = 'must be a 32-bit unsigned integer or an octal string';
5656
* @returns {number}
5757
*/
5858
function parseFileMode(value, name, def) {
59-
if (value == null && def !== undefined) {
60-
return def;
61-
}
62-
63-
if (isUint32(value)) {
64-
return value;
65-
}
66-
67-
if (typeof value === 'number') {
68-
validateInt32(value, name, 0, 2 ** 32 - 1);
69-
}
70-
59+
value ??= def;
7160
if (typeof value === 'string') {
7261
if (!RegExpPrototypeTest(octalReg, value)) {
7362
throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc);
7463
}
75-
return NumberParseInt(value, 8);
64+
value = NumberParseInt(value, 8);
7665
}
7766

78-
throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc);
67+
validateInt32(value, name, 0, 2 ** 32 - 1);
68+
return value;
7969
}
8070

8171
const validateInteger = hideStackFrames(

‎test/parallel/test-file-validate-mode-flag.js‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
'use strict';
2+
3+
// Checks for crash regression: https://github.com/nodejs/node/issues/37430
4+
5+
const common = require('../common');
6+
const assert = require('assert');
7+
const {
8+
open,
9+
openSync,
10+
promises: {
11+
open: openPromise,
12+
},
13+
} = require('fs');
14+
15+
// These should throw, not crash.
16+
17+
assert.throws(() => open(__filename, 2176057344, common.mustNotCall()), {
18+
code: 'ERR_OUT_OF_RANGE'
19+
});
20+
21+
assert.throws(() => open(__filename, 0, 2176057344, common.mustNotCall()), {
22+
code: 'ERR_OUT_OF_RANGE'
23+
});
24+
25+
assert.throws(() => openSync(__filename, 2176057344), {
26+
code: 'ERR_OUT_OF_RANGE'
27+
});
28+
29+
assert.throws(() => openSync(__filename, 0, 2176057344), {
30+
code: 'ERR_OUT_OF_RANGE'
31+
});
32+
33+
assert.rejects(openPromise(__filename, 2176057344), {
34+
code: 'ERR_OUT_OF_RANGE'
35+
});
36+
37+
assert.rejects(openPromise(__filename, 0, 2176057344), {
38+
code: 'ERR_OUT_OF_RANGE'
39+
});

‎test/parallel/test-fs-chmod.js‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,7 @@ fs.open(file2, 'w', common.mustSucceed((fd) => {
106106
assert.throws(
107107
() => fs.fchmod(fd, {}),
108108
{
109-
code: 'ERR_INVALID_ARG_VALUE',
110-
name: 'TypeError',
111-
message: 'The argument \'mode\' must be a 32-bit unsigned integer ' +
112-
'or an octal string. Received {}'
109+
code: 'ERR_INVALID_ARG_TYPE',
113110
}
114111
);
115112

‎test/parallel/test-fs-fchmod.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
'use strict';
22
const common = require('../common');
33
const assert = require('assert');
4-
const util = require('util');
54
const fs = require('fs');
65

76
// This test ensures that input for fchmod is valid, testing for valid
@@ -20,17 +19,18 @@ const fs = require('fs');
2019
});
2120

2221

23-
[false, null, undefined, {}, [], '', '123x'].forEach((input) => {
22+
[false, null, {}, []].forEach((input) => {
2423
const errObj = {
25-
code: 'ERR_INVALID_ARG_VALUE',
26-
name: 'TypeError',
27-
message: 'The argument \'mode\' must be a 32-bit unsigned integer or an ' +
28-
`octal string. Received ${util.inspect(input)}`
24+
code: 'ERR_INVALID_ARG_TYPE',
2925
};
3026
assert.throws(() => fs.fchmod(1, input), errObj);
3127
assert.throws(() => fs.fchmodSync(1, input), errObj);
3228
});
3329

30+
assert.throws(() => fs.fchmod(1, '123x'), {
31+
code: 'ERR_INVALID_ARG_VALUE'
32+
});
33+
3434
[-1, 2 ** 32].forEach((input) => {
3535
const errObj = {
3636
code: 'ERR_OUT_OF_RANGE',

‎test/parallel/test-fs-lchmod.js‎

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

33
const common = require('../common');
44
const assert = require('assert');
5-
const util = require('util');
65
const fs = require('fs');
76
const { promises } = fs;
87
const f = __filename;
@@ -38,18 +37,22 @@ assert.throws(() => fs.lchmod(f, {}), { code: 'ERR_INVALID_CALLBACK' });
3837
});
3938

4039
// Check mode
41-
[false, null, undefined, {}, [], '', '123x'].forEach((input) => {
40+
[false, null, {}, []].forEach((input) => {
4241
const errObj = {
43-
code: 'ERR_INVALID_ARG_VALUE',
44-
name: 'TypeError',
45-
message: 'The argument \'mode\' must be a 32-bit unsigned integer or an ' +
46-
`octal string. Received ${util.inspect(input)}`
42+
code: 'ERR_INVALID_ARG_TYPE',
4743
};
4844

4945
assert.rejects(promises.lchmod(f, input, () => {}), errObj);
5046
assert.throws(() => fs.lchmodSync(f, input), errObj);
5147
});
5248

49+
assert.throws(() => fs.lchmod(f, '123x', common.mustNotCall()), {
50+
code: 'ERR_INVALID_ARG_VALUE'
51+
});
52+
assert.throws(() => fs.lchmodSync(f, '123x'), {
53+
code: 'ERR_INVALID_ARG_VALUE'
54+
});
55+
5356
[-1, 2 ** 32].forEach((input) => {
5457
const errObj = {
5558
code: 'ERR_OUT_OF_RANGE',

‎test/parallel/test-fs-open.js‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,22 +102,19 @@ for (const extra of [[], ['r'], ['r', 0], ['r', 0, 'bad callback']]) {
102102
assert.throws(
103103
() => fs.open(__filename, 'r', mode, common.mustNotCall()),
104104
{
105-
message: /'mode' must be a 32-bit/,
106-
code: 'ERR_INVALID_ARG_VALUE'
105+
code: 'ERR_INVALID_ARG_TYPE'
107106
}
108107
);
109108
assert.throws(
110109
() => fs.openSync(__filename, 'r', mode, common.mustNotCall()),
111110
{
112-
message: /'mode' must be a 32-bit/,
113-
code: 'ERR_INVALID_ARG_VALUE'
111+
code: 'ERR_INVALID_ARG_TYPE'
114112
}
115113
);
116114
assert.rejects(
117115
fs.promises.open(__filename, 'r', mode),
118116
{
119-
message: /'mode' must be a 32-bit/,
120-
code: 'ERR_INVALID_ARG_VALUE'
117+
code: 'ERR_INVALID_ARG_TYPE'
121118
}
122119
);
123120
});

‎test/parallel/test-process-umask.js‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,13 @@ assert.strictEqual(process.umask(), old);
5353
assert.throws(() => {
5454
process.umask({});
5555
}, {
56-
code: 'ERR_INVALID_ARG_VALUE',
57-
message: 'The argument \'mask\' must be a 32-bit unsigned integer ' +
58-
'or an octal string. Received {}'
56+
code: 'ERR_INVALID_ARG_TYPE',
5957
});
6058

6159
['123x', 'abc', '999'].forEach((value) => {
6260
assert.throws(() => {
6361
process.umask(value);
6462
}, {
6563
code: 'ERR_INVALID_ARG_VALUE',
66-
message: 'The argument \'mask\' must be a 32-bit unsigned integer ' +
67-
`or an octal string. Received '${value}'`
6864
});
6965
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL