1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

lib: mask mode_t type of arguments with 0o777

Browse Source 
- 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: https://github.com/nodejs/node/pull/20636
Fixes: https://github.com/nodejs/node/issues/20498
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This commit is contained in:
Joyee Cheung 2018-05-09 22:44:44 +08:00
parent 0d9500daed
commit a18e130e59
No known key found for this signature in database
GPG Key ID: F586868AAD831D0C
13 changed files with 323 additions and 141 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
lib/fs.js
View File

@ -52,7 +52,6 @@ const internalUtil = require('internal/util');
const {
copyObject,
getOptions,
modeNum,
nullCheck,
preprocessSymlinkDestination,
Stats,
@ -71,6 +70,7 @@ const {
} = require('internal/constants');
const {
isUint32,
validateAndMaskMode,
validateInt32,
validateUint32
} = require('internal/validators');
@ -531,32 +531,36 @@ fs.closeSync = function(fd) {
handleErrorFromBinding(ctx);
};
fs.open = function(path, flags, mode, callback_) {
var callback = makeCallback(arguments[arguments.length - 1]);
mode = modeNum(mode, 0o666);
fs.open = function(path, flags, mode, callback) {
path = getPathFromURL(path);
validatePath(path);
validateUint32(mode, 'mode');
const flagsNumber = stringToFlags(flags);
if (arguments.length < 4) {
callback = makeCallback(mode);
mode = 0o666;
} else {
mode = validateAndMaskMode(mode, 'mode', 0o666);
callback = makeCallback(callback);
}
const req = new FSReqWrap();
req.oncomplete = callback;
binding.open(pathModule.toNamespacedPath(path),
stringToFlags(flags),
flagsNumber,
mode,
req);
};
fs.openSync = function(path, flags, mode) {
mode = modeNum(mode, 0o666);
path = getPathFromURL(path);
validatePath(path);
validateUint32(mode, 'mode');
const flagsNumber = stringToFlags(flags);
mode = validateAndMaskMode(mode, 'mode', 0o666);
const ctx = { path };
const result = binding.open(pathModule.toNamespacedPath(path),
stringToFlags(flags), mode,
flagsNumber, mode,
undefined, ctx);
handleErrorFromBinding(ctx);
return result;
@ -834,12 +838,16 @@ fs.fsyncSync = function(fd) {
};
fs.mkdir = function(path, mode, callback) {
if (typeof mode === 'function') callback = mode;
callback = makeCallback(callback);
path = getPathFromURL(path);
validatePath(path);
mode = modeNum(mode, 0o777);
validateUint32(mode, 'mode');
if (arguments.length < 3) {
callback = makeCallback(mode);
mode = 0o777;
} else {
callback = makeCallback(callback);
mode = validateAndMaskMode(mode, 'mode', 0o777);
}
const req = new FSReqWrap();
req.oncomplete = callback;
@ -849,8 +857,7 @@ fs.mkdir = function(path, mode, callback) {
fs.mkdirSync = function(path, mode) {
path = getPathFromURL(path);
validatePath(path);
mode = modeNum(mode, 0o777);
validateUint32(mode, 'mode');
mode = validateAndMaskMode(mode, 'mode', 0o777);
const ctx = { path };
binding.mkdir(pathModule.toNamespacedPath(path), mode, undefined, ctx);
handleErrorFromBinding(ctx);
@ -1032,25 +1039,18 @@ fs.unlinkSync = function(path) {
};
fs.fchmod = function(fd, mode, callback) {
mode = modeNum(mode);
validateUint32(fd, 'fd');
validateUint32(mode, 'mode');
// Values for mode < 0 are already checked via the validateUint32 function
if (mode > 0o777)
throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
mode = validateAndMaskMode(mode, 'mode');
callback = makeCallback(callback);
const req = new FSReqWrap();
req.oncomplete = makeCallback(callback);
req.oncomplete = callback;
binding.fchmod(fd, mode, req);
};
fs.fchmodSync = function(fd, mode) {
mode = modeNum(mode);
validateUint32(fd, 'fd');
validateUint32(mode, 'mode');
// Values for mode < 0 are already checked via the validateUint32 function
if (mode > 0o777)
throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
mode = validateAndMaskMode(mode, 'mode');
const ctx = {};
binding.fchmod(fd, mode, undefined, ctx);
handleErrorFromBinding(ctx);
@ -1091,11 +1091,10 @@ if (constants.O_SYMLINK !== undefined) {
fs.chmod = function(path, mode, callback) {
callback = makeCallback(callback);
path = getPathFromURL(path);
validatePath(path);
mode = modeNum(mode);
validateUint32(mode, 'mode');
mode = validateAndMaskMode(mode, 'mode');
callback = makeCallback(callback);
const req = new FSReqWrap();
req.oncomplete = callback;
@ -1105,8 +1104,8 @@ fs.chmod = function(path, mode, callback) {
fs.chmodSync = function(path, mode) {
path = getPathFromURL(path);
validatePath(path);
mode = modeNum(mode);
validateUint32(mode, 'mode');
mode = validateAndMaskMode(mode, 'mode');
const ctx = { path };
binding.chmod(pathModule.toNamespacedPath(path), mode, undefined, ctx);
handleErrorFromBinding(ctx);

19
lib/internal/fs/promises.js
View File

@ -12,8 +12,7 @@ const { Buffer, kMaxLength } = require('buffer');
const {
ERR_FS_FILE_TOO_LARGE,
ERR_INVALID_ARG_TYPE,
ERR_METHOD_NOT_IMPLEMENTED,
ERR_OUT_OF_RANGE
ERR_METHOD_NOT_IMPLEMENTED
} = require('internal/errors').codes;
const { getPathFromURL } = require('internal/url');
const { isUint8Array } = require('internal/util/types');
@ -21,7 +20,6 @@ const {
copyObject,
getOptions,
getStatsFromBinding,
modeNum,
nullCheck,
preprocessSymlinkDestination,
stringToFlags,
@ -34,6 +32,7 @@ const {
} = require('internal/fs/utils');
const {
isUint32,
validateAndMaskMode,
validateInt32,
validateUint32
} = require('internal/validators');
@ -191,10 +190,9 @@ async function copyFile(src, dest, flags) {
// Note that unlike fs.open() which uses numeric file descriptors,
// fsPromises.open() uses the fs.FileHandle class.
async function open(path, flags, mode) {
mode = modeNum(mode, 0o666);
path = getPathFromURL(path);
validatePath(path);
validateUint32(mode, 'mode');
mode = validateAndMaskMode(mode, 'mode', 0o666);
return new FileHandle(
await binding.openFileHandle(pathModule.toNamespacedPath(path),
stringToFlags(flags),
@ -287,10 +285,9 @@ async function fsync(handle) {
}
async function mkdir(path, mode) {
mode = modeNum(mode, 0o777);
path = getPathFromURL(path);
validatePath(path);
validateUint32(mode, 'mode');
mode = validateAndMaskMode(mode, 'mode', 0o777);
return binding.mkdir(pathModule.toNamespacedPath(path), mode, kUsePromises);
}
@ -361,19 +358,15 @@ async function unlink(path) {
}
async function fchmod(handle, mode) {
mode = modeNum(mode);
validateFileHandle(handle);
validateUint32(mode, 'mode');
if (mode > 0o777)
throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
mode = validateAndMaskMode(mode, 'mode');
return binding.fchmod(handle.fd, mode, kUsePromises);
}
async function chmod(path, mode) {
path = getPathFromURL(path);
validatePath(path);
mode = modeNum(mode);
validateUint32(mode, 'mode');
mode = validateAndMaskMode(mode, 'mode');
return binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises);
}

16
lib/internal/fs/utils.js
View File

@ -70,21 +70,6 @@ function getOptions(options, defaultOptions) {
return options;
}
function modeNum(m, def) {
if (typeof m === 'number')
return m;
if (typeof m === 'string') {
const parsed = parseInt(m, 8);
if (Number.isNaN(parsed))
return m;
return parsed;
}
// TODO(BridgeAR): Only return `def` in case `m == null`
if (def !== undefined)
return def;
return m;
}
// Check if the path contains null types if it is a string nor Uint8Array,
// otherwise return silently.
function nullCheck(path, propName, throwError = true) {
@ -391,7 +376,6 @@ module.exports = {
assertEncoding,
copyObject,
getOptions,
modeNum,
nullCheck,
preprocessSymlinkDestination,
realpathCacheKey: Symbol('realpathCacheKey'),

26
lib/internal/process/methods.js
View File

@ -2,10 +2,10 @@
const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_UNKNOWN_CREDENTIAL
} = require('internal/errors').codes;
const {
validateAndMaskMode,
validateUint32
} = require('internal/validators');
@ -30,29 +30,13 @@ function setupProcessMethods() {
return _chdir(directory);
}
const octalReg = /^[0-7]+$/;
function umask(mask) {
if (typeof mask === 'undefined') {
if (mask === undefined) {
// Get the mask
return _umask(mask);
}
if (typeof mask === 'number') {
validateUint32(mask, 'mask');
return _umask(mask);
}
if (typeof mask === 'string') {
if (!octalReg.test(mask)) {
throw new ERR_INVALID_ARG_VALUE('mask', mask,
'must be an octal string');
}
const octal = Number.parseInt(mask, 8);
validateUint32(octal, 'mask');
return _umask(octal);
}
throw new ERR_INVALID_ARG_TYPE('mask', ['number', 'string', 'undefined'],
mask);
mask = validateAndMaskMode(mask, 'mask');
return _umask(mask);
}
}

36
lib/internal/validators.js
View File

@ -2,6 +2,7 @@
const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_OUT_OF_RANGE
} = require('internal/errors').codes;
@ -13,6 +14,40 @@ function isUint32(value) {
return value === (value >>> 0);
}
const octalReg = /^[0-7]+$/;
const modeDesc = 'must be a 32-bit unsigned integer or an octal string';
// Validator for mode_t (the S_* constants). Valid numbers or octal strings
// will be masked with 0o777 to be consistent with the behavior in POSIX APIs.
function validateAndMaskMode(value, name, def) {
if (isUint32(value)) {
return value & 0o777;
}
if (typeof value === 'number') {
if (!Number.isInteger(value)) {
throw new ERR_OUT_OF_RANGE(name, 'an integer', value);
} else {
// 2 ** 32 === 4294967296
throw new ERR_OUT_OF_RANGE(name, '>= 0 && < 4294967296', value);
}
}
if (typeof value === 'string') {
if (!octalReg.test(value)) {
throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc);
}
const parsed = parseInt(value, 8);
return parsed & 0o777;
}
// TODO(BridgeAR): Only return `def` in case `value == null`
if (def !== undefined) {
return def;
}
throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc);
}
function validateInt32(value, name) {
if (!isInt32(value)) {
let err;
@ -53,6 +88,7 @@ function validateUint32(value, name, positive) {
module.exports = {
isInt32,
isUint32,
validateAndMaskMode,
validateInt32,
validateUint32
};

95
test/parallel/test-fs-chmod-mask.js Normal file
View File

@ -0,0 +1,95 @@
'use strict';
// This tests that mode > 0o777 will be masked off with 0o777 in fs APIs.
const common = require('../common');
const assert = require('assert');
const path = require('path');
const fs = require('fs');
let mode;
// On Windows chmod is only able to manipulate write permission
if (common.isWindows) {
mode = 0o444; // read-only
} else {
mode = 0o777;
}
const maskToIgnore = 0o10000;
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
function test(mode, asString) {
const suffix = asString ? 'str' : 'num';
const input = asString ?
(mode | maskToIgnore).toString(8) : (mode | maskToIgnore);
{
const file = path.join(tmpdir.path, `chmod-async-${suffix}.txt`);
fs.writeFileSync(file, 'test', 'utf-8');
fs.chmod(file, input, common.mustCall((err) => {
assert.ifError(err);
assert.strictEqual(fs.statSync(file).mode & 0o777, mode);
}));
}
{
const file = path.join(tmpdir.path, `chmodSync-${suffix}.txt`);
fs.writeFileSync(file, 'test', 'utf-8');
fs.chmodSync(file, input);
assert.strictEqual(fs.statSync(file).mode & 0o777, mode);
}
{
const file = path.join(tmpdir.path, `fchmod-async-${suffix}.txt`);
fs.writeFileSync(file, 'test', 'utf-8');
fs.open(file, 'w', common.mustCall((err, fd) => {
assert.ifError(err);
fs.fchmod(fd, input, common.mustCall((err) => {
assert.ifError(err);
assert.strictEqual(fs.fstatSync(fd).mode & 0o777, mode);
fs.close(fd, assert.ifError);
}));
}));
}
{
const file = path.join(tmpdir.path, `fchmodSync-${suffix}.txt`);
fs.writeFileSync(file, 'test', 'utf-8');
const fd = fs.openSync(file, 'w');
fs.fchmodSync(fd, input);
assert.strictEqual(fs.fstatSync(fd).mode & 0o777, mode);
fs.close(fd, assert.ifError);
}
if (fs.lchmod) {
const link = path.join(tmpdir.path, `lchmod-src-${suffix}`);
const file = path.join(tmpdir.path, `lchmod-dest-${suffix}`);
fs.writeFileSync(file, 'test', 'utf-8');
fs.symlinkSync(file, link);
fs.lchmod(link, input, common.mustCall((err) => {
assert.ifError(err);
assert.strictEqual(fs.lstatSync(link).mode & 0o777, mode);
}));
}
if (fs.lchmodSync) {
const link = path.join(tmpdir.path, `lchmodSync-src-${suffix}`);
const file = path.join(tmpdir.path, `lchmodSync-dest-${suffix}`);
fs.writeFileSync(file, 'test', 'utf-8');
fs.symlinkSync(file, link);
fs.lchmodSync(link, input);
assert.strictEqual(fs.lstatSync(link).mode & 0o777, mode);
}
}
test(mode, true);
test(mode, false);

8
test/parallel/test-fs-chmod.js
View File

@ -62,7 +62,7 @@ function closeSync() {
}
// On Windows chmod is only able to manipulate read-only bit
// On Windows chmod is only able to manipulate write permission
if (common.isWindows) {
mode_async = 0o400; // read-only
mode_sync = 0o600; // read-write
@ -112,10 +112,10 @@ fs.open(file2, 'w', common.mustCall((err, fd) => {
common.expectsError(
() => fs.fchmod(fd, {}),
{
code: 'ERR_INVALID_ARG_TYPE',
code: 'ERR_INVALID_ARG_VALUE',
type: TypeError,
message: 'The "mode" argument must be of type number. ' +
'Received type object'
message: 'The argument \'mode\' must be a 32-bit unsigned integer ' +
'or an octal string. Received {}'
}
);

38
test/parallel/test-fs-fchmod.js
View File

@ -1,6 +1,7 @@
'use strict';
const common = require('../common');
require('../common');
const assert = require('assert');
const util = require('util');
const fs = require('fs');
// This test ensures that input for fchmod is valid, testing for valid
@ -16,7 +17,16 @@ const fs = require('fs');
};
assert.throws(() => fs.fchmod(input), errObj);
assert.throws(() => fs.fchmodSync(input), errObj);
errObj.message = errObj.message.replace('fd', 'mode');
});
[false, null, undefined, {}, [], '', '123x'].forEach((input) => {
const errObj = {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError [ERR_INVALID_ARG_VALUE]',
message: 'The argument \'mode\' must be a 32-bit unsigned integer or an ' +
`octal string. Received ${util.inspect(input)}`
};
assert.throws(() => fs.fchmod(1, input), errObj);
assert.throws(() => fs.fchmodSync(1, input), errObj);
});
@ -62,27 +72,3 @@ const fs = require('fs');
assert.throws(() => fs.fchmod(1, input), errObj);
assert.throws(() => fs.fchmodSync(1, input), errObj);
});
// Check for mode values range
const modeUpperBoundaryValue = 0o777;
fs.fchmod(1, modeUpperBoundaryValue, common.mustCall());
fs.fchmodSync(1, modeUpperBoundaryValue);
// umask of 0o777 is equal to 775
const modeOutsideUpperBoundValue = 776;
assert.throws(
() => fs.fchmod(1, modeOutsideUpperBoundValue),
{
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError [ERR_OUT_OF_RANGE]',
message: 'The value of "mode" is out of range. Received 776'
}
);
assert.throws(
() => fs.fchmodSync(1, modeOutsideUpperBoundValue),
{
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError [ERR_OUT_OF_RANGE]',
message: 'The value of "mode" is out of range. Received 776'
}
);

45
test/parallel/test-fs-mkdir-mode-mask.js Normal file
View File

@ -0,0 +1,45 @@
'use strict';
// This tests that mode > 0o777 will be masked off with 0o777 in fs.mkdir().
const common = require('../common');
const assert = require('assert');
const path = require('path');
const fs = require('fs');
let mode;
if (common.isWindows) {
common.skip('mode is not supported in mkdir on Windows');
return;
} else {
mode = 0o644;
}
const maskToIgnore = 0o10000;
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
function test(mode, asString) {
const suffix = asString ? 'str' : 'num';
const input = asString ?
(mode | maskToIgnore).toString(8) : (mode | maskToIgnore);
{
const dir = path.join(tmpdir.path, `mkdirSync-${suffix}`);
fs.mkdirSync(dir, input);
assert.strictEqual(fs.statSync(dir).mode & 0o777, mode);
}
{
const dir = path.join(tmpdir.path, `mkdir-${suffix}`);
fs.mkdir(dir, input, common.mustCall((err) => {
assert.ifError(err);
assert.strictEqual(fs.statSync(dir).mode & 0o777, mode);
}));
}
}
test(mode, true);
test(mode, false);

48
test/parallel/test-fs-open-mode-mask.js Normal file
View File

@ -0,0 +1,48 @@
'use strict';
// This tests that mode > 0o777 will be masked off with 0o777 in fs.open().
const common = require('../common');
const assert = require('assert');
const path = require('path');
const fs = require('fs');
let mode;
if (common.isWindows) {
mode = 0o444;
} else {
mode = 0o644;
}
const maskToIgnore = 0o10000;
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
function test(mode, asString) {
const suffix = asString ? 'str' : 'num';
const input = asString ?
(mode | maskToIgnore).toString(8) : (mode | maskToIgnore);
{
const file = path.join(tmpdir.path, `openSync-${suffix}.txt`);
const fd = fs.openSync(file, 'w+', input);
assert.strictEqual(fs.fstatSync(fd).mode & 0o777, mode);
fs.closeSync(fd);
assert.strictEqual(fs.statSync(file).mode & 0o777, mode);
}
{
const file = path.join(tmpdir.path, `open-${suffix}.txt`);
fs.open(file, 'w+', input, common.mustCall((err, fd) => {
assert.ifError(err);
assert.strictEqual(fs.fstatSync(fd).mode & 0o777, mode);
fs.closeSync(fd);
assert.strictEqual(fs.statSync(file).mode & 0o777, mode);
}));
}
}
test(mode, true);
test(mode, false);

26
test/parallel/test-fs-promises.js
View File

@ -113,28 +113,10 @@ function verifyStatObject(stat) {
await fchmod(handle, 0o666);
await handle.chmod(0o666);
// `mode` can't be > 0o777
assert.rejects(
async () => chmod(handle, (0o777 + 1)),
{
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError [ERR_INVALID_ARG_TYPE]'
}
);
assert.rejects(
async () => fchmod(handle, (0o777 + 1)),
{
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError [ERR_OUT_OF_RANGE]'
}
);
assert.rejects(
async () => handle.chmod(handle, (0o777 + 1)),
{
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError [ERR_INVALID_ARG_TYPE]'
}
);
// Mode larger than 0o777 should be masked off.
await chmod(dest, (0o777 + 1));
await fchmod(handle, 0o777 + 1);
await handle.chmod(0o777 + 1);
await utimes(dest, new Date(), new Date());

29
test/parallel/test-process-umask-mask.js Normal file
View File

@ -0,0 +1,29 @@
'use strict';
// This tests that mask > 0o777 will be masked off with 0o777 in
// process.umask()
const common = require('../common');
const assert = require('assert');
let mask;
if (common.isWindows) {
mask = 0o600;
} else {
mask = 0o664;
}
const maskToIgnore = 0o10000;
const old = process.umask();
function test(input, output) {
process.umask(input);
assert.strictEqual(process.umask(), output);
process.umask(old);
}
test(mask | maskToIgnore, mask);
test((mask | maskToIgnore).toString(8), mask);

15
test/parallel/test-process-umask.js
View File

@ -33,20 +33,20 @@ if (common.isWindows) {
const old = process.umask(mask);
assert.strictEqual(parseInt(mask, 8), process.umask(old));
assert.strictEqual(process.umask(old), parseInt(mask, 8));
// confirm reading the umask does not modify it.
// 1. If the test fails, this call will succeed, but the mask will be set to 0
assert.strictEqual(old, process.umask());
assert.strictEqual(process.umask(), old);
// 2. If the test fails, process.umask() will return 0
assert.strictEqual(old, process.umask());
assert.strictEqual(process.umask(), old);
assert.throws(() => {
process.umask({});
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "mask" argument must be one of type number, string, or ' +
'undefined. Received type object'
code: 'ERR_INVALID_ARG_VALUE',
message: 'The argument \'mask\' must be a 32-bit unsigned integer ' +
'or an octal string. Received {}'
});
['123x', 'abc', '999'].forEach((value) => {
@ -54,6 +54,7 @@ assert.throws(() => {
process.umask(value);
}, {
code: 'ERR_INVALID_ARG_VALUE',
message: `The argument 'mask' must be an octal string. Received '${value}'`
message: 'The argument \'mask\' must be a 32-bit unsigned integer ' +
`or an octal string. Received '${value}'`
});
});