Skip to content

Commit

Permalink
src: migrate string_bytes.cc to throw errors with code
Browse files Browse the repository at this point in the history 
PR-URL: nodejs#19739
Fixes: nodejs#3175
Fixes: nodejs#9489
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
  • Loading branch information
@joyeecheung
joyeecheung committed Apr 7, 2018
1 parent 289d152 commit 63eb267
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 44 deletions.
41 changes: 16 additions & 25 deletions src/string_bytes.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#include "base64.h"
#include "node_internals.h"
#include "node_errors.h"
#include "node_buffer.h"

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

// TODO(addaleax): These should all have better error messages. In particular,
// they should mention what the actual limits are.
#define SB_MALLOC_FAILED_ERROR \
v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))

#define SB_STRING_TOO_LONG_ERROR \
v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))

#define SB_BUFFER_CREATION_ERROR \
v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))

#define SB_BUFFER_SIZE_EXCEEDED_ERROR \
v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))

namespace node {

using v8::HandleScope;
Expand Down Expand Up @@ -93,7 +80,7 @@ class ExternString: public ResourceType {

TypeName* new_data = node::UncheckedMalloc<TypeName>(length);
if (new_data == nullptr) {
*error = SB_MALLOC_FAILED_ERROR;
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
return MaybeLocal<Value>();
}
memcpy(new_data, data, length * sizeof(*new_data));
Expand Down Expand Up @@ -126,7 +113,7 @@ class ExternString: public ResourceType {

if (str.IsEmpty()) {
delete h_str;
*error = SB_STRING_TOO_LONG_ERROR;
*error = node::ERR_STRING_TOO_LARGE(isolate);
return MaybeLocal<Value>();
}

Expand Down Expand Up @@ -183,7 +170,7 @@ MaybeLocal<Value> ExternOneByteString::NewSimpleFromCopy(Isolate* isolate,
v8::NewStringType::kNormal,
length);
if (str.IsEmpty()) {
*error = SB_STRING_TOO_LONG_ERROR;
*error = node::ERR_STRING_TOO_LARGE(isolate);
return MaybeLocal<Value>();
}
return str.ToLocalChecked();
Expand All @@ -201,7 +188,7 @@ MaybeLocal<Value> ExternTwoByteString::NewSimpleFromCopy(Isolate* isolate,
v8::NewStringType::kNormal,
length);
if (str.IsEmpty()) {
*error = SB_STRING_TOO_LONG_ERROR;
*error = node::ERR_STRING_TOO_LARGE(isolate);
return MaybeLocal<Value>();
}
return str.ToLocalChecked();
Expand Down Expand Up @@ -616,7 +603,7 @@ static size_t hex_encode(const char* src, size_t slen, char* dst, size_t dlen) {
#define CHECK_BUFLEN_IN_RANGE(len) \
do { \
if ((len) > Buffer::kMaxLength) { \
*error = SB_BUFFER_SIZE_EXCEEDED_ERROR; \
*error = node::ERR_BUFFER_TOO_LARGE(isolate); \
return MaybeLocal<Value>(); \
} \
} while (0)
Expand All @@ -639,9 +626,13 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
switch (encoding) {
case BUFFER:
{
if (buflen > node::Buffer::kMaxLength) {
*error = node::ERR_BUFFER_TOO_LARGE(isolate);
return MaybeLocal<Value>();
}
auto maybe_buf = Buffer::Copy(isolate, buf, buflen);
if (maybe_buf.IsEmpty()) {
*error = SB_BUFFER_CREATION_ERROR;
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
return MaybeLocal<Value>();
}
return maybe_buf.ToLocalChecked();
Expand All @@ -651,7 +642,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
if (contains_non_ascii(buf, buflen)) {
char* out = node::UncheckedMalloc(buflen);
if (out == nullptr) {
*error = SB_MALLOC_FAILED_ERROR;
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
return MaybeLocal<Value>();
}
force_ascii(buf, out, buflen);
Expand All @@ -666,7 +657,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
v8::NewStringType::kNormal,
buflen);
if (val.IsEmpty()) {
*error = SB_STRING_TOO_LONG_ERROR;
*error = node::ERR_STRING_TOO_LARGE(isolate);
return MaybeLocal<Value>();
}
return val.ToLocalChecked();
Expand All @@ -678,7 +669,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
size_t dlen = base64_encoded_size(buflen);
char* dst = node::UncheckedMalloc(dlen);
if (dst == nullptr) {
*error = SB_MALLOC_FAILED_ERROR;
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
return MaybeLocal<Value>();
}

Expand All @@ -692,7 +683,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
size_t dlen = buflen * 2;
char* dst = node::UncheckedMalloc(dlen);
if (dst == nullptr) {
*error = SB_MALLOC_FAILED_ERROR;
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
return MaybeLocal<Value>();
}
size_t written = hex_encode(buf, buflen, dst, dlen);
Expand Down Expand Up @@ -723,7 +714,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
if (IsBigEndian()) {
uint16_t* dst = node::UncheckedMalloc<uint16_t>(buflen);
if (dst == nullptr) {
*error = SB_MALLOC_FAILED_ERROR;
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
return MaybeLocal<Value>();
}
size_t nbytes = buflen * sizeof(uint16_t);
Expand Down
10 changes: 7 additions & 3 deletions ...addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-ascii.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ if (!common.enoughTestMem)
common.skip(skipMessage);

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

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

assert.throws(function() {
const stringLengthHex = kStringMaxLength.toString(16);
common.expectsError(function() {
buf.toString('ascii');
}, /"toString\(\)" failed/);
}, {
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
code: 'ERR_STRING_TOO_LARGE',
type: Error
});
10 changes: 7 additions & 3 deletions ...ddons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-base64.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ if (!common.enoughTestMem)
common.skip(skipMessage);

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

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

assert.throws(function() {
const stringLengthHex = kStringMaxLength.toString(16);
common.expectsError(function() {
buf.toString('base64');
}, /"toString\(\)" failed/);
}, {
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
code: 'ERR_STRING_TOO_LARGE',
type: Error
});
9 changes: 7 additions & 2 deletions ...ddons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,14 @@ try {
if (!binding.ensureAllocation(2 * kStringMaxLength))
common.skip(skipMessage);

assert.throws(function() {
const stringLengthHex = kStringMaxLength.toString(16);
common.expectsError(function() {
buf.toString('latin1');
}, /"toString\(\)" failed/);
}, {
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
code: 'ERR_STRING_TOO_LARGE',
type: Error
});

let maxString = buf.toString('latin1', 1);
assert.strictEqual(maxString.length, kStringMaxLength);
Expand Down
10 changes: 7 additions & 3 deletions test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-hex.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ if (!common.enoughTestMem)
common.skip(skipMessage);

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

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

assert.throws(function() {
const stringLengthHex = kStringMaxLength.toString(16);
common.expectsError(function() {
buf.toString('hex');
}, /"toString\(\)" failed/);
}, {
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
code: 'ERR_STRING_TOO_LARGE',
type: Error
});
23 changes: 20 additions & 3 deletions .../addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,27 @@ try {
if (!binding.ensureAllocation(2 * kStringMaxLength))
common.skip(skipMessage);

const stringLengthHex = kStringMaxLength.toString(16);

assert.throws(function() {
buf.toString();
}, /"toString\(\)" failed|Array buffer allocation failed/);
}, function(e) {
if (e.message !== 'Array buffer allocation failed') {
common.expectsError({
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
code: 'ERR_STRING_TOO_LARGE',
type: Error
})(e);
return true;
} else {
return true;
}
});

assert.throws(function() {
common.expectsError(function() {
buf.toString('utf8');
}, /"toString\(\)" failed/);
}, {
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
code: 'ERR_STRING_TOO_LARGE',
type: Error
});
11 changes: 8 additions & 3 deletions test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ if (!common.enoughTestMem)
common.skip(skipMessage);

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

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

assert.throws(function() {
const stringLengthHex = kStringMaxLength.toString(16);

common.expectsError(function() {
buf.toString('utf16le');
}, /"toString\(\)" failed/);
}, {
message: `Cannot create a string larger than 0x${stringLengthHex} bytes`,
code: 'ERR_STRING_TOO_LARGE',
type: Error
});
11 changes: 9 additions & 2 deletions test/sequential/test-fs-readfile-tostring-fail.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,15 @@ stream.end();
stream.on('finish', common.mustCall(function() {
fs.readFile(file, 'utf8', common.mustCall(function(err, buf) {
assert.ok(err instanceof Error);
assert.ok(/^(Array buffer allocation failed|"toString\(\)" failed)$/
.test(err.message));
if (err.message !== 'Array buffer allocation failed') {
const stringLengthHex = kStringMaxLength.toString(16);
common.expectsError({
message: 'Cannot create a string larger than ' +
`0x${stringLengthHex} bytes`,
code: 'ERR_STRING_TOO_LARGE',
type: Error
})(err);
}
assert.strictEqual(buf, undefined);
}));
}));
Expand Down

0 comments on commit 63eb267

Please sign in to comment.