Skip to content

Commit a45c38f

Browse files
RaisinTenBethGriggs
authored andcommitted
fs: allow position parameter to be a BigInt in read and readSync
Fixes: #36185 PR-URL: #36190 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Zeyu Yang <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent 0b28e43 commit a45c38f

File tree

4 files changed

+127
-10
lines changed

4 files changed

+127
-10
lines changed

doc/api/fs.md

+4-4
Original file line numberDiff line numberDiff line change
@@ -2904,7 +2904,7 @@ changes:
29042904
* `buffer` {Buffer|TypedArray|DataView}
29052905
* `offset` {integer}
29062906
* `length` {integer}
2907-
* `position` {integer}
2907+
* `position` {integer|bigint}
29082908
* `callback` {Function}
29092909
* `err` {Error}
29102910
* `bytesRead` {integer}
@@ -2945,7 +2945,7 @@ changes:
29452945
* `buffer` {Buffer|TypedArray|DataView} **Default:** `Buffer.alloc(16384)`
29462946
* `offset` {integer} **Default:** `0`
29472947
* `length` {integer} **Default:** `buffer.length`
2948-
* `position` {integer} **Default:** `null`
2948+
* `position` {integer|bigint} **Default:** `null`
29492949
* `callback` {Function}
29502950
* `err` {Error}
29512951
* `bytesRead` {integer}
@@ -3264,7 +3264,7 @@ changes:
32643264
* `buffer` {Buffer|TypedArray|DataView}
32653265
* `offset` {integer}
32663266
* `length` {integer}
3267-
* `position` {integer}
3267+
* `position` {integer|bigint}
32683268
* Returns: {number}
32693269

32703270
Returns the number of `bytesRead`.
@@ -3287,7 +3287,7 @@ changes:
32873287
* `options` {Object}
32883288
* `offset` {integer} **Default:** `0`
32893289
* `length` {integer} **Default:** `buffer.length`
3290-
* `position` {integer} **Default:** `null`
3290+
* `position` {integer|bigint} **Default:** `null`
32913291
* Returns: {number}
32923292

32933293
Returns the number of `bytesRead`.

lib/fs.js

+32-4
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ const {
3636
Map,
3737
MathMax,
3838
Number,
39-
NumberIsSafeInteger,
4039
ObjectCreate,
4140
ObjectDefineProperties,
4241
ObjectDefineProperty,
@@ -69,7 +68,8 @@ const {
6968
ERR_INVALID_ARG_VALUE,
7069
ERR_INVALID_ARG_TYPE,
7170
ERR_INVALID_CALLBACK,
72-
ERR_FEATURE_UNAVAILABLE_ON_PLATFORM
71+
ERR_FEATURE_UNAVAILABLE_ON_PLATFORM,
72+
ERR_OUT_OF_RANGE,
7373
},
7474
hideStackFrames,
7575
uvErrmapGet,
@@ -553,9 +553,23 @@ function read(fd, buffer, offset, length, position, callback) {
553553

554554
validateOffsetLengthRead(offset, length, buffer.byteLength);
555555

556-
if (!NumberIsSafeInteger(position))
556+
if (position == null)
557557
position = -1;
558558

559+
if (typeof position === 'number') {
560+
validateInteger(position, 'position');
561+
} else if (typeof position === 'bigint') {
562+
if (!(position >= -(2n ** 63n) && position <= 2n ** 63n - 1n)) {
563+
throw new ERR_OUT_OF_RANGE('position',
564+
`>= ${-(2n ** 63n)} && <= ${2n ** 63n - 1n}`,
565+
position);
566+
}
567+
} else {
568+
throw new ERR_INVALID_ARG_TYPE('position',
569+
['integer', 'bigint'],
570+
position);
571+
}
572+
559573
function wrapper(err, bytesRead) {
560574
// Retain a reference to buffer so that it can't be GC'ed too soon.
561575
callback(err, bytesRead || 0, buffer);
@@ -605,9 +619,23 @@ function readSync(fd, buffer, offset, length, position) {
605619

606620
validateOffsetLengthRead(offset, length, buffer.byteLength);
607621

608-
if (!NumberIsSafeInteger(position))
622+
if (position == null)
609623
position = -1;
610624

625+
if (typeof position === 'number') {
626+
validateInteger(position, 'position');
627+
} else if (typeof position === 'bigint') {
628+
if (!(position >= -(2n ** 63n) && position <= 2n ** 63n - 1n)) {
629+
throw new ERR_OUT_OF_RANGE('position',
630+
`>= ${-(2n ** 63n)} && <= ${2n ** 63n - 1n}`,
631+
position);
632+
}
633+
} else {
634+
throw new ERR_INVALID_ARG_TYPE('position',
635+
['integer', 'bigint'],
636+
position);
637+
}
638+
611639
const ctx = {};
612640
const result = binding.read(fd, buffer, offset, length, position,
613641
undefined, ctx);

src/node_file.cc

+5-2
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ namespace node {
5151
namespace fs {
5252

5353
using v8::Array;
54+
using v8::BigInt;
5455
using v8::Boolean;
5556
using v8::Context;
5657
using v8::EscapableHandleScope;
@@ -2037,8 +2038,10 @@ static void Read(const FunctionCallbackInfo<Value>& args) {
20372038
const size_t len = static_cast<size_t>(args[3].As<Int32>()->Value());
20382039
CHECK(Buffer::IsWithinBounds(off, len, buffer_length));
20392040

2040-
CHECK(IsSafeJsInt(args[4]));
2041-
const int64_t pos = args[4].As<Integer>()->Value();
2041+
CHECK(IsSafeJsInt(args[4]) || args[4]->IsBigInt());
2042+
const int64_t pos = args[4]->IsNumber() ?
2043+
args[4].As<Integer>()->Value() :
2044+
args[4].As<BigInt>()->Int64Value();
20422045

20432046
char* buf = buffer_data + off;
20442047
uv_buf_t uvbuf = uv_buf_init(buf, len);

test/parallel/test-fs-read-type.js

+86
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,47 @@ assert.throws(() => {
7474
'It must be >= 0. Received -1'
7575
});
7676

77+
[true, () => {}, {}, ''].forEach((value) => {
78+
assert.throws(() => {
79+
fs.read(fd,
80+
Buffer.allocUnsafe(expected.length),
81+
0,
82+
expected.length,
83+
value,
84+
common.mustNotCall());
85+
}, {
86+
code: 'ERR_INVALID_ARG_TYPE',
87+
name: 'TypeError'
88+
});
89+
});
90+
91+
[0.5, 2 ** 53, 2n ** 63n].forEach((value) => {
92+
assert.throws(() => {
93+
fs.read(fd,
94+
Buffer.allocUnsafe(expected.length),
95+
0,
96+
expected.length,
97+
value,
98+
common.mustNotCall());
99+
}, {
100+
code: 'ERR_OUT_OF_RANGE',
101+
name: 'RangeError'
102+
});
103+
});
104+
105+
fs.read(fd,
106+
Buffer.allocUnsafe(expected.length),
107+
0,
108+
expected.length,
109+
0n,
110+
common.mustCall());
111+
112+
fs.read(fd,
113+
Buffer.allocUnsafe(expected.length),
114+
0,
115+
expected.length,
116+
2n ** 53n - 1n,
117+
common.mustCall());
77118

78119
assert.throws(
79120
() => fs.readSync(fd, expected.length, 0, 'utf-8'),
@@ -147,3 +188,48 @@ assert.throws(() => {
147188
message: 'The value of "length" is out of range. ' +
148189
'It must be <= 4. Received 5'
149190
});
191+
192+
[true, () => {}, {}, ''].forEach((value) => {
193+
assert.throws(() => {
194+
fs.readSync(fd,
195+
Buffer.allocUnsafe(expected.length),
196+
0,
197+
expected.length,
198+
value);
199+
}, {
200+
code: 'ERR_INVALID_ARG_TYPE',
201+
name: 'TypeError'
202+
});
203+
});
204+
205+
[0.5, 2 ** 53, 2n ** 63n].forEach((value) => {
206+
assert.throws(() => {
207+
fs.readSync(fd,
208+
Buffer.allocUnsafe(expected.length),
209+
0,
210+
expected.length,
211+
value);
212+
}, {
213+
code: 'ERR_OUT_OF_RANGE',
214+
name: 'RangeError'
215+
});
216+
});
217+
218+
fs.readSync(fd,
219+
Buffer.allocUnsafe(expected.length),
220+
0,
221+
expected.length,
222+
0n);
223+
224+
try {
225+
fs.readSync(fd,
226+
Buffer.allocUnsafe(expected.length),
227+
0,
228+
expected.length,
229+
2n ** 53n - 1n);
230+
} catch (err) {
231+
// On systems where max file size is below 2^53-1, we'd expect a EFBIG error.
232+
// This is not using `assert.throws` because the above call should not raise
233+
// any error on systems that allows file of that size.
234+
if (err.code !== 'EFBIG') throw err;
235+
}

0 commit comments

Comments
 (0)