Skip to content

Commit

Permalink
lib,src: switch Buffer::kMaxLength to size_t
Browse files Browse the repository at this point in the history
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: #31399
Refs: libuv/libuv#1501

PR-URL: #31406
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
  • Loading branch information
bnoordhuis authored and Trott committed Jan 21, 2020
1 parent da66054 commit 5005c3c
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 14 deletions.
10 changes: 7 additions & 3 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 1 addition & 3 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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 = [];
Expand Down
3 changes: 2 additions & 1 deletion src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1178,7 +1179,7 @@ void Initialize(Local<Object> 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"),
Expand Down
2 changes: 1 addition & 1 deletion src/node_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
11 changes: 7 additions & 4 deletions test/parallel/test-fs-util-validateoffsetlengthwrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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),
{
Expand Down

0 comments on commit 5005c3c

Please sign in to comment.