Skip to content

Commit

Permalink
lib: mask mode_t type of arguments with 0o777
Browse files Browse the repository at this point in the history 
- 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
  • Loading branch information
@joyeecheung @targos
joyeecheung authored and targos committed Jun 13, 2018
1 parent fc2956d commit 2fe88d2
Show file tree
Hide file tree
Showing 12 changed files with 313 additions and 116 deletions.
63 changes: 31 additions & 32 deletions lib/fs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ const internalUtil = require('internal/util');
const {
copyObject,
getOptions,
modeNum,
nullCheck,
preprocessSymlinkDestination,
Stats,
Expand All @@ -85,6 +84,7 @@ const {
} = require('internal/constants');
const {
isUint32,
validateAndMaskMode,
validateInteger,
validateUint32
} = require('internal/validators');
Expand Down Expand Up @@ -549,32 +549,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;
Expand Down Expand Up @@ -849,12 +853,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;
Expand All @@ -864,8 +872,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);
Expand Down Expand Up @@ -1047,25 +1054,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);
Expand Down Expand Up @@ -1106,11 +1106,10 @@ if (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;
Expand All @@ -1120,8 +1119,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);
Expand Down
19 changes: 6 additions & 13 deletions lib/internal/fs/promises.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,14 @@ 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');
const {
copyObject,
getOptions,
getStatsFromBinding,
modeNum,
nullCheck,
preprocessSymlinkDestination,
stringToFlags,
Expand All @@ -33,6 +31,7 @@ const {
validatePath
} = require('internal/fs/utils');
const {
validateAndMaskMode,
validateInteger,
validateUint32
} = require('internal/validators');
Expand Down Expand Up @@ -190,10 +189,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),
Expand Down Expand Up @@ -286,10 +284,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);
}

Expand Down Expand Up @@ -360,19 +357,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);
}

Expand Down
16 changes: 0 additions & 16 deletions lib/internal/fs/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -391,7 +376,6 @@ module.exports = {
assertEncoding,
copyObject,
getOptions,
modeNum,
nullCheck,
preprocessSymlinkDestination,
realpathCacheKey: Symbol('realpathCacheKey'),
Expand Down
36 changes: 36 additions & 0 deletions lib/internal/validators.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_OUT_OF_RANGE
} = require('internal/errors').codes;

Expand All @@ -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 validateInteger(value, name) {
let err;

Expand Down Expand Up @@ -67,6 +102,7 @@ function validateUint32(value, name, positive) {
module.exports = {
isInt32,
isUint32,
validateAndMaskMode,
validateInteger,
validateInt32,
validateUint32
Expand Down
Loading

0 comments on commit 2fe88d2

Please sign in to comment.