FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: adjust default `length` for `fs.readSync` and fsPromises/`read` · nodejs/node@576c1ae · GitHub

/ node Public

Commit 576c1ae

Browse files
LiviaMedeiros
authored and
sxa
committed
fs: adjust default length for fs.readSync and fsPromises/read
Makes default length reasonable when nonzero offset is set. PR-URL: #42128 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 0b8efea commit 576c1ae

4 files changed

Lines changed: 62 additions & 30 deletions

File tree

‎doc/api/fs.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5185,7 +5185,7 @@ changes:
51855185
* `buffer` {Buffer|TypedArray|DataView}
51865186
* `options` {Object}
51875187
* `offset` {integer} **Default:** `0`
5188-
* `length` {integer} **Default:** `buffer.byteLength`
5188+
* `length` {integer} **Default:** `buffer.byteLength - offset`
51895189
* `position` {integer|bigint} **Default:** `null`
51905190
* Returns: {number}
51915191

‎lib/fs.js‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,7 @@ function read(fd, buffer, offset, length, position, callback) {
605605

606606
if (arguments.length <= 3) {
607607
// Assume fs.read(fd, options, callback)
608-
let options = {};
608+
let options = ObjectCreate(null);
609609
if (arguments.length < 3) {
610610
// This is fs.read(fd, callback)
611611
// buffer will be the callback
@@ -622,7 +622,7 @@ function read(fd, buffer, offset, length, position, callback) {
622622
buffer = Buffer.alloc(16384),
623623
offset = 0,
624624
length = buffer.byteLength - offset,
625-
position
625+
position = null
626626
} = options);
627627
}
628628

@@ -687,10 +687,14 @@ function readSync(fd, buffer, offset, length, position) {
687687
validateBuffer(buffer);
688688

689689
if (arguments.length <= 3) {
690-
// Assume fs.read(fd, buffer, options)
691-
const options = offset || {};
690+
// Assume fs.readSync(fd, buffer, options)
691+
const options = offset || ObjectCreate(null);
692692

693-
({ offset = 0, length = buffer.byteLength, position } = options);
693+
({
694+
offset = 0,
695+
length = buffer.byteLength - offset,
696+
position = null
697+
} = options);
694698
}
695699

696700
if (offset == null) {

‎lib/internal/fs/promises.js‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const {
55
Error,
66
MathMax,
77
MathMin,
8+
ObjectCreate,
89
NumberIsSafeInteger,
910
Promise,
1011
PromisePrototypeThen,
@@ -510,18 +511,15 @@ async function open(path, flags, mode) {
510511
async function read(handle, bufferOrOptions, offset, length, position) {
511512
let buffer = bufferOrOptions;
512513
if (!isArrayBufferView(buffer)) {
513-
if (bufferOrOptions === undefined) {
514-
bufferOrOptions = {};
515-
}
516-
if (bufferOrOptions.buffer) {
517-
buffer = bufferOrOptions.buffer;
518-
validateBuffer(buffer);
519-
} else {
520-
buffer = Buffer.alloc(16384);
521-
}
522-
offset = bufferOrOptions.offset || 0;
523-
length = bufferOrOptions.length ?? buffer.byteLength;
524-
position = bufferOrOptions.position ?? null;
514+
bufferOrOptions ??= ObjectCreate(null);
515+
({
516+
buffer = Buffer.alloc(16384),
517+
offset = 0,
518+
length = buffer.byteLength - offset,
519+
position = null
520+
} = bufferOrOptions);
521+
522+
validateBuffer(buffer);
525523
}
526524

527525
if (offset == null) {

‎test/parallel/test-fs-readSync-optional-params.js‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,53 @@ const fixtures = require('../common/fixtures');
55
const fs = require('fs');
66
const assert = require('assert');
77
const filepath = fixtures.path('x.txt');
8-
const fd = fs.openSync(filepath, 'r');
98

109
const expected = Buffer.from('xyz\n');
1110

1211
function runTest(defaultBuffer, options) {
13-
const result = fs.readSync(fd, defaultBuffer, options);
14-
assert.strictEqual(result, expected.length);
15-
assert.deepStrictEqual(defaultBuffer, expected);
12+
let fd;
13+
try {
14+
fd = fs.openSync(filepath, 'r');
15+
const result = fs.readSync(fd, defaultBuffer, options);
16+
assert.strictEqual(result, expected.length);
17+
assert.deepStrictEqual(defaultBuffer, expected);
18+
} finally {
19+
if (fd != null) fs.closeSync(fd);
20+
}
1621
}
1722

18-
// Test passing in an empty options object
19-
runTest(Buffer.allocUnsafe(expected.length), { position: 0 });
23+
for (const options of [
2024

21-
// Test not passing in any options object
22-
runTest(Buffer.allocUnsafe(expected.length));
25+
// Test options object
26+
{ offset: 0 },
27+
{ length: expected.length },
28+
{ position: 0 },
29+
{ offset: 0, length: expected.length },
30+
{ offset: 0, position: 0 },
31+
{ length: expected.length, position: 0 },
32+
{ offset: 0, length: expected.length, position: 0 },
2333

24-
// Test passing in options
25-
runTest(Buffer.allocUnsafe(expected.length), { offset: 0,
26-
length: expected.length,
27-
position: 0 });
34+
{ offset: null },
35+
{ position: null },
36+
{ position: -1 },
37+
{ position: 0n },
38+
39+
// Test default params
40+
{},
41+
null,
42+
undefined,
43+
44+
// Test if bad params are interpreted as default (not mandatory)
45+
false,
46+
true,
47+
Infinity,
48+
42n,
49+
Symbol(),
50+
51+
// Test even more malicious corner cases
52+
'4'.repeat(expected.length),
53+
new String('4444'),
54+
[4, 4, 4, 4],
55+
]) {
56+
runTest(Buffer.allocUnsafe(expected.length), options);
57+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL