From beb24285ad3c8beea1595fb846e5477f8c9d3157 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 2 Apr 2018 10:36:17 +0800 Subject: [PATCH 1/2] src: add error code helpers to src/node_errors.h This commit adds node::ERR_*(isolate, message) helpers in the C++ land to assign error codes to existing C++ errors. The following errors are added: - ERR_MEMORY_ALLOCATION_FAILED - ERR_STRING_TOO_LARGE --- doc/api/errors.md | 12 ++++++++ node.gyp | 1 + src/node_errors.h | 73 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+) create mode 100644 src/node_errors.h diff --git a/doc/api/errors.md b/doc/api/errors.md index 936b1b0cd5d53c..e96848b2960d40 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1242,6 +1242,12 @@ An attempt was made to open an IPC communication channel with a synchronously forked Node.js process. See the documentation for the [`child_process`][] module for more information. + +### ERR_MEMORY_ALLOCATION_FAILED + +An attempt was made to allocate memory (usually in the C++ layer) but it +failed. + ### ERR_METHOD_NOT_IMPLEMENTED @@ -1468,6 +1474,12 @@ additional details. A stream method was called that cannot complete because the stream was destroyed using `stream.destroy()`. + +### ERR_STRING_TOO_LARGE + +An attempt has been made to create a string larger than the maximum allowed +size. + ### ERR_TLS_CERT_ALTNAME_INVALID diff --git a/node.gyp b/node.gyp index 4bc9a3650f7781..4709e5ec1e2cb6 100644 --- a/node.gyp +++ b/node.gyp @@ -316,6 +316,7 @@ 'src/node_contextify.cc', 'src/node_debug_options.cc', 'src/node_domain.cc', + 'src/node_errors.h', 'src/node_file.cc', 'src/node_http2.cc', 'src/node_http_parser.cc', diff --git a/src/node_errors.h b/src/node_errors.h new file mode 100644 index 00000000000000..8e328ac2f30644 --- /dev/null +++ b/src/node_errors.h @@ -0,0 +1,73 @@ +#ifndef SRC_NODE_ERRORS_H_ +#define SRC_NODE_ERRORS_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "node.h" +#include "util-inl.h" +#include "env-inl.h" +#include "v8.h" + +namespace node { + +// Helpers to construct errors similar to the ones provided by +// lib/internal/errors.js. +// Example: with `V(ERR_INVALID_ARG_TYPE, TypeError)`, there will be +// `node::ERR_INVALID_ARG_TYPE(isolate, "message")` returning +// a `Local` containing the TypeError with proper code and message + +#define ERRORS_WITH_CODE(V) \ + V(ERR_MEMORY_ALLOCATION_FAILED, Error) \ + V(ERR_STRING_TOO_LARGE, Error) \ + V(ERR_BUFFER_TOO_LARGE, Error) + +#define V(code, type) \ + inline v8::Local code(v8::Isolate* isolate, \ + const char* message) { \ + v8::Local js_code = OneByteString(isolate, #code); \ + v8::Local js_msg = OneByteString(isolate, message); \ + v8::Local e = \ + v8::Exception::type(js_msg)->ToObject( \ + isolate->GetCurrentContext()).ToLocalChecked(); \ + e->Set(isolate->GetCurrentContext(), OneByteString(isolate, "code"), \ + js_code).FromJust(); \ + return e; \ + } + ERRORS_WITH_CODE(V) +#undef V + +// Errors with predefined static messages + +#define PREDEFINED_ERROR_MESSAGES(V) \ + V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory") + +#define V(code, message) \ + inline v8::Local code(v8::Isolate* isolate) { \ + return code(isolate, message); \ + } + PREDEFINED_ERROR_MESSAGES(V) +#undef V + +// Errors with predefined non-static messages + +inline v8::Local ERR_BUFFER_TOO_LARGE(v8::Isolate *isolate) { + char message[128]; + snprintf(message, sizeof(message), + "Cannot create a Buffer larger than 0x%lx bytes", + v8::TypedArray::kMaxLength); + return ERR_BUFFER_TOO_LARGE(isolate, message); +} + +inline v8::Local ERR_STRING_TOO_LARGE(v8::Isolate *isolate) { + char message[128]; + snprintf(message, sizeof(message), + "Cannot create a string larger than 0x%x bytes", + v8::String::kMaxLength); + return ERR_STRING_TOO_LARGE(isolate, message); +} + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#endif // SRC_NODE_ERRORS_H_ From 6294175a6f1b69a8d72a20106ea798525bc98386 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 2 Apr 2018 10:39:26 +0800 Subject: [PATCH 2/2] src: migrate string_bytes.cc to throw errors with code --- src/string_bytes.cc | 41 ++++++++----------- ...ingbytes-external-exceed-max-by-1-ascii.js | 10 +++-- ...ngbytes-external-exceed-max-by-1-base64.js | 10 +++-- ...ngbytes-external-exceed-max-by-1-binary.js | 9 +++- ...tringbytes-external-exceed-max-by-1-hex.js | 10 +++-- ...ringbytes-external-exceed-max-by-1-utf8.js | 23 +++++++++-- .../test-stringbytes-external-exceed-max.js | 11 +++-- .../test-fs-readfile-tostring-fail.js | 11 ++++- 8 files changed, 81 insertions(+), 44 deletions(-) diff --git a/src/string_bytes.cc b/src/string_bytes.cc index 624eb92b930c12..bbd381d33a5cf7 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -23,6 +23,7 @@ #include "base64.h" #include "node_internals.h" +#include "node_errors.h" #include "node_buffer.h" #include @@ -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; @@ -93,7 +80,7 @@ class ExternString: public ResourceType { TypeName* new_data = node::UncheckedMalloc(length); if (new_data == nullptr) { - *error = SB_MALLOC_FAILED_ERROR; + *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate); return MaybeLocal(); } memcpy(new_data, data, length * sizeof(*new_data)); @@ -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(); } @@ -183,7 +170,7 @@ MaybeLocal 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(); } return str.ToLocalChecked(); @@ -201,7 +188,7 @@ MaybeLocal 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(); } return str.ToLocalChecked(); @@ -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(); \ } \ } while (0) @@ -639,9 +626,13 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, switch (encoding) { case BUFFER: { + if (buflen > node::Buffer::kMaxLength) { + *error = node::ERR_BUFFER_TOO_LARGE(isolate); + return MaybeLocal(); + } 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(); } return maybe_buf.ToLocalChecked(); @@ -651,7 +642,7 @@ MaybeLocal 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(); } force_ascii(buf, out, buflen); @@ -666,7 +657,7 @@ MaybeLocal 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(); } return val.ToLocalChecked(); @@ -678,7 +669,7 @@ MaybeLocal 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(); } @@ -692,7 +683,7 @@ MaybeLocal 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(); } size_t written = hex_encode(buf, buflen, dst, dlen); @@ -723,7 +714,7 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, if (IsBigEndian()) { uint16_t* dst = node::UncheckedMalloc(buflen); if (dst == nullptr) { - *error = SB_MALLOC_FAILED_ERROR; + *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate); return MaybeLocal(); } size_t nbytes = buflen * sizeof(uint16_t); diff --git a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-ascii.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-ascii.js index 43b8c63f1e466e..96a3273254d8d7 100644 --- a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-ascii.js +++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-ascii.js @@ -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 @@ -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 +}); diff --git a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-base64.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-base64.js index a94a57e18037fd..90e13ce788efcc 100644 --- a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-base64.js +++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-base64.js @@ -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 @@ -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 +}); diff --git a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js index 996c01752da7c6..0ffd1eb4166989 100644 --- a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js +++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js @@ -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); diff --git a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-hex.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-hex.js index 17d9ae5d68f033..bc64ef396dc2d3 100644 --- a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-hex.js +++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-hex.js @@ -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 @@ -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 +}); diff --git a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js index d3368ca7b2ea6b..f6c9f21e4b06e2 100644 --- a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js +++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js @@ -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 +}); diff --git a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max.js index 319a8a93558c45..e4b66d8f30bae3 100644 --- a/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max.js +++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max.js @@ -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 @@ -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 +}); diff --git a/test/sequential/test-fs-readfile-tostring-fail.js b/test/sequential/test-fs-readfile-tostring-fail.js index 81c7a941eb9476..f8b0c666a01b23 100644 --- a/test/sequential/test-fs-readfile-tostring-fail.js +++ b/test/sequential/test-fs-readfile-tostring-fail.js @@ -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); })); }));