diff --git a/doc/api/fs.md b/doc/api/fs.md index f2e4e2757cb4eb..2f9705752556d9 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -5396,7 +5396,7 @@ changes: * `buffer` {Buffer|TypedArray|DataView} * `options` {Object} * `offset` {integer} **Default:** `0` - * `length` {integer} **Default:** `buffer.byteLength` + * `length` {integer} **Default:** `buffer.byteLength - offset` * `position` {integer|bigint} **Default:** `null` * Returns: {number} diff --git a/lib/fs.js b/lib/fs.js index b8a5cefa1b0aa8..611eaaa90c6c9c 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -604,7 +604,7 @@ function read(fd, buffer, offset, length, position, callback) { if (arguments.length <= 3) { // Assume fs.read(fd, options, callback) - let options = {}; + let options = ObjectCreate(null); if (arguments.length < 3) { // This is fs.read(fd, callback) // buffer will be the callback @@ -621,7 +621,7 @@ function read(fd, buffer, offset, length, position, callback) { buffer = Buffer.alloc(16384), offset = 0, length = buffer.byteLength - offset, - position + position = null } = options); } @@ -686,10 +686,14 @@ function readSync(fd, buffer, offset, length, position) { validateBuffer(buffer); if (arguments.length <= 3) { - // Assume fs.read(fd, buffer, options) - const options = offset || {}; + // Assume fs.readSync(fd, buffer, options) + const options = offset || ObjectCreate(null); - ({ offset = 0, length = buffer.byteLength, position } = options); + ({ + offset = 0, + length = buffer.byteLength - offset, + position = null + } = options); } if (offset == null) { diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index fbc4de640ade3c..868c7df2f1ed79 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -5,6 +5,7 @@ const { Error, MathMax, MathMin, + ObjectCreate, NumberIsSafeInteger, Promise, PromisePrototypeThen, @@ -510,18 +511,15 @@ async function open(path, flags, mode) { async function read(handle, bufferOrOptions, offset, length, position) { let buffer = bufferOrOptions; if (!isArrayBufferView(buffer)) { - if (bufferOrOptions === undefined) { - bufferOrOptions = {}; - } - if (bufferOrOptions.buffer) { - buffer = bufferOrOptions.buffer; - validateBuffer(buffer); - } else { - buffer = Buffer.alloc(16384); - } - offset = bufferOrOptions.offset || 0; - length = bufferOrOptions.length ?? buffer.byteLength; - position = bufferOrOptions.position ?? null; + bufferOrOptions ??= ObjectCreate(null); + ({ + buffer = Buffer.alloc(16384), + offset = 0, + length = buffer.byteLength - offset, + position = null + } = bufferOrOptions); + + validateBuffer(buffer); } if (offset == null) { diff --git a/test/parallel/test-fs-readSync-optional-params.js b/test/parallel/test-fs-readSync-optional-params.js index 37d3d24911db51..00f1a5531cf6ea 100644 --- a/test/parallel/test-fs-readSync-optional-params.js +++ b/test/parallel/test-fs-readSync-optional-params.js @@ -5,23 +5,53 @@ const fixtures = require('../common/fixtures'); const fs = require('fs'); const assert = require('assert'); const filepath = fixtures.path('x.txt'); -const fd = fs.openSync(filepath, 'r'); const expected = Buffer.from('xyz\n'); function runTest(defaultBuffer, options) { - const result = fs.readSync(fd, defaultBuffer, options); - assert.strictEqual(result, expected.length); - assert.deepStrictEqual(defaultBuffer, expected); + let fd; + try { + fd = fs.openSync(filepath, 'r'); + const result = fs.readSync(fd, defaultBuffer, options); + assert.strictEqual(result, expected.length); + assert.deepStrictEqual(defaultBuffer, expected); + } finally { + if (fd != null) fs.closeSync(fd); + } } -// Test passing in an empty options object -runTest(Buffer.allocUnsafe(expected.length), { position: 0 }); +for (const options of [ -// Test not passing in any options object -runTest(Buffer.allocUnsafe(expected.length)); + // Test options object + { offset: 0 }, + { length: expected.length }, + { position: 0 }, + { offset: 0, length: expected.length }, + { offset: 0, position: 0 }, + { length: expected.length, position: 0 }, + { offset: 0, length: expected.length, position: 0 }, -// Test passing in options -runTest(Buffer.allocUnsafe(expected.length), { offset: 0, - length: expected.length, - position: 0 }); + { offset: null }, + { position: null }, + { position: -1 }, + { position: 0n }, + + // Test default params + {}, + null, + undefined, + + // Test if bad params are interpreted as default (not mandatory) + false, + true, + Infinity, + 42n, + Symbol(), + + // Test even more malicious corner cases + '4'.repeat(expected.length), + new String('4444'), + [4, 4, 4, 4], +]) { + runTest(Buffer.allocUnsafe(expected.length), options); +}