From 5005c3c72c232915935b97800781467767964660 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 18 Jan 2020 10:55:31 +0100 Subject: [PATCH] lib,src: switch Buffer::kMaxLength to size_t MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 PR-URL: https://github.com/nodejs/node/pull/31406 Reviewed-By: Richard Lau Reviewed-By: David Carlier Reviewed-By: Colin Ihrig Reviewed-By: Luigi Pinca Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott Reviewed-By: Tobias Nießen Reviewed-By: Shelley Vohr --- lib/fs.js | 10 +++++++--- lib/internal/errors.js | 4 +--- lib/internal/fs/promises.js | 8 ++++++-- src/node_buffer.cc | 3 ++- src/node_buffer.h | 2 +- .../test-fs-util-validateoffsetlengthwrite.js | 11 +++++++---- 6 files changed, 24 insertions(+), 14 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 edd286a20afee4..3dda42aef9ef33 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -803,9 +803,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 2fc6f8fb9589e3..e0c4e2ca4a49e7 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, @@ -163,7 +167,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.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"), 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), {