From 5e3b4d6ed957ca03ae7178c7697549f470b6d86e Mon Sep 17 00:00:00 2001 From: Zach Bjornson Date: Wed, 27 Mar 2019 15:23:59 -0700 Subject: [PATCH] fs: allow int64 offset in fs.write/writeSync/fd.write Ref https://github.com/nodejs/node/issues/26563 PR-URL: https://github.com/nodejs/node/pull/26572 Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: Ujjwal Sharma Reviewed-By: James M Snell Reviewed-By: Rich Trott --- lib/fs.js | 10 ++++++-- lib/internal/fs/promises.js | 5 +++- lib/internal/fs/utils.js | 7 +++--- src/node_file.cc | 10 ++++---- .../test-fs-util-validateoffsetlengthwrite.js | 16 ------------- test/parallel/test-fs-write-buffer.js | 24 +++++++++++++++++++ 6 files changed, 45 insertions(+), 27 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index ecb61aefa6aa31..2b962cc373242c 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -541,8 +541,11 @@ function write(fd, buffer, offset, length, position, callback) { if (isArrayBufferView(buffer)) { callback = maybeCallback(callback || position || length || offset); - if (typeof offset !== 'number') + if (offset == null || typeof offset === 'function') { offset = 0; + } else { + validateSafeInteger(offset, 'offset'); + } if (typeof length !== 'number') length = buffer.length - offset; if (typeof position !== 'number') @@ -580,8 +583,11 @@ function writeSync(fd, buffer, offset, length, position) { if (isArrayBufferView(buffer)) { if (position === undefined) position = null; - if (typeof offset !== 'number') + if (offset == null) { offset = 0; + } else { + validateSafeInteger(offset, 'offset'); + } if (typeof length !== 'number') length = buffer.byteLength - offset; validateOffsetLengthWrite(offset, length, buffer.byteLength); diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 67237c954babc1..6364fb6e9db0e4 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -240,8 +240,11 @@ async function write(handle, buffer, offset, length, position) { return { bytesWritten: 0, buffer }; if (isUint8Array(buffer)) { - if (typeof offset !== 'number') + if (offset == null) { offset = 0; + } else { + validateSafeInteger(offset, 'offset'); + } if (typeof length !== 'number') length = buffer.length - offset; if (typeof position !== 'number') diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index e91918a00772cf..2924f7582e7663 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -2,7 +2,7 @@ const { Object, Reflect } = primordials; -const { Buffer, kMaxLength } = require('buffer'); +const { Buffer } = require('buffer'); const { codes: { ERR_FS_INVALID_SYMLINK_TYPE, @@ -476,9 +476,8 @@ const validateOffsetLengthWrite = hideStackFrames( throw new ERR_OUT_OF_RANGE('offset', `<= ${byteLength}`, offset); } - const max = byteLength > kMaxLength ? kMaxLength : byteLength; - if (length > max - offset) { - throw new ERR_OUT_OF_RANGE('length', `<= ${max - offset}`, length); + if (length > byteLength - offset) { + throw new ERR_OUT_OF_RANGE('length', `<= ${byteLength - offset}`, length); } } ); diff --git a/src/node_file.cc b/src/node_file.cc index 4301cdc7b4e410..c4e3b83392c5cc 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -88,7 +88,7 @@ constexpr char kPathSeparator = '/'; const char* const kPathSeparator = "\\/"; #endif -#define GET_OFFSET(a) ((a)->IsNumber() ? (a).As()->Value() : -1) +#define GET_OFFSET(a) (IsSafeJsInt(a) ? (a).As()->Value() : -1) #define TRACE_NAME(name) "fs.sync." #name #define GET_TRACE_ENABLED \ (*TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED \ @@ -1669,9 +1669,11 @@ static void WriteBuffer(const FunctionCallbackInfo& args) { char* buffer_data = Buffer::Data(buffer_obj); size_t buffer_length = Buffer::Length(buffer_obj); - CHECK(args[2]->IsInt32()); - const size_t off = static_cast(args[2].As()->Value()); - CHECK_LE(off, buffer_length); + CHECK(IsSafeJsInt(args[2])); + const int64_t off_64 = args[2].As()->Value(); + CHECK_GE(off_64, 0); + CHECK_LE(static_cast(off_64), buffer_length); + const size_t off = static_cast(off_64); CHECK(args[3]->IsInt32()); const size_t len = static_cast(args[3].As()->Value()); diff --git a/test/parallel/test-fs-util-validateoffsetlengthwrite.js b/test/parallel/test-fs-util-validateoffsetlengthwrite.js index 9aca956bd505bf..f2f80ebdaa577a 100644 --- a/test/parallel/test-fs-util-validateoffsetlengthwrite.js +++ b/test/parallel/test-fs-util-validateoffsetlengthwrite.js @@ -22,22 +22,6 @@ const { kMaxLength } = require('buffer'); ); } -// RangeError when byteLength > kMaxLength, and length > kMaxLength - offset . -{ - const offset = kMaxLength; - const length = 100; - const byteLength = kMaxLength + 1; - common.expectsError( - () => validateOffsetLengthWrite(offset, length, byteLength), - { - code: 'ERR_OUT_OF_RANGE', - type: RangeError, - message: 'The value of "length" is out of range. ' + - `It must be <= ${kMaxLength - offset}. Received ${length}` - } - ); -} - // RangeError when byteLength < kMaxLength, and length > byteLength - offset . { const offset = kMaxLength - 150; diff --git a/test/parallel/test-fs-write-buffer.js b/test/parallel/test-fs-write-buffer.js index 362571f2479c84..b2cd299bb186a7 100644 --- a/test/parallel/test-fs-write-buffer.js +++ b/test/parallel/test-fs-write-buffer.js @@ -148,3 +148,27 @@ tmpdir.refresh(); fs.write(fd, Uint8Array.from(expected), cb); })); } + +// fs.write with invalid offset type +{ + const filename = path.join(tmpdir.path, 'write7.txt'); + fs.open(filename, 'w', 0o644, common.mustCall((err, fd) => { + assert.ifError(err); + + assert.throws(() => { + fs.write(fd, + Buffer.from('abcd'), + NaN, + expected.length, + 0, + common.mustNotCall()); + }, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "offset" is out of range. ' + + 'It must be an integer. Received NaN' + }); + + fs.closeSync(fd); + })); +}