Skip to content

Commit

Permalink
lib: unmask mode_t values with 0o777
Browse files Browse the repository at this point in the history
This commit allows permission bits higher than 0o777 to go through
the API (e.g. `S_ISVTX`=`0o1000`, `S_ISGID`=`0o2000`,
`S_ISUID`=`0o4000`).

Also documents that these bits are not exposed through `fs.constants`
and their behaviors are platform-specific, so the users need to
use them on their own risk.

PR-URL: #20975
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>

Backport-PR-URL: #21172
  • Loading branch information
joyeecheung authored and targos committed Jun 13, 2018
1 parent 9f9355d commit b4b7d36
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 27 deletions.
6 changes: 6 additions & 0 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,12 @@ For example, the octal value `0o765` means:
* The group may read and write the file.
* Others may read and execute the file.

Note: When using raw numbers where file modes are expected,
any value larger than `0o777` may result in platform-specific
behaviors that are not supported to work consistently.
Therefore constants like `S_ISVTX`, `S_ISGID` or `S_ISUID` are
not exposed in `fs.constants`.

Caveats: on Windows only the write permission can be changed, and the
distinction among the permissions of group, owner or others is not
implemented.
Expand Down
18 changes: 9 additions & 9 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const {
} = require('internal/constants');
const {
isUint32,
validateAndMaskMode,
validateMode,
validateInteger,
validateInt32,
validateUint32
Expand Down Expand Up @@ -416,7 +416,7 @@ function open(path, flags, mode, callback) {
callback = makeCallback(mode);
mode = 0o666;
} else {
mode = validateAndMaskMode(mode, 'mode', 0o666);
mode = validateMode(mode, 'mode', 0o666);
callback = makeCallback(callback);
}

Expand All @@ -434,7 +434,7 @@ function openSync(path, flags, mode) {
path = getPathFromURL(path);
validatePath(path);
const flagsNumber = stringToFlags(flags);
mode = validateAndMaskMode(mode, 'mode', 0o666);
mode = validateMode(mode, 'mode', 0o666);

const ctx = { path };
const result = binding.open(pathModule.toNamespacedPath(path),
Expand Down Expand Up @@ -721,7 +721,7 @@ function mkdir(path, mode, callback) {
mode = 0o777;
} else {
callback = makeCallback(callback);
mode = validateAndMaskMode(mode, 'mode', 0o777);
mode = validateMode(mode, 'mode', 0o777);
}

const req = new FSReqWrap();
Expand All @@ -732,7 +732,7 @@ function mkdir(path, mode, callback) {
function mkdirSync(path, mode) {
path = getPathFromURL(path);
validatePath(path);
mode = validateAndMaskMode(mode, 'mode', 0o777);
mode = validateMode(mode, 'mode', 0o777);
const ctx = { path };
binding.mkdir(pathModule.toNamespacedPath(path), mode, undefined, ctx);
handleErrorFromBinding(ctx);
Expand Down Expand Up @@ -915,7 +915,7 @@ function unlinkSync(path) {

function fchmod(fd, mode, callback) {
validateInt32(fd, 'fd', 0);
mode = validateAndMaskMode(mode, 'mode');
mode = validateMode(mode, 'mode');
callback = makeCallback(callback);

const req = new FSReqWrap();
Expand All @@ -925,7 +925,7 @@ function fchmod(fd, mode, callback) {

function fchmodSync(fd, mode) {
validateInt32(fd, 'fd', 0);
mode = validateAndMaskMode(mode, 'mode');
mode = validateMode(mode, 'mode');
const ctx = {};
binding.fchmod(fd, mode, undefined, ctx);
handleErrorFromBinding(ctx);
Expand Down Expand Up @@ -966,7 +966,7 @@ function lchmodSync(path, mode) {
function chmod(path, mode, callback) {
path = getPathFromURL(path);
validatePath(path);
mode = validateAndMaskMode(mode, 'mode');
mode = validateMode(mode, 'mode');
callback = makeCallback(callback);

const req = new FSReqWrap();
Expand All @@ -977,7 +977,7 @@ function chmod(path, mode, callback) {
function chmodSync(path, mode) {
path = getPathFromURL(path);
validatePath(path);
mode = validateAndMaskMode(mode, 'mode');
mode = validateMode(mode, 'mode');

const ctx = { path };
binding.chmod(pathModule.toNamespacedPath(path), mode, undefined, ctx);
Expand Down
10 changes: 5 additions & 5 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ const {
validatePath
} = require('internal/fs/utils');
const {
validateAndMaskMode,
validateInteger,
validateMode,
validateUint32
} = require('internal/validators');
const pathModule = require('path');
Expand Down Expand Up @@ -191,7 +191,7 @@ async function copyFile(src, dest, flags) {
async function open(path, flags, mode) {
path = getPathFromURL(path);
validatePath(path);
mode = validateAndMaskMode(mode, 'mode', 0o666);
mode = validateMode(mode, 'mode', 0o666);
return new FileHandle(
await binding.openFileHandle(pathModule.toNamespacedPath(path),
stringToFlags(flags),
Expand Down Expand Up @@ -286,7 +286,7 @@ async function fsync(handle) {
async function mkdir(path, mode) {
path = getPathFromURL(path);
validatePath(path);
mode = validateAndMaskMode(mode, 'mode', 0o777);
mode = validateMode(mode, 'mode', 0o777);
return binding.mkdir(pathModule.toNamespacedPath(path), mode, kUsePromises);
}

Expand Down Expand Up @@ -358,14 +358,14 @@ async function unlink(path) {

async function fchmod(handle, mode) {
validateFileHandle(handle);
mode = validateAndMaskMode(mode, 'mode');
mode = validateMode(mode, 'mode');
return binding.fchmod(handle.fd, mode, kUsePromises);
}

async function chmod(path, mode) {
path = getPathFromURL(path);
validatePath(path);
mode = validateAndMaskMode(mode, 'mode');
mode = validateMode(mode, 'mode');
return binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises);
}

Expand Down
23 changes: 17 additions & 6 deletions lib/internal/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,22 @@ function isUint32(value) {

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) {

/**
* Validate values that will be converted into mode_t (the S_* constants).
* Only valid numbers and octal strings are allowed. They could be converted
* to 32-bit unsigned integers or non-negative signed integers in the C++
* land, but any value higher than 0o777 will result in platform-specific
* behaviors.
*
* @param {*} value Values to be validated
* @param {string} name Name of the argument
* @param {number} def If specified, will be returned for invalid values
* @returns {number}
*/
function validateMode(value, name, def) {
if (isUint32(value)) {
return value & 0o777;
return value;
}

if (typeof value === 'number') {
Expand All @@ -37,7 +48,7 @@ function validateAndMaskMode(value, name, def) {
throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc);
}
const parsed = parseInt(value, 8);
return parsed & 0o777;
return parsed;
}

// TODO(BridgeAR): Only return `def` in case `value == null`
Expand Down Expand Up @@ -106,7 +117,7 @@ function validateUint32(value, name, positive) {
module.exports = {
isInt32,
isUint32,
validateAndMaskMode,
validateMode,
validateInteger,
validateInt32,
validateUint32
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-chmod-mask.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

// This tests that mode > 0o777 will be masked off with 0o777 in fs APIs.
// This tests that the lower bits of mode > 0o777 still works in fs APIs.

const common = require('../common');
const assert = require('assert');
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-mkdir-mode-mask.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

// This tests that mode > 0o777 will be masked off with 0o777 in fs.mkdir().
// This tests that the lower bits of mode > 0o777 still works in fs.mkdir().

const common = require('../common');
const assert = require('assert');
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-open-mode-mask.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

// This tests that mode > 0o777 will be masked off with 0o777 in fs.open().
// This tests that the lower bits of mode > 0o777 still works in fs.open().

const common = require('../common');
const assert = require('assert');
Expand Down
5 changes: 2 additions & 3 deletions test/parallel/test-fs-promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,8 @@ function verifyStatObject(stat) {
await chmod(dest, 0o666);
await handle.chmod(0o666);

// Mode larger than 0o777 should be masked off.
await chmod(dest, (0o777 + 1));
await handle.chmod(0o777 + 1);
await chmod(dest, (0o10777));
await handle.chmod(0o10777);

await utimes(dest, new Date(), new Date());

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-process-umask-mask.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

// This tests that mask > 0o777 will be masked off with 0o777 in
// This tests that the lower bits of mode > 0o777 still works in
// process.umask()

const common = require('../common');
Expand Down

0 comments on commit b4b7d36

Please sign in to comment.