Skip to content

Commit 28fc814

Browse files
committed
fs: ignore prototype properties
This is mainly a minor refactoring to reduce the code overhead with the side effect of removing support for prototype properties in passed through options. Copying options is mostly done for enumerable own properties and prototype properties are ignored.
1 parent 4455f60 commit 28fc814

File tree

5 files changed

+47
-275
lines changed

5 files changed

+47
-275
lines changed

lib/fs.js

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ const { FSReqCallback, statValues } = binding;
6767
const { toPathIfFileURL } = require('internal/url');
6868
const internalUtil = require('internal/util');
6969
const {
70-
copyObject,
7170
Dirent,
7271
getDirents,
7372
getOptions,
@@ -1321,25 +1320,20 @@ function appendFile(path, data, options, callback) {
13211320
callback = maybeCallback(callback || options);
13221321
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });
13231322

1324-
// Don't make changes directly on options object
1325-
options = copyObject(options);
1326-
13271323
// Force append behavior when using a supplied file descriptor
13281324
if (!options.flag || isFd(path))
1329-
options.flag = 'a';
1325+
options = { ...options, flag: 'a' };
13301326

13311327
fs.writeFile(path, data, options, callback);
13321328
}
13331329

13341330
function appendFileSync(path, data, options) {
13351331
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });
13361332

1337-
// Don't make changes directly on options object
1338-
options = copyObject(options);
1339-
13401333
// Force append behavior when using a supplied file descriptor
1341-
if (!options.flag || isFd(path))
1342-
options.flag = 'a';
1334+
if (!options.flag || isFd(path)) {
1335+
options = { ...options, flag: 'a' };
1336+
}
13431337

13441338
fs.writeFileSync(path, data, options);
13451339
}
@@ -1348,10 +1342,7 @@ function watch(filename, options, listener) {
13481342
if (typeof options === 'function') {
13491343
listener = options;
13501344
}
1351-
options = getOptions(options, {});
1352-
1353-
// Don't make changes directly on options object
1354-
options = copyObject(options);
1345+
options = { ...getOptions(options, {}) };
13551346

13561347
if (options.persistent === undefined) options.persistent = true;
13571348
if (options.recursive === undefined) options.recursive = false;

lib/internal/fs/promises.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ const {
2525
const { isUint8Array } = require('internal/util/types');
2626
const { rimrafPromises } = require('internal/fs/rimraf');
2727
const {
28-
copyObject,
2928
getDirents,
3029
getOptions,
3130
getStatsFromBinding,
@@ -496,8 +495,9 @@ async function writeFile(path, data, options) {
496495

497496
async function appendFile(path, data, options) {
498497
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });
499-
options = copyObject(options);
500-
options.flag = options.flag || 'a';
498+
if (!options.flag) {
499+
options = { ...options, flag: 'a' };
500+
}
501501
return writeFile(path, data, options);
502502
}
503503

lib/internal/fs/streams.js

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ const { validateNumber } = require('internal/validators');
2020
const fs = require('fs');
2121
const { Buffer } = require('buffer');
2222
const {
23-
copyObject,
2423
getOptions,
2524
} = require('internal/fs/utils');
2625
const { Readable, Writable } = require('stream');
@@ -69,7 +68,7 @@ function ReadStream(path, options) {
6968
return new ReadStream(path, options);
7069

7170
// A little bit bigger buffer and water marks by default
72-
options = copyObject(getOptions(options, {}));
71+
options = { ...getOptions(options, {}) };
7372
if (options.highWaterMark === undefined)
7473
options.highWaterMark = 64 * 1024;
7574

@@ -292,10 +291,8 @@ function WriteStream(path, options) {
292291
if (!(this instanceof WriteStream))
293292
return new WriteStream(path, options);
294293

295-
options = copyObject(getOptions(options, {}));
296-
297294
// Only buffers are supported.
298-
options.decodeStrings = true;
295+
options = { ...getOptions(options, {}), decodeStrings: true };
299296

300297
// For backwards compat do not emit close on destroy.
301298
if (options.emitClose === undefined) {

lib/internal/fs/utils.js

Lines changed: 37 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -166,49 +166,38 @@ for (const name of ReflectOwnKeys(Dirent.prototype)) {
166166
};
167167
}
168168

169-
function copyObject(source) {
170-
const target = {};
171-
for (const key in source)
172-
target[key] = source[key];
173-
return target;
174-
}
175-
176169
function getDirents(path, [names, types], callback) {
177-
let i;
178-
if (typeof callback === 'function') {
179-
const len = names.length;
180-
let toFinish = 0;
181-
callback = once(callback);
182-
for (i = 0; i < len; i++) {
183-
const type = types[i];
184-
if (type === UV_DIRENT_UNKNOWN) {
185-
const name = names[i];
186-
const idx = i;
187-
toFinish++;
188-
lazyLoadFs().lstat(pathModule.join(path, name), (err, stats) => {
189-
if (err) {
190-
callback(err);
191-
return;
192-
}
193-
names[idx] = new DirentFromStats(name, stats);
194-
if (--toFinish === 0) {
195-
callback(null, names);
196-
}
197-
});
198-
} else {
199-
names[i] = new Dirent(names[i], types[i]);
200-
}
201-
}
202-
if (toFinish === 0) {
203-
callback(null, names);
204-
}
205-
} else {
206-
const len = names.length;
207-
for (i = 0; i < len; i++) {
170+
if (typeof callback !== 'function') {
171+
for (let i = 0; i < names.length; i++) {
208172
names[i] = getDirent(path, names[i], types[i]);
209173
}
210174
return names;
211175
}
176+
let toFinish = 0;
177+
callback = once(callback);
178+
for (let i = 0; i < names.length; i++) {
179+
const type = types[i];
180+
if (type === UV_DIRENT_UNKNOWN) {
181+
const name = names[i];
182+
const idx = i;
183+
toFinish++;
184+
lazyLoadFs().lstat(pathModule.join(path, name), (err, stats) => {
185+
if (err) {
186+
callback(err);
187+
return;
188+
}
189+
names[idx] = new DirentFromStats(name, stats);
190+
if (--toFinish === 0) {
191+
callback(null, names);
192+
}
193+
});
194+
} else {
195+
names[i] = new Dirent(names[i], types[i]);
196+
}
197+
}
198+
if (toFinish === 0) {
199+
callback(null, names);
200+
}
212201
}
213202

214203
function getDirent(path, name, type, callback) {
@@ -239,9 +228,7 @@ function getOptions(options, defaultOptions) {
239228
}
240229

241230
if (typeof options === 'string') {
242-
defaultOptions = { ...defaultOptions };
243-
defaultOptions.encoding = options;
244-
options = defaultOptions;
231+
options = { ...defaultOptions, encoding: options };
245232
} else if (typeof options !== 'object') {
246233
throw new ERR_INVALID_ARG_TYPE('options', ['string', 'Object'], options);
247234
}
@@ -271,13 +258,16 @@ function handleErrorFromBinding(ctx) {
271258
// Check if the path contains null types if it is a string nor Uint8Array,
272259
// otherwise return silently.
273260
const nullCheck = hideStackFrames((path, propName, throwError = true) => {
274-
const pathIsString = typeof path === 'string';
275-
const pathIsUint8Array = isUint8Array(path);
276-
277261
// We can only perform meaningful checks on strings and Uint8Arrays.
278-
if ((!pathIsString && !pathIsUint8Array) ||
279-
(pathIsString && !path.includes('\u0000')) ||
280-
(pathIsUint8Array && !path.includes(0))) {
262+
if (typeof path === 'string') {
263+
if (!path.includes('\u0000')) {
264+
return;
265+
}
266+
} else if (isUint8Array(path)) {
267+
if (!path.includes(0)) {
268+
return;
269+
}
270+
} else {
281271
return;
282272
}
283273

@@ -654,7 +644,6 @@ const getValidMode = hideStackFrames((mode, type) => {
654644
module.exports = {
655645
assertEncoding,
656646
BigIntStats, // for testing
657-
copyObject,
658647
Dirent,
659648
getDirent,
660649
getDirents,

0 commit comments

Comments
 (0)