diff --git a/lib/fs.js b/lib/fs.js index 66113899a5bfdc..c6e16a35eec2df 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -67,7 +67,6 @@ const { FSReqCallback, statValues } = binding; const { toPathIfFileURL } = require('internal/url'); const internalUtil = require('internal/util'); const { - copyObject, Dirent, getDirents, getOptions, @@ -1321,12 +1320,9 @@ function appendFile(path, data, options, callback) { callback = maybeCallback(callback || options); options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' }); - // Don't make changes directly on options object - options = copyObject(options); - // Force append behavior when using a supplied file descriptor if (!options.flag || isFd(path)) - options.flag = 'a'; + options = { ...options, flag: 'a' }; fs.writeFile(path, data, options, callback); } @@ -1334,12 +1330,10 @@ function appendFile(path, data, options, callback) { function appendFileSync(path, data, options) { options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' }); - // Don't make changes directly on options object - options = copyObject(options); - // Force append behavior when using a supplied file descriptor - if (!options.flag || isFd(path)) - options.flag = 'a'; + if (!options.flag || isFd(path)) { + options = { ...options, flag: 'a' }; + } fs.writeFileSync(path, data, options); } @@ -1348,10 +1342,7 @@ function watch(filename, options, listener) { if (typeof options === 'function') { listener = options; } - options = getOptions(options, {}); - - // Don't make changes directly on options object - options = copyObject(options); + options = { ...getOptions(options, {}) }; if (options.persistent === undefined) options.persistent = true; if (options.recursive === undefined) options.recursive = false; diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 5f8be113f44c0c..896d8025bdaec6 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -25,7 +25,6 @@ const { const { isUint8Array } = require('internal/util/types'); const { rimrafPromises } = require('internal/fs/rimraf'); const { - copyObject, getDirents, getOptions, getStatsFromBinding, @@ -496,8 +495,9 @@ async function writeFile(path, data, options) { async function appendFile(path, data, options) { options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' }); - options = copyObject(options); - options.flag = options.flag || 'a'; + if (!options.flag) { + options = { ...options, flag: 'a' }; + } return writeFile(path, data, options); } diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index d1603f6a024b81..c86fb7d3fab4a0 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -20,7 +20,6 @@ const { validateNumber } = require('internal/validators'); const fs = require('fs'); const { Buffer } = require('buffer'); const { - copyObject, getOptions, } = require('internal/fs/utils'); const { Readable, Writable } = require('stream'); @@ -69,7 +68,7 @@ function ReadStream(path, options) { return new ReadStream(path, options); // A little bit bigger buffer and water marks by default - options = copyObject(getOptions(options, {})); + options = { ...getOptions(options, {}) }; if (options.highWaterMark === undefined) options.highWaterMark = 64 * 1024; @@ -292,10 +291,8 @@ function WriteStream(path, options) { if (!(this instanceof WriteStream)) return new WriteStream(path, options); - options = copyObject(getOptions(options, {})); - // Only buffers are supported. - options.decodeStrings = true; + options = { ...getOptions(options, {}), decodeStrings: true }; // For backwards compat do not emit close on destroy. if (options.emitClose === undefined) { diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 18d8e07d78e3cb..1c7cda24e0edd8 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -166,49 +166,38 @@ for (const name of ReflectOwnKeys(Dirent.prototype)) { }; } -function copyObject(source) { - const target = {}; - for (const key in source) - target[key] = source[key]; - return target; -} - function getDirents(path, [names, types], callback) { - let i; - if (typeof callback === 'function') { - const len = names.length; - let toFinish = 0; - callback = once(callback); - for (i = 0; i < len; i++) { - const type = types[i]; - if (type === UV_DIRENT_UNKNOWN) { - const name = names[i]; - const idx = i; - toFinish++; - lazyLoadFs().lstat(pathModule.join(path, name), (err, stats) => { - if (err) { - callback(err); - return; - } - names[idx] = new DirentFromStats(name, stats); - if (--toFinish === 0) { - callback(null, names); - } - }); - } else { - names[i] = new Dirent(names[i], types[i]); - } - } - if (toFinish === 0) { - callback(null, names); - } - } else { - const len = names.length; - for (i = 0; i < len; i++) { + if (typeof callback !== 'function') { + for (let i = 0; i < names.length; i++) { names[i] = getDirent(path, names[i], types[i]); } return names; } + let toFinish = 0; + callback = once(callback); + for (let i = 0; i < names.length; i++) { + const type = types[i]; + if (type === UV_DIRENT_UNKNOWN) { + const name = names[i]; + const idx = i; + toFinish++; + lazyLoadFs().lstat(pathModule.join(path, name), (err, stats) => { + if (err) { + callback(err); + return; + } + names[idx] = new DirentFromStats(name, stats); + if (--toFinish === 0) { + callback(null, names); + } + }); + } else { + names[i] = new Dirent(names[i], types[i]); + } + } + if (toFinish === 0) { + callback(null, names); + } } function getDirent(path, name, type, callback) { @@ -239,9 +228,7 @@ function getOptions(options, defaultOptions) { } if (typeof options === 'string') { - defaultOptions = { ...defaultOptions }; - defaultOptions.encoding = options; - options = defaultOptions; + options = { ...defaultOptions, encoding: options }; } else if (typeof options !== 'object') { throw new ERR_INVALID_ARG_TYPE('options', ['string', 'Object'], options); } @@ -271,13 +258,16 @@ function handleErrorFromBinding(ctx) { // Check if the path contains null types if it is a string nor Uint8Array, // otherwise return silently. const nullCheck = hideStackFrames((path, propName, throwError = true) => { - const pathIsString = typeof path === 'string'; - const pathIsUint8Array = isUint8Array(path); - // We can only perform meaningful checks on strings and Uint8Arrays. - if ((!pathIsString && !pathIsUint8Array) || - (pathIsString && !path.includes('\u0000')) || - (pathIsUint8Array && !path.includes(0))) { + if (typeof path === 'string') { + if (!path.includes('\u0000')) { + return; + } + } else if (isUint8Array(path)) { + if (!path.includes(0)) { + return; + } + } else { return; } @@ -654,7 +644,6 @@ const getValidMode = hideStackFrames((mode, type) => { module.exports = { assertEncoding, BigIntStats, // for testing - copyObject, Dirent, getDirent, getDirents, diff --git a/test/parallel/test-fs-read-stream-inherit.js b/test/parallel/test-fs-read-stream-inherit.js deleted file mode 100644 index 5c8bbef0e499c0..00000000000000 --- a/test/parallel/test-fs-read-stream-inherit.js +++ /dev/null @@ -1,205 +0,0 @@ -'use strict'; - -const common = require('../common'); - -const assert = require('assert'); -const fs = require('fs'); -const fixtures = require('../common/fixtures'); - -const fn = fixtures.path('elipses.txt'); -const rangeFile = fixtures.path('x.txt'); - -{ - let paused = false; - - const file = fs.ReadStream(fn); - - file.on('open', common.mustCall(function(fd) { - file.length = 0; - assert.strictEqual(typeof fd, 'number'); - assert.ok(file.readable); - - // GH-535 - file.pause(); - file.resume(); - file.pause(); - file.resume(); - })); - - file.on('data', common.mustCallAtLeast(function(data) { - assert.ok(data instanceof Buffer); - assert.ok(!paused); - file.length += data.length; - - paused = true; - file.pause(); - - setTimeout(function() { - paused = false; - file.resume(); - }, 10); - })); - - - file.on('end', common.mustCall()); - - - file.on('close', common.mustCall(function() { - assert.strictEqual(file.length, 30000); - })); -} - -{ - const file = fs.createReadStream(fn, Object.create({ encoding: 'utf8' })); - file.length = 0; - file.on('data', function(data) { - assert.strictEqual(typeof data, 'string'); - file.length += data.length; - - for (let i = 0; i < data.length; i++) { - // http://www.fileformat.info/info/unicode/char/2026/index.htm - assert.strictEqual(data[i], '\u2026'); - } - }); - - file.on('close', common.mustCall(function() { - assert.strictEqual(file.length, 10000); - })); -} - -{ - const options = Object.create({ bufferSize: 1, start: 1, end: 2 }); - const file = fs.createReadStream(rangeFile, options); - assert.strictEqual(file.start, 1); - assert.strictEqual(file.end, 2); - let contentRead = ''; - file.on('data', function(data) { - contentRead += data.toString('utf-8'); - }); - file.on('end', common.mustCall(function() { - assert.strictEqual(contentRead, 'yz'); - })); -} - -{ - const options = Object.create({ bufferSize: 1, start: 1 }); - const file = fs.createReadStream(rangeFile, options); - assert.strictEqual(file.start, 1); - file.data = ''; - file.on('data', function(data) { - file.data += data.toString('utf-8'); - }); - file.on('end', common.mustCall(function() { - assert.strictEqual(file.data, 'yz\n'); - })); -} - -// https://github.com/joyent/node/issues/2320 -{ - const options = Object.create({ bufferSize: 1.23, start: 1 }); - const file = fs.createReadStream(rangeFile, options); - assert.strictEqual(file.start, 1); - file.data = ''; - file.on('data', function(data) { - file.data += data.toString('utf-8'); - }); - file.on('end', common.mustCall(function() { - assert.strictEqual(file.data, 'yz\n'); - })); -} - -{ - const message = - 'The value of "start" is out of range. It must be <= "end" (here: 2).' + - ' Received 10'; - - assert.throws( - () => { - fs.createReadStream(rangeFile, Object.create({ start: 10, end: 2 })); - }, - { - code: 'ERR_OUT_OF_RANGE', - message, - name: 'RangeError' - }); -} - -{ - const options = Object.create({ start: 0, end: 0 }); - const stream = fs.createReadStream(rangeFile, options); - assert.strictEqual(stream.start, 0); - assert.strictEqual(stream.end, 0); - stream.data = ''; - - stream.on('data', function(chunk) { - stream.data += chunk; - }); - - stream.on('end', common.mustCall(function() { - assert.strictEqual(stream.data, 'x'); - })); -} - -// Pause and then resume immediately. -{ - const pauseRes = fs.createReadStream(rangeFile); - pauseRes.pause(); - pauseRes.resume(); -} - -{ - let data = ''; - let file = - fs.createReadStream(rangeFile, Object.create({ autoClose: false })); - assert.strictEqual(file.autoClose, false); - file.on('data', (chunk) => { data += chunk; }); - file.on('end', common.mustCall(function() { - process.nextTick(common.mustCall(function() { - assert(!file.closed); - assert(!file.destroyed); - assert.strictEqual(data, 'xyz\n'); - fileNext(); - })); - })); - - function fileNext() { - // This will tell us if the fd is usable again or not. - file = fs.createReadStream(null, Object.create({ fd: file.fd, start: 0 })); - file.data = ''; - file.on('data', function(data) { - file.data += data; - }); - file.on('end', common.mustCall(function() { - assert.strictEqual(file.data, 'xyz\n'); - })); - } - process.on('exit', function() { - assert(file.closed); - assert(file.destroyed); - }); -} - -// Just to make sure autoClose won't close the stream because of error. -{ - const options = Object.create({ fd: 13337, autoClose: false }); - const file = fs.createReadStream(null, options); - file.on('data', common.mustNotCall()); - file.on('error', common.mustCall()); - process.on('exit', function() { - assert(!file.closed); - assert(!file.destroyed); - assert(file.fd); - }); -} - -// Make sure stream is destroyed when file does not exist. -{ - const file = fs.createReadStream('/path/to/file/that/does/not/exist'); - file.on('data', common.mustNotCall()); - file.on('error', common.mustCall()); - - process.on('exit', function() { - assert(!file.closed); - assert(file.destroyed); - }); -}