From f21f7175e62adc06dd31bb0f968fde0a140b7251 Mon Sep 17 00:00:00 2001 From: raisinten Date: Fri, 20 Nov 2020 19:25:41 +0530 Subject: [PATCH] fs: allow `position` parameter to be a `BigInt` in read and readSync Fixes: https://github.com/nodejs/node/issues/36185 PR-URL: https://github.com/nodejs/node/pull/36190 Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell Reviewed-By: Rich Trott Reviewed-By: Zeyu Yang Reviewed-By: Joyee Cheung Reviewed-By: Antoine du Hamel --- doc/api/fs.md | 8 +-- lib/fs.js | 36 +++++++++++-- src/node_file.cc | 7 ++- test/parallel/test-fs-read-type.js | 86 ++++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 10 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 780d19e76ac181..971def3708826b 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -2904,7 +2904,7 @@ changes: * `buffer` {Buffer|TypedArray|DataView} * `offset` {integer} * `length` {integer} -* `position` {integer} +* `position` {integer|bigint} * `callback` {Function} * `err` {Error} * `bytesRead` {integer} @@ -2945,7 +2945,7 @@ changes: * `buffer` {Buffer|TypedArray|DataView} **Default:** `Buffer.alloc(16384)` * `offset` {integer} **Default:** `0` * `length` {integer} **Default:** `buffer.length` - * `position` {integer} **Default:** `null` + * `position` {integer|bigint} **Default:** `null` * `callback` {Function} * `err` {Error} * `bytesRead` {integer} @@ -3264,7 +3264,7 @@ changes: * `buffer` {Buffer|TypedArray|DataView} * `offset` {integer} * `length` {integer} -* `position` {integer} +* `position` {integer|bigint} * Returns: {number} Returns the number of `bytesRead`. @@ -3287,7 +3287,7 @@ changes: * `options` {Object} * `offset` {integer} **Default:** `0` * `length` {integer} **Default:** `buffer.length` - * `position` {integer} **Default:** `null` + * `position` {integer|bigint} **Default:** `null` * Returns: {number} Returns the number of `bytesRead`. diff --git a/lib/fs.js b/lib/fs.js index 7325a24f080182..3decda30c3757f 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -36,7 +36,6 @@ const { Map, MathMax, Number, - NumberIsSafeInteger, ObjectCreate, ObjectDefineProperties, ObjectDefineProperty, @@ -69,7 +68,8 @@ const { ERR_INVALID_ARG_VALUE, ERR_INVALID_ARG_TYPE, ERR_INVALID_CALLBACK, - ERR_FEATURE_UNAVAILABLE_ON_PLATFORM + ERR_FEATURE_UNAVAILABLE_ON_PLATFORM, + ERR_OUT_OF_RANGE, }, hideStackFrames, uvErrmapGet, @@ -553,9 +553,23 @@ function read(fd, buffer, offset, length, position, callback) { validateOffsetLengthRead(offset, length, buffer.byteLength); - if (!NumberIsSafeInteger(position)) + if (position == null) position = -1; + if (typeof position === 'number') { + validateInteger(position, 'position'); + } else if (typeof position === 'bigint') { + if (!(position >= -(2n ** 63n) && position <= 2n ** 63n - 1n)) { + throw new ERR_OUT_OF_RANGE('position', + `>= ${-(2n ** 63n)} && <= ${2n ** 63n - 1n}`, + position); + } + } else { + throw new ERR_INVALID_ARG_TYPE('position', + ['integer', 'bigint'], + position); + } + function wrapper(err, bytesRead) { // Retain a reference to buffer so that it can't be GC'ed too soon. callback(err, bytesRead || 0, buffer); @@ -605,9 +619,23 @@ function readSync(fd, buffer, offset, length, position) { validateOffsetLengthRead(offset, length, buffer.byteLength); - if (!NumberIsSafeInteger(position)) + if (position == null) position = -1; + if (typeof position === 'number') { + validateInteger(position, 'position'); + } else if (typeof position === 'bigint') { + if (!(position >= -(2n ** 63n) && position <= 2n ** 63n - 1n)) { + throw new ERR_OUT_OF_RANGE('position', + `>= ${-(2n ** 63n)} && <= ${2n ** 63n - 1n}`, + position); + } + } else { + throw new ERR_INVALID_ARG_TYPE('position', + ['integer', 'bigint'], + position); + } + const ctx = {}; const result = binding.read(fd, buffer, offset, length, position, undefined, ctx); diff --git a/src/node_file.cc b/src/node_file.cc index 58727908a2cfd9..2a7b6dfcec7779 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -51,6 +51,7 @@ namespace node { namespace fs { using v8::Array; +using v8::BigInt; using v8::Boolean; using v8::Context; using v8::EscapableHandleScope; @@ -2037,8 +2038,10 @@ static void Read(const FunctionCallbackInfo& args) { const size_t len = static_cast(args[3].As()->Value()); CHECK(Buffer::IsWithinBounds(off, len, buffer_length)); - CHECK(IsSafeJsInt(args[4])); - const int64_t pos = args[4].As()->Value(); + CHECK(IsSafeJsInt(args[4]) || args[4]->IsBigInt()); + const int64_t pos = args[4]->IsNumber() ? + args[4].As()->Value() : + args[4].As()->Int64Value(); char* buf = buffer_data + off; uv_buf_t uvbuf = uv_buf_init(buf, len); diff --git a/test/parallel/test-fs-read-type.js b/test/parallel/test-fs-read-type.js index 02298ed1c3523d..f28c3d34c699d1 100644 --- a/test/parallel/test-fs-read-type.js +++ b/test/parallel/test-fs-read-type.js @@ -74,6 +74,47 @@ assert.throws(() => { 'It must be >= 0. Received -1' }); +[true, () => {}, {}, ''].forEach((value) => { + assert.throws(() => { + fs.read(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + value, + common.mustNotCall()); + }, { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' + }); +}); + +[0.5, 2 ** 53, 2n ** 63n].forEach((value) => { + assert.throws(() => { + fs.read(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + value, + common.mustNotCall()); + }, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError' + }); +}); + +fs.read(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + 0n, + common.mustCall()); + +fs.read(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + 2n ** 53n - 1n, + common.mustCall()); assert.throws( () => fs.readSync(fd, expected.length, 0, 'utf-8'), @@ -147,3 +188,48 @@ assert.throws(() => { message: 'The value of "length" is out of range. ' + 'It must be <= 4. Received 5' }); + +[true, () => {}, {}, ''].forEach((value) => { + assert.throws(() => { + fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + value); + }, { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' + }); +}); + +[0.5, 2 ** 53, 2n ** 63n].forEach((value) => { + assert.throws(() => { + fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + value); + }, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError' + }); +}); + +fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + 0n); + +try { + fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + 2n ** 53n - 1n); +} catch (err) { + // On systems where max file size is below 2^53-1, we'd expect a EFBIG error. + // This is not using `assert.throws` because the above call should not raise + // any error on systems that allows file of that size. + if (err.code !== 'EFBIG') throw err; +}