diff --git a/lib/fs.js b/lib/fs.js index 9cab1e9e25..56e70d2f47 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -56,6 +56,13 @@ function getOptions(options, defaultOptions) { return options; } +function copyObject(source, target) { + target = arguments.length >= 2 ? target : {}; + for (const key in source) + target[key] = source[key]; + return target; +} + function rethrow() { // TODO(thefourtheye) Throw error instead of warning in major version > 7 process.emitWarning( @@ -1230,11 +1237,11 @@ fs.appendFile = function(path, data, options, callback) { callback = maybeCallback(arguments[arguments.length - 1]); options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' }); - if (!options.flag) - options = util._extend({ flag: 'a' }, options); + // Don't make changes directly on options object + options = copyObject(options); // force append behavior when using a supplied file descriptor - if (isFd(path)) + if (!options.flag || isFd(path)) options.flag = 'a'; fs.writeFile(path, data, options, callback); @@ -1243,11 +1250,11 @@ fs.appendFile = function(path, data, options, callback) { fs.appendFileSync = function(path, data, options) { options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' }); - if (!options.flag) - options = util._extend({ flag: 'a' }, options); + // Don't make changes directly on options object + options = copyObject(options); // force append behavior when using a supplied file descriptor - if (isFd(path)) + if (!options.flag || isFd(path)) options.flag = 'a'; fs.writeFileSync(path, data, options); @@ -1305,6 +1312,9 @@ fs.watch = function(filename, options, listener) { } options = getOptions(options, {}); + // Don't make changes directly on options object + options = copyObject(options); + if (options.persistent === undefined) options.persistent = true; if (options.recursive === undefined) options.recursive = false; @@ -1705,7 +1715,7 @@ function ReadStream(path, options) { return new ReadStream(path, options); // a little bit bigger buffer and water marks by default - options = Object.create(getOptions(options, {})); + options = copyObject(getOptions(options, {})); if (options.highWaterMark === undefined) options.highWaterMark = 64 * 1024; @@ -1871,7 +1881,7 @@ function WriteStream(path, options) { if (!(this instanceof WriteStream)) return new WriteStream(path, options); - options = Object.create(getOptions(options, {})); + options = copyObject(getOptions(options, {})); Writable.call(this, options); diff --git a/test/parallel/test-fs-options-immutable.js b/test/parallel/test-fs-options-immutable.js new file mode 100644 index 0000000000..47796a94a5 --- /dev/null +++ b/test/parallel/test-fs-options-immutable.js @@ -0,0 +1,98 @@ +'use strict'; + +/* + * These tests make sure that the `options` object passed to these functions are + * never altered. + * + * Refer: https://github.com/nodejs/node/issues/7655 + */ + +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); +const errHandler = (e) => assert.ifError(e); +const options = Object.freeze({}); +common.refreshTmpDir(); + +{ + assert.doesNotThrow(() => + fs.readFile(__filename, options, common.mustCall(errHandler)) + ); + assert.doesNotThrow(() => fs.readFileSync(__filename, options)); +} + +{ + assert.doesNotThrow(() => + fs.readdir(__dirname, options, common.mustCall(errHandler)) + ); + assert.doesNotThrow(() => fs.readdirSync(__dirname, options)); +} + +{ + const sourceFile = path.resolve(common.tmpDir, 'test-readlink'); + const linkFile = path.resolve(common.tmpDir, 'test-readlink-link'); + + fs.writeFileSync(sourceFile, ''); + fs.symlinkSync(sourceFile, linkFile); + + assert.doesNotThrow(() => + fs.readlink(linkFile, options, common.mustCall(errHandler)) + ); + assert.doesNotThrow(() => fs.readlinkSync(linkFile, options)); +} + +{ + const fileName = path.resolve(common.tmpDir, 'writeFile'); + assert.doesNotThrow(() => fs.writeFileSync(fileName, 'ABCD', options)); + assert.doesNotThrow(() => + fs.writeFile(fileName, 'ABCD', options, common.mustCall(errHandler)) + ); +} + +{ + const fileName = path.resolve(common.tmpDir, 'appendFile'); + assert.doesNotThrow(() => fs.appendFileSync(fileName, 'ABCD', options)); + assert.doesNotThrow(() => + fs.appendFile(fileName, 'ABCD', options, common.mustCall(errHandler)) + ); +} + +if (!common.isAix) { + // TODO(thefourtheye) Remove this guard once + // https://github.com/nodejs/node/issues/5085 is fixed + { + let watch; + assert.doesNotThrow(() => watch = fs.watch(__filename, options, () => {})); + watch.close(); + } + + { + assert.doesNotThrow(() => fs.watchFile(__filename, options, () => {})); + fs.unwatchFile(__filename); + } +} + +{ + assert.doesNotThrow(() => fs.realpathSync(__filename, options)); + assert.doesNotThrow(() => + fs.realpath(__filename, options, common.mustCall(errHandler)) + ); +} + +{ + const tempFileName = path.resolve(common.tmpDir, 'mkdtemp-'); + assert.doesNotThrow(() => fs.mkdtempSync(tempFileName, options)); + assert.doesNotThrow(() => + fs.mkdtemp(tempFileName, options, common.mustCall(errHandler)) + ); +} + +{ + const fileName = path.resolve(common.tmpDir, 'streams'); + assert.doesNotThrow(() => { + fs.WriteStream(fileName, options).once('open', () => { + assert.doesNotThrow(() => fs.ReadStream(fileName, options)); + }); + }); +}