lib: unmask mode_t values with 0o777

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
nodejs · May 26, 2018 · b4b7d368bee9f059b2786ab3f901c759ff72722b · b4b7d36
1 parent 9f9355d
commit b4b7d368bee9f059b2786ab3f901c759ff72722b
diff --git 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
@@ -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
@@ -31,8 +31,8 @@ const {
  validatePath
 } = require('internal/fs/utils');
 const {
-  
-  validateAndMaskMode,
  validateInteger,
+  
+  validateMode,
  validateUint32
 } = require('internal/validators');
 const pathModule = require('path');
@@ -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),
@@ -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);
 }
@@ -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);
 }

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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());

diff --git 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');