Skip to content

Commit 63eb267

Browse files
joyeecheungjoyeecheung
committed
src: migrate string_bytes.cc to throw errors with code
PR-URL: #19739 Fixes: #3175 Fixes: #9489 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 289d152 commit 63eb267

8 files changed

+81
-44
lines changed

‎src/string_bytes.cc‎

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
#include "base64.h"
2525
#include "node_internals.h"
26+
#include "node_errors.h"
2627
#include "node_buffer.h"
2728

2829
#include <limits.h>
@@ -36,20 +37,6 @@
3637
// use external string resources.
3738
#define EXTERN_APEX 0xFBEE9
3839

39-
// TODO(addaleax): These should all have better error messages. In particular,
40-
// they should mention what the actual limits are.
41-
#define SB_MALLOC_FAILED_ERROR \
42-
v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))
43-
44-
#define SB_STRING_TOO_LONG_ERROR \
45-
v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))
46-
47-
#define SB_BUFFER_CREATION_ERROR \
48-
v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))
49-
50-
#define SB_BUFFER_SIZE_EXCEEDED_ERROR \
51-
v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))
52-
5340
namespace node {
5441

5542
using v8::HandleScope;
@@ -93,7 +80,7 @@ class ExternString: public ResourceType {
9380

9481
TypeName* new_data = node::UncheckedMalloc<TypeName>(length);
9582
if (new_data == nullptr) {
96-
*error = SB_MALLOC_FAILED_ERROR;
83+
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
9784
return MaybeLocal<Value>();
9885
}
9986
memcpy(new_data, data, length * sizeof(*new_data));
@@ -126,7 +113,7 @@ class ExternString: public ResourceType {
126113

127114
if (str.IsEmpty()) {
128115
delete h_str;
129-
*error = SB_STRING_TOO_LONG_ERROR;
116+
*error = node::ERR_STRING_TOO_LARGE(isolate);
130117
return MaybeLocal<Value>();
131118
}
132119

@@ -183,7 +170,7 @@ MaybeLocal<Value> ExternOneByteString::NewSimpleFromCopy(Isolate* isolate,
183170
v8::NewStringType::kNormal,
184171
length);
185172
if (str.IsEmpty()) {
186-
*error = SB_STRING_TOO_LONG_ERROR;
173+
*error = node::ERR_STRING_TOO_LARGE(isolate);
187174
return MaybeLocal<Value>();
188175
}
189176
return str.ToLocalChecked();
@@ -201,7 +188,7 @@ MaybeLocal<Value> ExternTwoByteString::NewSimpleFromCopy(Isolate* isolate,
201188
v8::NewStringType::kNormal,
202189
length);
203190
if (str.IsEmpty()) {
204-
*error = SB_STRING_TOO_LONG_ERROR;
191+
*error = node::ERR_STRING_TOO_LARGE(isolate);
205192
return MaybeLocal<Value>();
206193
}
207194
return str.ToLocalChecked();
@@ -616,7 +603,7 @@ static size_t hex_encode(const char* src, size_t slen, char* dst, size_t dlen) {
616603
#define CHECK_BUFLEN_IN_RANGE(len) \
617604
do { \
618605
if ((len) > Buffer::kMaxLength) { \
619-
*error = SB_BUFFER_SIZE_EXCEEDED_ERROR; \
606+
*error = node::ERR_BUFFER_TOO_LARGE(isolate); \
620607
return MaybeLocal<Value>(); \
621608
} \
622609
} while (0)
@@ -639,9 +626,13 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
639626
switch (encoding) {
640627
case BUFFER:
641628
{
629+
if (buflen > node::Buffer::kMaxLength) {
630+
*error = node::ERR_BUFFER_TOO_LARGE(isolate);
631+
return MaybeLocal<Value>();
632+
}
642633
auto maybe_buf = Buffer::Copy(isolate, buf, buflen);
643634
if (maybe_buf.IsEmpty()) {
644-
*error = SB_BUFFER_CREATION_ERROR;
635+
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
645636
return MaybeLocal<Value>();
646637
}
647638
return maybe_buf.ToLocalChecked();
@@ -651,7 +642,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
651642
if (contains_non_ascii(buf, buflen)) {
652643
char* out = node::UncheckedMalloc(buflen);
653644
if (out == nullptr) {
654-
*error = SB_MALLOC_FAILED_ERROR;
645+
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
655646
return MaybeLocal<Value>();
656647
}
657648
force_ascii(buf, out, buflen);
@@ -666,7 +657,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
666657
v8::NewStringType::kNormal,
667658
buflen);
668659
if (val.IsEmpty()) {
669-
*error = SB_STRING_TOO_LONG_ERROR;
660+
*error = node::ERR_STRING_TOO_LARGE(isolate);
670661
return MaybeLocal<Value>();
671662
}
672663
return val.ToLocalChecked();
@@ -678,7 +669,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
678669
size_t dlen = base64_encoded_size(buflen);
679670
char* dst = node::UncheckedMalloc(dlen);
680671
if (dst == nullptr) {
681-
*error = SB_MALLOC_FAILED_ERROR;
672+
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
682673
return MaybeLocal<Value>();
683674
}
684675

@@ -692,7 +683,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
692683
size_t dlen = buflen * 2;
693684
char* dst = node::UncheckedMalloc(dlen);
694685
if (dst == nullptr) {
695-
*error = SB_MALLOC_FAILED_ERROR;
686+
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
696687
return MaybeLocal<Value>();
697688
}
698689
size_t written = hex_encode(buf, buflen, dst, dlen);
@@ -723,7 +714,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
723714
if (IsBigEndian()) {
724715
uint16_t* dst = node::UncheckedMalloc<uint16_t>(buflen);
725716
if (dst == nullptr) {
726-
*error = SB_MALLOC_FAILED_ERROR;
717+
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
727718
return MaybeLocal<Value>();
728719
}
729720
size_t nbytes = buflen * sizeof(uint16_t);

‎test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-ascii.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ if (!common.enoughTestMem)
66
common.skip(skipMessage);
77

88
const binding = require(`./build/${common.buildType}/binding`);
9-
const assert = require('assert');
109

1110
// v8 fails silently if string length > v8::String::kMaxLength
1211
// v8::String::kMaxLength defined in v8.h
@@ -25,6 +24,11 @@ try {
2524
if (!binding.ensureAllocation(2 * kStringMaxLength))
2625
common.skip(skipMessage);
2726

28-
assert.throws(function() {
27+
const stringLengthHex = kStringMaxLength.toString(16);
28+
common.expectsError(function() {
2929
buf.toString('ascii');
30-
}, /"toString\(\)" failed/);
30+
}, {
31+
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
32+
code: 'ERR_STRING_TOO_LARGE',
33+
type: Error
34+
});

‎test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-base64.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ if (!common.enoughTestMem)
66
common.skip(skipMessage);
77

88
const binding = require(`./build/${common.buildType}/binding`);
9-
const assert = require('assert');
109

1110
// v8 fails silently if string length > v8::String::kMaxLength
1211
// v8::String::kMaxLength defined in v8.h
@@ -25,6 +24,11 @@ try {
2524
if (!binding.ensureAllocation(2 * kStringMaxLength))
2625
common.skip(skipMessage);
2726

28-
assert.throws(function() {
27+
const stringLengthHex = kStringMaxLength.toString(16);
28+
common.expectsError(function() {
2929
buf.toString('base64');
30-
}, /"toString\(\)" failed/);
30+
}, {
31+
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
32+
code: 'ERR_STRING_TOO_LARGE',
33+
type: Error
34+
});

‎test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,14 @@ try {
2525
if (!binding.ensureAllocation(2 * kStringMaxLength))
2626
common.skip(skipMessage);
2727

28-
assert.throws(function() {
28+
const stringLengthHex = kStringMaxLength.toString(16);
29+
common.expectsError(function() {
2930
buf.toString('latin1');
30-
}, /"toString\(\)" failed/);
31+
}, {
32+
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
33+
code: 'ERR_STRING_TOO_LARGE',
34+
type: Error
35+
});
3136

3237
let maxString = buf.toString('latin1', 1);
3338
assert.strictEqual(maxString.length, kStringMaxLength);

‎test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-hex.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ if (!common.enoughTestMem)
66
common.skip(skipMessage);
77

88
const binding = require(`./build/${common.buildType}/binding`);
9-
const assert = require('assert');
109

1110
// v8 fails silently if string length > v8::String::kMaxLength
1211
// v8::String::kMaxLength defined in v8.h
@@ -25,6 +24,11 @@ try {
2524
if (!binding.ensureAllocation(2 * kStringMaxLength))
2625
common.skip(skipMessage);
2726

28-
assert.throws(function() {
27+
const stringLengthHex = kStringMaxLength.toString(16);
28+
common.expectsError(function() {
2929
buf.toString('hex');
30-
}, /"toString\(\)" failed/);
30+
}, {
31+
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
32+
code: 'ERR_STRING_TOO_LARGE',
33+
type: Error
34+
});

‎test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,27 @@ try {
2525
if (!binding.ensureAllocation(2 * kStringMaxLength))
2626
common.skip(skipMessage);
2727

28+
const stringLengthHex = kStringMaxLength.toString(16);
29+
2830
assert.throws(function() {
2931
buf.toString();
30-
}, /"toString\(\)" failed|Array buffer allocation failed/);
32+
}, function(e) {
33+
if (e.message !== 'Array buffer allocation failed') {
34+
common.expectsError({
35+
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
36+
code: 'ERR_STRING_TOO_LARGE',
37+
type: Error
38+
})(e);
39+
return true;
40+
} else {
41+
return true;
42+
}
43+
});
3144

32-
assert.throws(function() {
45+
common.expectsError(function() {
3346
buf.toString('utf8');
34-
}, /"toString\(\)" failed/);
47+
}, {
48+
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
49+
code: 'ERR_STRING_TOO_LARGE',
50+
type: Error
51+
});

‎test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max.js‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ if (!common.enoughTestMem)
66
common.skip(skipMessage);
77

88
const binding = require(`./build/${common.buildType}/binding`);
9-
const assert = require('assert');
109

1110
// v8 fails silently if string length > v8::String::kMaxLength
1211
// v8::String::kMaxLength defined in v8.h
@@ -25,6 +24,12 @@ try {
2524
if (!binding.ensureAllocation(2 * kStringMaxLength))
2625
common.skip(skipMessage);
2726

28-
assert.throws(function() {
27+
const stringLengthHex = kStringMaxLength.toString(16);
28+
29+
common.expectsError(function() {
2930
buf.toString('utf16le');
30-
}, /"toString\(\)" failed/);
31+
}, {
32+
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
33+
code: 'ERR_STRING_TOO_LARGE',
34+
type: Error
35+
});

‎test/sequential/test-fs-readfile-tostring-fail.js‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,15 @@ stream.end();
3232
stream.on('finish', common.mustCall(function() {
3333
fs.readFile(file, 'utf8', common.mustCall(function(err, buf) {
3434
assert.ok(err instanceof Error);
35-
assert.ok(/^(Array buffer allocation failed|"toString\(\)" failed)$/
36-
.test(err.message));
35+
if (err.message !== 'Array buffer allocation failed') {
36+
const stringLengthHex = kStringMaxLength.toString(16);
37+
common.expectsError({
38+
message: 'Cannot create a string larger than ' +
39+
`0x${stringLengthHex} bytes`,
40+
code: 'ERR_STRING_TOO_LARGE',
41+
type: Error
42+
})(err);
43+
}
3744
assert.strictEqual(buf, undefined);
3845
}));
3946
}));

0 commit comments

Comments
 (0)