diff --git a/doc/api/fs.md b/doc/api/fs.md index ec86064a94f6e5..3917001d6c07c0 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -5493,8 +5493,8 @@ On Linux, positional writes don't work when the file is opened in append mode. The kernel ignores the position argument and always appends the data to the end of the file. -Modifying a file rather than replacing it may require a flags mode of `'r+'` -rather than the default mode `'w'`. +Modifying a file rather than replacing it may require the `flag` option to be +set to `'r+'` rather than the default `'w'`. The behavior of some flags are platform-specific. As such, opening a directory on macOS and Linux with the `'a+'` flag, as in the example below, will return an diff --git a/lib/fs.js b/lib/fs.js index 3810ad2e6ca916..66113899a5bfdc 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -72,6 +72,7 @@ const { getDirents, getOptions, getValidatedPath, + getValidMode, handleErrorFromBinding, nullCheck, preprocessSymlinkDestination, @@ -99,7 +100,7 @@ const { } = require('internal/constants'); const { isUint32, - parseMode, + parseFileMode, validateBuffer, validateInteger, validateInt32, @@ -183,8 +184,7 @@ function access(path, mode, callback) { } path = getValidatedPath(path); - - mode = mode | 0; + mode = getValidMode(mode, 'access'); const req = new FSReqCallback(); req.oncomplete = makeCallback(callback); binding.access(pathModule.toNamespacedPath(path), mode, req); @@ -192,11 +192,7 @@ function access(path, mode, callback) { function accessSync(path, mode) { path = getValidatedPath(path); - - if (mode === undefined) - mode = F_OK; - else - mode = mode | 0; + mode = getValidMode(mode, 'access'); const ctx = { path }; binding.access(pathModule.toNamespacedPath(path), mode, undefined, ctx); @@ -310,8 +306,9 @@ function readFile(path, options, callback) { } path = getValidatedPath(path); + const flagsNumber = stringToFlags(options.flags); binding.open(pathModule.toNamespacedPath(path), - stringToFlags(options.flag || 'r'), + flagsNumber, 0o666, req); } @@ -428,11 +425,10 @@ function open(path, flags, mode, callback) { } else if (typeof mode === 'function') { callback = mode; mode = 0o666; + } else { + mode = parseFileMode(mode, 'mode', 0o666); } const flagsNumber = stringToFlags(flags); - if (arguments.length >= 4) { - mode = parseMode(mode, 'mode', 0o666); - } callback = makeCallback(callback); const req = new FSReqCallback(); @@ -447,8 +443,8 @@ function open(path, flags, mode, callback) { function openSync(path, flags, mode) { path = getValidatedPath(path); - const flagsNumber = stringToFlags(flags || 'r'); - mode = parseMode(mode, 'mode', 0o666); + const flagsNumber = stringToFlags(flags); + mode = parseFileMode(mode, 'mode', 0o666); const ctx = { path }; const result = binding.open(pathModule.toNamespacedPath(path), @@ -814,16 +810,18 @@ function fsyncSync(fd) { } function mkdir(path, options, callback) { + let mode = 0o777; + let recursive = false; if (typeof options === 'function') { callback = options; - options = {}; } else if (typeof options === 'number' || typeof options === 'string') { - options = { mode: options }; + mode = options; + } else if (options) { + if (options.recursive !== undefined) + recursive = options.recursive; + if (options.mode !== undefined) + mode = options.mode; } - const { - recursive = false, - mode = 0o777 - } = options || {}; callback = makeCallback(callback); path = getValidatedPath(path); @@ -833,25 +831,27 @@ function mkdir(path, options, callback) { const req = new FSReqCallback(); req.oncomplete = callback; binding.mkdir(pathModule.toNamespacedPath(path), - parseMode(mode, 'mode', 0o777), recursive, req); + parseFileMode(mode, 'mode'), recursive, req); } function mkdirSync(path, options) { + let mode = 0o777; + let recursive = false; if (typeof options === 'number' || typeof options === 'string') { - options = { mode: options }; + mode = options; + } else if (options) { + if (options.recursive !== undefined) + recursive = options.recursive; + if (options.mode !== undefined) + mode = options.mode; } - const { - recursive = false, - mode = 0o777 - } = options || {}; - path = getValidatedPath(path); if (typeof recursive !== 'boolean') throw new ERR_INVALID_ARG_TYPE('recursive', 'boolean', recursive); const ctx = { path }; binding.mkdir(pathModule.toNamespacedPath(path), - parseMode(mode, 'mode', 0o777), recursive, undefined, + parseFileMode(mode, 'mode'), recursive, undefined, ctx); handleErrorFromBinding(ctx); } @@ -1070,7 +1070,7 @@ function unlinkSync(path) { function fchmod(fd, mode, callback) { validateInt32(fd, 'fd', 0); - mode = parseMode(mode, 'mode'); + mode = parseFileMode(mode, 'mode'); callback = makeCallback(callback); const req = new FSReqCallback(); @@ -1080,7 +1080,7 @@ function fchmod(fd, mode, callback) { function fchmodSync(fd, mode) { validateInt32(fd, 'fd', 0); - mode = parseMode(mode, 'mode'); + mode = parseFileMode(mode, 'mode'); const ctx = {}; binding.fchmod(fd, mode, undefined, ctx); handleErrorFromBinding(ctx); @@ -1120,7 +1120,7 @@ function lchmodSync(path, mode) { function chmod(path, mode, callback) { path = getValidatedPath(path); - mode = parseMode(mode, 'mode'); + mode = parseFileMode(mode, 'mode'); callback = makeCallback(callback); const req = new FSReqCallback(); @@ -1130,7 +1130,7 @@ function chmod(path, mode, callback) { function chmodSync(path, mode) { path = getValidatedPath(path); - mode = parseMode(mode, 'mode'); + mode = parseFileMode(mode, 'mode'); const ctx = { path }; binding.chmod(pathModule.toNamespacedPath(path), mode, undefined, ctx); @@ -1791,10 +1791,10 @@ function mkdtempSync(prefix, options) { } -function copyFile(src, dest, flags, callback) { - if (typeof flags === 'function') { - callback = flags; - flags = 0; +function copyFile(src, dest, mode, callback) { + if (typeof mode === 'function') { + callback = mode; + mode = 0; } else if (typeof callback !== 'function') { throw new ERR_INVALID_CALLBACK(callback); } @@ -1804,14 +1804,14 @@ function copyFile(src, dest, flags, callback) { src = pathModule._makeLong(src); dest = pathModule._makeLong(dest); - flags = flags | 0; + mode = getValidMode(mode, 'copyFile'); const req = new FSReqCallback(); req.oncomplete = makeCallback(callback); - binding.copyFile(src, dest, flags, req); + binding.copyFile(src, dest, mode, req); } -function copyFileSync(src, dest, flags) { +function copyFileSync(src, dest, mode) { src = getValidatedPath(src, 'src'); dest = getValidatedPath(dest, 'dest'); @@ -1819,8 +1819,8 @@ function copyFileSync(src, dest, flags) { src = pathModule._makeLong(src); dest = pathModule._makeLong(dest); - flags = flags | 0; - binding.copyFile(src, dest, flags, undefined, ctx); + mode = getValidMode(mode, 'copyFile'); + binding.copyFile(src, dest, mode, undefined, ctx); handleErrorFromBinding(ctx); } diff --git a/lib/internal/bootstrap/switches/does_own_process_state.js b/lib/internal/bootstrap/switches/does_own_process_state.js index 15023ea3f55fed..5ee7f079d10124 100644 --- a/lib/internal/bootstrap/switches/does_own_process_state.js +++ b/lib/internal/bootstrap/switches/does_own_process_state.js @@ -23,7 +23,7 @@ if (credentials.implementsPosixCredentials) { // ---- compare the setups side-by-side ----- const { - parseMode, + parseFileMode, validateString } = require('internal/validators'); @@ -119,7 +119,7 @@ function wrappedChdir(directory) { function wrappedUmask(mask) { if (mask !== undefined) { - mask = parseMode(mask, 'mask'); + mask = parseFileMode(mask, 'mask'); } return rawMethods.umask(mask); } diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 65e4fdb63c7ecb..5f8be113f44c0c 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -30,6 +30,7 @@ const { getOptions, getStatsFromBinding, getValidatedPath, + getValidMode, nullCheck, preprocessSymlinkDestination, stringToFlags, @@ -43,7 +44,7 @@ const { } = require('internal/fs/utils'); const { opendir } = require('internal/fs/dir'); const { - parseMode, + parseFileMode, validateBuffer, validateInteger, validateUint32 @@ -189,27 +190,27 @@ async function readFileHandle(filehandle, options) { async function access(path, mode = F_OK) { path = getValidatedPath(path); - mode = mode | 0; + mode = getValidMode(mode, 'access'); return binding.access(pathModule.toNamespacedPath(path), mode, kUsePromises); } -async function copyFile(src, dest, flags) { +async function copyFile(src, dest, mode) { src = getValidatedPath(src, 'src'); dest = getValidatedPath(dest, 'dest'); - flags = flags | 0; + mode = getValidMode(mode, 'copyFile'); return binding.copyFile(pathModule.toNamespacedPath(src), pathModule.toNamespacedPath(dest), - flags, kUsePromises); + mode, + kUsePromises); } // Note that unlike fs.open() which uses numeric file descriptors, // fsPromises.open() uses the fs.FileHandle class. async function open(path, flags, mode) { path = getValidatedPath(path); - if (arguments.length < 2) flags = 'r'; const flagsNumber = stringToFlags(flags); - mode = parseMode(mode, 'mode', 0o666); + mode = parseFileMode(mode, 'mode', 0o666); return new FileHandle( await binding.openFileHandle(pathModule.toNamespacedPath(path), flagsNumber, mode, kUsePromises)); @@ -342,7 +343,7 @@ async function mkdir(path, options) { throw new ERR_INVALID_ARG_TYPE('recursive', 'boolean', recursive); return binding.mkdir(pathModule.toNamespacedPath(path), - parseMode(mode, 'mode', 0o777), recursive, + parseFileMode(mode, 'mode', 0o777), recursive, kUsePromises); } @@ -410,13 +411,13 @@ async function unlink(path) { async function fchmod(handle, mode) { validateFileHandle(handle); - mode = parseMode(mode, 'mode'); + mode = parseFileMode(mode, 'mode'); return binding.fchmod(handle.fd, mode, kUsePromises); } async function chmod(path, mode) { path = getValidatedPath(path); - mode = parseMode(mode, 'mode'); + mode = parseFileMode(mode, 'mode'); return binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises); } diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index bca8c618dab60c..18d8e07d78e3cb 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -7,6 +7,7 @@ const { Error, Number, NumberIsFinite, + MathMin, ObjectSetPrototypeOf, ReflectOwnKeys, Symbol, @@ -40,8 +41,16 @@ const { const pathModule = require('path'); const kType = Symbol('type'); const kStats = Symbol('stats'); +const assert = require('internal/assert'); const { + F_OK = 0, + W_OK = 0, + R_OK = 0, + X_OK = 0, + COPYFILE_EXCL, + COPYFILE_FICLONE, + COPYFILE_FICLONE_FORCE, O_APPEND, O_CREAT, O_EXCL, @@ -70,6 +79,26 @@ const { UV_DIRENT_BLOCK } = internalBinding('constants').fs; +// The access modes can be any of F_OK, R_OK, W_OK or X_OK. Some might not be +// available on specific systems. They can be used in combination as well +// (F_OK | R_OK | W_OK | X_OK). +const kMinimumAccessMode = MathMin(F_OK, W_OK, R_OK, X_OK); +const kMaximumAccessMode = F_OK | W_OK | R_OK | X_OK; + +const kDefaultCopyMode = 0; +// The copy modes can be any of COPYFILE_EXCL, COPYFILE_FICLONE or +// COPYFILE_FICLONE_FORCE. They can be used in combination as well +// (COPYFILE_EXCL | COPYFILE_FICLONE | COPYFILE_FICLONE_FORCE). +const kMinimumCopyMode = MathMin( + kDefaultCopyMode, + COPYFILE_EXCL, + COPYFILE_FICLONE, + COPYFILE_FICLONE_FORCE +); +const kMaximumCopyMode = COPYFILE_EXCL | + COPYFILE_FICLONE | + COPYFILE_FICLONE_FORCE; + const isWindows = process.platform === 'win32'; let fs; @@ -434,6 +463,10 @@ function stringToFlags(flags) { return flags; } + if (flags == null) { + return O_RDONLY; + } + switch (flags) { case 'r' : return O_RDONLY; case 'rs' : // Fall through. @@ -594,6 +627,29 @@ const validateRmdirOptions = hideStackFrames((options) => { return options; }); +const getValidMode = hideStackFrames((mode, type) => { + let min = kMinimumAccessMode; + let max = kMaximumAccessMode; + let def = F_OK; + if (type === 'copyFile') { + min = kMinimumCopyMode; + max = kMaximumCopyMode; + def = mode || kDefaultCopyMode; + } else { + assert(type === 'access'); + } + if (mode == null) { + return def; + } + if (Number.isInteger(mode) && mode >= min && mode <= max) { + return mode; + } + if (typeof mode !== 'number') { + throw new ERR_INVALID_ARG_TYPE('mode', 'integer', mode); + } + throw new ERR_OUT_OF_RANGE( + 'mode', `an integer >= ${min} && <= ${max}`, mode); +}); module.exports = { assertEncoding, @@ -604,6 +660,7 @@ module.exports = { getDirents, getOptions, getValidatedPath, + getValidMode, handleErrorFromBinding, nullCheck, preprocessSymlinkDestination, diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 079ab3d2f53c20..329410ef3d8d0a 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -44,7 +44,11 @@ const modeDesc = 'must be a 32-bit unsigned integer or an octal string'; * @param {number} def If specified, will be returned for invalid values * @returns {number} */ -function parseMode(value, name, def) { +function parseFileMode(value, name, def) { + if (value == null && def !== undefined) { + return def; + } + if (isUint32(value)) { return value; } @@ -60,10 +64,6 @@ function parseMode(value, name, def) { return parseInt(value, 8); } - if (def !== undefined && value == null) { - return def; - } - throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc); } @@ -158,7 +158,7 @@ function validateEncoding(data, encoding) { module.exports = { isInt32, isUint32, - parseMode, + parseFileMode, validateBuffer, validateEncoding, validateInteger, diff --git a/test/parallel/test-fs-access.js b/test/parallel/test-fs-access.js index 2a431134af322e..ec35623c1400f1 100644 --- a/test/parallel/test-fs-access.js +++ b/test/parallel/test-fs-access.js @@ -158,6 +158,55 @@ fs.accessSync(__filename); const mode = fs.F_OK | fs.R_OK | fs.W_OK; fs.accessSync(readWriteFile, mode); +// Invalid modes should throw. +[ + false, + 1n, + { [Symbol.toPrimitive]() { return fs.R_OK; } }, + [1], + 'r' +].forEach((mode, i) => { + console.log(mode, i); + assert.throws( + () => fs.access(readWriteFile, mode, common.mustNotCall()), + { + code: 'ERR_INVALID_ARG_TYPE', + message: /"mode" argument.+integer/ + } + ); + assert.throws( + () => fs.accessSync(readWriteFile, mode), + { + code: 'ERR_INVALID_ARG_TYPE', + message: /"mode" argument.+integer/ + } + ); +}); + +// Out of range modes should throw +[ + -1, + 8, + Infinity, + NaN +].forEach((mode, i) => { + console.log(mode, i); + assert.throws( + () => fs.access(readWriteFile, mode, common.mustNotCall()), + { + code: 'ERR_OUT_OF_RANGE', + message: /"mode".+It must be an integer >= 0 && <= 7/ + } + ); + assert.throws( + () => fs.accessSync(readWriteFile, mode), + { + code: 'ERR_OUT_OF_RANGE', + message: /"mode".+It must be an integer >= 0 && <= 7/ + } + ); +}); + assert.throws( () => { fs.accessSync(doesNotExist); }, (err) => { diff --git a/test/parallel/test-fs-copyfile.js b/test/parallel/test-fs-copyfile.js index ab27ed66848c6b..97828528527852 100644 --- a/test/parallel/test-fs-copyfile.js +++ b/test/parallel/test-fs-copyfile.js @@ -114,28 +114,57 @@ assert.throws(() => { () => fs.copyFile(i, dest, common.mustNotCall()), { code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError' + name: 'TypeError', + message: /src/ } ); assert.throws( () => fs.copyFile(src, i, common.mustNotCall()), { code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError' + name: 'TypeError', + message: /dest/ } ); assert.throws( () => fs.copyFileSync(i, dest), { code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError' + name: 'TypeError', + message: /src/ } ); assert.throws( () => fs.copyFileSync(src, i), { code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError' + name: 'TypeError', + message: /dest/ } ); }); + +assert.throws(() => { + fs.copyFileSync(src, dest, 'r'); +}, { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError', + message: /mode/ +}); + +assert.throws(() => { + fs.copyFileSync(src, dest, 8); +}, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "mode" is out of range. It must be an integer ' + + '>= 0 && <= 7. Received 8' +}); + +assert.throws(() => { + fs.copyFile(src, dest, 'r', common.mustNotCall()); +}, { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError', + message: /mode/ +}); diff --git a/test/parallel/test-fs-error-messages.js b/test/parallel/test-fs-error-messages.js index 05e4515ad40abd..dd9639d99657b6 100644 --- a/test/parallel/test-fs-error-messages.js +++ b/test/parallel/test-fs-error-messages.js @@ -663,21 +663,17 @@ if (!common.isAIX) { ); } -// Check copyFile with invalid flags. +// Check copyFile with invalid modes. { const validateError = { - // TODO: Make sure the error message always also contains the src. - message: `EINVAL: invalid argument, copyfile -> '${nonexistentFile}'`, - errno: UV_EINVAL, - code: 'EINVAL', - syscall: 'copyfile' + message: /"mode".+must be an integer >= 0 && <= 7\. Received -1/, + code: 'ERR_OUT_OF_RANGE' }; - fs.copyFile(existingFile, nonexistentFile, -1, - common.expectsError(validateError)); - - validateError.message = 'EINVAL: invalid argument, copyfile ' + - `'${existingFile}' -> '${nonexistentFile}'`; + assert.throws( + () => fs.copyFile(existingFile, nonexistentFile, -1, () => {}), + validateError + ); assert.throws( () => fs.copyFileSync(existingFile, nonexistentFile, -1), validateError diff --git a/test/parallel/test-fs-open-flags.js b/test/parallel/test-fs-open-flags.js index 016363b8028817..80e54d169d5264 100644 --- a/test/parallel/test-fs-open-flags.js +++ b/test/parallel/test-fs-open-flags.js @@ -84,11 +84,6 @@ assert.throws( { code: 'ERR_INVALID_OPT_VALUE', name: 'TypeError' } ); -assert.throws( - () => stringToFlags(null), - { code: 'ERR_INVALID_OPT_VALUE', name: 'TypeError' } -); - if (common.isLinux || common.isOSX) { const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); diff --git a/test/parallel/test-fs-promises-file-handle-sync.js b/test/parallel/test-fs-promises-file-handle-sync.js index 2c0eca13a73b8c..53eb2424549f93 100644 --- a/test/parallel/test-fs-promises-file-handle-sync.js +++ b/test/parallel/test-fs-promises-file-handle-sync.js @@ -7,11 +7,22 @@ const tmpdir = require('../common/tmpdir'); const { access, copyFile, open } = require('fs').promises; const path = require('path'); -async function validateSync() { +async function validate() { tmpdir.refresh(); const dest = path.resolve(tmpdir.path, 'baz.js'); + await assert.rejects( + copyFile(fixtures.path('baz.js'), dest, 'r'), + { + code: 'ERR_INVALID_ARG_TYPE', + message: /mode.*integer.*string/ + } + ); await copyFile(fixtures.path('baz.js'), dest); - await access(dest, 'r'); + await assert.rejects( + access(dest, 'r'), + { code: 'ERR_INVALID_ARG_TYPE', message: /mode/ } + ); + await access(dest); const handle = await open(dest, 'r+'); await handle.datasync(); await handle.sync(); @@ -23,4 +34,4 @@ async function validateSync() { await handle.close(); } -validateSync(); +validate(); diff --git a/test/parallel/test-fs-promises.js b/test/parallel/test-fs-promises.js index b9d9b36ea898f3..08b86ea4ed41f0 100644 --- a/test/parallel/test-fs-promises.js +++ b/test/parallel/test-fs-promises.js @@ -47,17 +47,33 @@ assert.strictEqual( ); { - access(__filename, 'r') + access(__filename, 0) .then(common.mustCall()); - access('this file does not exist', 'r') - .then(common.mustNotCall()) - .catch(common.expectsError({ + assert.rejects( + access('this file does not exist', 0), + { code: 'ENOENT', name: 'Error', - message: - /^ENOENT: no such file or directory, access/ - })); + message: /^ENOENT: no such file or directory, access/ + } + ); + + assert.rejects( + access(__filename, 8), + { + code: 'ERR_OUT_OF_RANGE', + message: /"mode".*must be an integer >= 0 && <= 7\. Received 8$/ + } + ); + + assert.rejects( + access(__filename, { [Symbol.toPrimitive]() { return 5; } }), + { + code: 'ERR_INVALID_ARG_TYPE', + message: /"mode" argument.+integer\. Received an instance of Object$/ + } + ); } function verifyStatObject(stat) { @@ -68,7 +84,7 @@ function verifyStatObject(stat) { async function getHandle(dest) { await copyFile(fixtures.path('baz.js'), dest); - await access(dest, 'r'); + await access(dest); return open(dest, 'r+'); }