From 967aa51d85e90ca06afdb3f572843ee3baad0b63 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 18 Jan 2020 10:55:31 +0100 Subject: [PATCH 1/2] lib,src: switch Buffer::kMaxLength to size_t Change the type of `Buffer::kMaxLength` to size_t because upcoming changes in V8 will allow typed arrays > 2 GB on 64 bits platforms. Not all platforms handle file reads and writes > 2 GB though so keep enforcing the 2 GB typed array limit for I/O operations. Fixes: https://github.com/nodejs/node/issues/31399 Refs: https://github.com/libuv/libuv/pull/1501 --- lib/fs.js | 10 +++++++--- lib/internal/errors.js | 4 +--- lib/internal/fs/promises.js | 8 ++++++-- src/node_buffer.h | 2 +- .../test-fs-util-validateoffsetlengthwrite.js | 11 +++++++---- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 66113899a5bfdc..40453bc15c1efb 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -24,6 +24,10 @@ 'use strict'; +// Most platforms don't allow reads or writes >= 2 GB. +// See https://github.com/libuv/libuv/pull/1501. +const kIoMaxLength = 2 ** 31 - 1; + const { Map, MathMax, @@ -52,7 +56,7 @@ const { const pathModule = require('path'); const { isArrayBufferView } = require('internal/util/types'); const binding = internalBinding('fs'); -const { Buffer, kMaxLength } = require('buffer'); +const { Buffer } = require('buffer'); const { codes: { ERR_FS_FILE_TOO_LARGE, @@ -269,7 +273,7 @@ function readFileAfterStat(err, stats) { const size = context.size = isFileType(stats, S_IFREG) ? stats[8] : 0; - if (size > kMaxLength) { + if (size > kIoMaxLength) { err = new ERR_FS_FILE_TOO_LARGE(size); return context.close(err); } @@ -327,7 +331,7 @@ function tryCreateBuffer(size, fd, isUserFd) { let threw = true; let buffer; try { - if (size > kMaxLength) { + if (size > kIoMaxLength) { throw new ERR_FS_FILE_TOO_LARGE(size); } buffer = Buffer.allocUnsafe(size); diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 206724eacb6f52..f40c18375f2045 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -802,9 +802,7 @@ E('ERR_FALSY_VALUE_REJECTION', function(reason) { this.reason = reason; return 'Promise was rejected with falsy value'; }, Error); -E('ERR_FS_FILE_TOO_LARGE', 'File size (%s) is greater than possible Buffer: ' + - `${kMaxLength} bytes`, - RangeError); +E('ERR_FS_FILE_TOO_LARGE', 'File size (%s) is greater than 2 GB', RangeError); E('ERR_FS_INVALID_SYMLINK_TYPE', 'Symlink type must be one of "dir", "file", or "junction". Received "%s"', Error); // Switch to TypeError. The current implementation does not seem right diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 5f8be113f44c0c..dbc573431e9f89 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -1,5 +1,9 @@ 'use strict'; +// Most platforms don't allow reads or writes >= 2 GB. +// See https://github.com/libuv/libuv/pull/1501. +const kIoMaxLength = 2 ** 31 - 1; + const { MathMax, MathMin, @@ -15,7 +19,7 @@ const { S_IFREG } = internalBinding('constants').fs; const binding = internalBinding('fs'); -const { Buffer, kMaxLength } = require('buffer'); +const { Buffer } = require('buffer'); const { ERR_FS_FILE_TOO_LARGE, ERR_INVALID_ARG_TYPE, @@ -160,7 +164,7 @@ async function readFileHandle(filehandle, options) { size = 0; } - if (size > kMaxLength) + if (size > kIoMaxLength) throw new ERR_FS_FILE_TOO_LARGE(size); const chunks = []; diff --git a/src/node_buffer.h b/src/node_buffer.h index 11010017ce0df8..606a6f5caa3b11 100644 --- a/src/node_buffer.h +++ b/src/node_buffer.h @@ -29,7 +29,7 @@ namespace node { namespace Buffer { -static const unsigned int kMaxLength = v8::TypedArray::kMaxLength; +static const size_t kMaxLength = v8::TypedArray::kMaxLength; typedef void (*FreeCallback)(char* data, void* hint); diff --git a/test/parallel/test-fs-util-validateoffsetlengthwrite.js b/test/parallel/test-fs-util-validateoffsetlengthwrite.js index 49746b0c5dfd8a..57fb7a9bc70674 100644 --- a/test/parallel/test-fs-util-validateoffsetlengthwrite.js +++ b/test/parallel/test-fs-util-validateoffsetlengthwrite.js @@ -5,7 +5,10 @@ require('../common'); const assert = require('assert'); const { validateOffsetLengthWrite } = require('internal/fs/utils'); -const { kMaxLength } = require('buffer'); + +// Most platforms don't allow reads or writes >= 2 GB. +// See https://github.com/libuv/libuv/pull/1501. +const kIoMaxLength = 2 ** 31 - 1; // RangeError when offset > byteLength { @@ -23,11 +26,11 @@ const { kMaxLength } = require('buffer'); ); } -// RangeError when byteLength < kMaxLength, and length > byteLength - offset . +// RangeError when byteLength < kIoMaxLength, and length > byteLength - offset. { - const offset = kMaxLength - 150; + const offset = kIoMaxLength - 150; const length = 200; - const byteLength = kMaxLength - 100; + const byteLength = kIoMaxLength - 100; assert.throws( () => validateOffsetLengthWrite(offset, length, byteLength), { From df9296737eeb3a5335f1443d46218ad7d6159d83 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 20 Jan 2020 16:50:18 +0100 Subject: [PATCH 2/2] fixup! use Number::New() --- src/node_buffer.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index f091ac96209721..9c5e1a8c534d80 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -62,6 +62,7 @@ using v8::Local; using v8::Maybe; using v8::MaybeLocal; using v8::Nothing; +using v8::Number; using v8::Object; using v8::String; using v8::Uint32; @@ -1178,7 +1179,7 @@ void Initialize(Local target, target->Set(env->context(), FIXED_ONE_BYTE_STRING(env->isolate(), "kMaxLength"), - Integer::NewFromUnsigned(env->isolate(), kMaxLength)).Check(); + Number::New(env->isolate(), kMaxLength)).Check(); target->Set(env->context(), FIXED_ONE_BYTE_STRING(env->isolate(), "kStringMaxLength"),