diff --git a/doc/api/fs.md b/doc/api/fs.md index 724f1b419c78ac..7f8d5d935e85e2 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -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. diff --git a/lib/fs.js b/lib/fs.js index b804336bcf6193..1c58e0cfd68f7a 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -77,7 +77,7 @@ const { } = require('internal/constants'); const { isUint32, - validateAndMaskMode, + validateMode, validateInteger, validateInt32, validateUint32 @@ -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); } @@ -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), @@ -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(); @@ -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); @@ -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(); @@ -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); @@ -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(); @@ -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); diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 687dfb5aead2a8..2564597bd66127 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -32,7 +32,7 @@ const { } = require('internal/fs/utils'); const { isUint32, - validateAndMaskMode, + validateMode, validateInteger, validateUint32 } = require('internal/validators'); @@ -192,7 +192,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), @@ -287,7 +287,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); } @@ -359,14 +359,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); } diff --git a/lib/internal/process/methods.js b/lib/internal/process/methods.js index 7ed993728dbd52..9ba78c57bdf964 100644 --- a/lib/internal/process/methods.js +++ b/lib/internal/process/methods.js @@ -5,7 +5,7 @@ const { ERR_UNKNOWN_CREDENTIAL } = require('internal/errors').codes; const { - validateAndMaskMode, + validateMode, validateUint32 } = require('internal/validators'); @@ -35,7 +35,7 @@ function setupProcessMethods() { // Get the mask return _umask(mask); } - mask = validateAndMaskMode(mask, 'mask'); + mask = validateMode(mask, 'mask'); return _umask(mask); } } diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 17b10dab189133..b7b21d29bfa097 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -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') { @@ -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` @@ -106,7 +117,7 @@ function validateUint32(value, name, positive) { module.exports = { isInt32, isUint32, - validateAndMaskMode, + validateMode, validateInteger, validateInt32, validateUint32 diff --git a/test/parallel/test-fs-chmod-mask.js b/test/parallel/test-fs-chmod-mask.js index 5f3a8d5ab82864..9564ca142bd643 100644 --- a/test/parallel/test-fs-chmod-mask.js +++ b/test/parallel/test-fs-chmod-mask.js @@ -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'); diff --git a/test/parallel/test-fs-mkdir-mode-mask.js b/test/parallel/test-fs-mkdir-mode-mask.js index e4e8a423483376..515b982054b82b 100644 --- a/test/parallel/test-fs-mkdir-mode-mask.js +++ b/test/parallel/test-fs-mkdir-mode-mask.js @@ -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'); diff --git a/test/parallel/test-fs-open-mode-mask.js b/test/parallel/test-fs-open-mode-mask.js index 4db41864af099c..0cd9a2c5923a71 100644 --- a/test/parallel/test-fs-open-mode-mask.js +++ b/test/parallel/test-fs-open-mode-mask.js @@ -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'); diff --git a/test/parallel/test-fs-promises.js b/test/parallel/test-fs-promises.js index 7dccdea9bc2fd8..e738513e6f55d9 100644 --- a/test/parallel/test-fs-promises.js +++ b/test/parallel/test-fs-promises.js @@ -102,9 +102,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()); diff --git a/test/parallel/test-process-umask-mask.js b/test/parallel/test-process-umask-mask.js index c312c061d2b76a..8ec8fc0074ac1b 100644 --- a/test/parallel/test-process-umask-mask.js +++ b/test/parallel/test-process-umask-mask.js @@ -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');