Skip to content

Commit 576c1ae

Browse files
LiviaMedeirossxa
authored andcommitted
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-
letoptions={};
608+
letoptions=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-
constoptions=offset||{};
690+
// Assume fs.readSync(fd, buffer, options)
691+
constoptions=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
asyncfunctionread(handle,bufferOrOptions,offset,length,position){
511512
letbuffer=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
constfs=require('fs');
66
constassert=require('assert');
77
constfilepath=fixtures.path('x.txt');
8-
constfd=fs.openSync(filepath,'r');
98

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

1211
functionrunTest(defaultBuffer,options){
13-
constresult=fs.readSync(fd,defaultBuffer,options);
14-
assert.strictEqual(result,expected.length);
15-
assert.deepStrictEqual(defaultBuffer,expected);
12+
letfd;
13+
try{
14+
fd=fs.openSync(filepath,'r');
15+
constresult=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(constoptionsof[
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+
newString('4444'),
54+
[4,4,4,4],
55+
]){
56+
runTest(Buffer.allocUnsafe(expected.length),options);
57+
}

0 commit comments

Comments
 (0)