From d56a7e640f766f15980b28d15bbf02bf60975ab2 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 30 Apr 2017 18:53:04 +0200 Subject: [PATCH] src: do proper StringBytes error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Return `MaybeLocal`s from `StringBytes::Encode` - Add an `error` out parameter to pass JS exceptions to the callers (instead of directly throwing) - Simplify some of the string generation methods in `string_bytes.cc` by unifying the `EXTERN_APEX` logic - Reduce usage of deprecated V8 APIs. - Remove error handling logic from JS, the `buffer.*Slice()` methods now throw errors themselves. - Left TODO comments for future semver-major error message improvements. This paves the way for better error messages coming out of the StringBytes methods. Ref: https://github.com/nodejs/node/issues/3175 PR-URL: https://github.com/nodejs/node/pull/12765 Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell Reviewed-By: Tobias Nießen --- lib/buffer.js | 42 ++++---- src/fs_event_wrap.cc | 14 ++- src/node.cc | 8 +- src/node_buffer.cc | 32 +++++- src/node_crypto.cc | 37 +++++-- src/node_file.cc | 78 +++++++++----- src/node_os.cc | 29 +++-- src/string_bytes.cc | 244 ++++++++++++++++++++++++++----------------- src/string_bytes.h | 25 +++-- 9 files changed, 323 insertions(+), 186 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index e5d15740349e63..c30cbc2d8509a8 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -567,34 +567,30 @@ Buffer.prototype.copy = function(target, targetStart, sourceStart, sourceEnd) { // This behaves neither like String nor Uint8Array in that we set start/end // to their upper/lower bounds if the value passed is out of range. Buffer.prototype.toString = function(encoding, start, end) { - var result; if (arguments.length === 0) { - result = this.utf8Slice(0, this.length); - } else { - const len = this.length; - if (len === 0) - return ''; + return this.utf8Slice(0, this.length); + } - if (!start || start < 0) - start = 0; - else if (start >= len) - return ''; + const len = this.length; + if (len === 0) + return ''; - if (end === undefined || end > len) - end = len; - else if (end <= 0) - return ''; + if (!start || start < 0) + start = 0; + else if (start >= len) + return ''; - start |= 0; - end |= 0; + if (end === undefined || end > len) + end = len; + else if (end <= 0) + return ''; - if (end <= start) - return ''; - result = stringSlice(this, encoding, start, end); - } - if (result === undefined) - throw new Error('"toString()" failed'); - return result; + start |= 0; + end |= 0; + + if (end <= start) + return ''; + return stringSlice(this, encoding, start, end); }; diff --git a/src/fs_event_wrap.cc b/src/fs_event_wrap.cc index a747f71c40c1ae..bc3b33027ac3a6 100644 --- a/src/fs_event_wrap.cc +++ b/src/fs_event_wrap.cc @@ -39,6 +39,7 @@ using v8::FunctionTemplate; using v8::HandleScope; using v8::Integer; using v8::Local; +using v8::MaybeLocal; using v8::Object; using v8::String; using v8::Value; @@ -187,17 +188,20 @@ void FSEventWrap::OnEvent(uv_fs_event_t* handle, const char* filename, }; if (filename != nullptr) { - Local fn = StringBytes::Encode(env->isolate(), - filename, - wrap->encoding_); + Local error; + MaybeLocal fn = StringBytes::Encode(env->isolate(), + filename, + wrap->encoding_, + &error); if (fn.IsEmpty()) { argv[0] = Integer::New(env->isolate(), UV_EINVAL); argv[2] = StringBytes::Encode(env->isolate(), filename, strlen(filename), - BUFFER); + BUFFER, + &error).ToLocalChecked(); } else { - argv[2] = fn; + argv[2] = fn.ToLocalChecked(); } } diff --git a/src/node.cc b/src/node.cc index 0a09a6111b82b5..97ba724ba8a077 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1570,11 +1570,15 @@ Local Encode(Isolate* isolate, size_t len, enum encoding encoding) { CHECK_NE(encoding, UCS2); - return StringBytes::Encode(isolate, buf, len, encoding); + Local error; + return StringBytes::Encode(isolate, buf, len, encoding, &error) + .ToLocalChecked(); } Local Encode(Isolate* isolate, const uint16_t* buf, size_t len) { - return StringBytes::Encode(isolate, buf, len); + Local error; + return StringBytes::Encode(isolate, buf, len, &error) + .ToLocalChecked(); } // Returns -1 if the handle was not valid for decoding diff --git a/src/node_buffer.cc b/src/node_buffer.cc index e8d494519437bf..367af6592ff39b 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -465,14 +465,26 @@ void StringSlice(const FunctionCallbackInfo& args) { SLICE_START_END(args[0], args[1], ts_obj_length) - args.GetReturnValue().Set( - StringBytes::Encode(isolate, ts_obj_data + start, length, encoding)); + Local error; + MaybeLocal ret = + StringBytes::Encode(isolate, + ts_obj_data + start, + length, + encoding, + &error); + if (ret.IsEmpty()) { + CHECK(!error.IsEmpty()); + isolate->ThrowException(error); + return; + } + args.GetReturnValue().Set(ret.ToLocalChecked()); } template <> void StringSlice(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + Isolate* isolate = args.GetIsolate(); + Environment* env = Environment::GetCurrent(isolate); THROW_AND_RETURN_UNLESS_BUFFER(env, args.This()); SPREAD_BUFFER_ARG(args.This(), ts_obj); @@ -509,10 +521,22 @@ void StringSlice(const FunctionCallbackInfo& args) { buf = reinterpret_cast(data); } - args.GetReturnValue().Set(StringBytes::Encode(env->isolate(), buf, length)); + Local error; + MaybeLocal ret = + StringBytes::Encode(isolate, + buf, + length, + &error); if (release) delete[] buf; + + if (ret.IsEmpty()) { + CHECK(!error.IsEmpty()); + isolate->ThrowException(error); + return; + } + args.GetReturnValue().Set(ret.ToLocalChecked()); } diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 361f1a00052c39..cac76c4331d355 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -104,6 +104,7 @@ using v8::Integer; using v8::Isolate; using v8::Local; using v8::Maybe; +using v8::MaybeLocal; using v8::Null; using v8::Object; using v8::Persistent; @@ -3811,12 +3812,20 @@ void Hmac::HmacDigest(const FunctionCallbackInfo& args) { md_len = 0; } - Local rc = StringBytes::Encode(env->isolate(), - reinterpret_cast(md_value), - md_len, - encoding); + Local error; + MaybeLocal rc = + StringBytes::Encode(env->isolate(), + reinterpret_cast(md_value), + md_len, + encoding, + &error); delete[] md_value; - args.GetReturnValue().Set(rc); + if (rc.IsEmpty()) { + CHECK(!error.IsEmpty()); + env->isolate()->ThrowException(error); + return; + } + args.GetReturnValue().Set(rc.ToLocalChecked()); } @@ -3936,11 +3945,19 @@ void Hash::HashDigest(const FunctionCallbackInfo& args) { EVP_MD_CTX_cleanup(&hash->mdctx_); hash->finalized_ = true; - Local rc = StringBytes::Encode(env->isolate(), - reinterpret_cast(md_value), - md_len, - encoding); - args.GetReturnValue().Set(rc); + Local error; + MaybeLocal rc = + StringBytes::Encode(env->isolate(), + reinterpret_cast(md_value), + md_len, + encoding, + &error); + if (rc.IsEmpty()) { + CHECK(!error.IsEmpty()); + env->isolate()->ThrowException(error); + return; + } + args.GetReturnValue().Set(rc.ToLocalChecked()); } diff --git a/src/node_file.cc b/src/node_file.cc index 9d00bb475d21da..4a0b1527d6aca4 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -57,6 +57,7 @@ using v8::FunctionTemplate; using v8::HandleScope; using v8::Integer; using v8::Local; +using v8::MaybeLocal; using v8::Number; using v8::Object; using v8::String; @@ -172,7 +173,8 @@ void After(uv_fs_t *req) { // Allocate space for two args. We may only use one depending on the case. // (Feel free to increase this if you need more) Local argv[2]; - Local link; + MaybeLocal link; + Local error; if (req->result < 0) { // An error happened. @@ -232,10 +234,13 @@ void After(uv_fs_t *req) { break; case UV_FS_MKDTEMP: + { link = StringBytes::Encode(env->isolate(), static_cast(req->path), - req_wrap->encoding_); + req_wrap->encoding_, + &error); if (link.IsEmpty()) { + // TODO(addaleax): Use `error` itself here. argv[0] = UVException(env->isolate(), UV_EINVAL, req_wrap->syscall(), @@ -243,15 +248,18 @@ void After(uv_fs_t *req) { req->path, req_wrap->data()); } else { - argv[1] = link; + argv[1] = link.ToLocalChecked(); } break; + } case UV_FS_READLINK: link = StringBytes::Encode(env->isolate(), static_cast(req->ptr), - req_wrap->encoding_); + req_wrap->encoding_, + &error); if (link.IsEmpty()) { + // TODO(addaleax): Use `error` itself here. argv[0] = UVException(env->isolate(), UV_EINVAL, req_wrap->syscall(), @@ -259,15 +267,17 @@ void After(uv_fs_t *req) { req->path, req_wrap->data()); } else { - argv[1] = link; + argv[1] = link.ToLocalChecked(); } break; case UV_FS_REALPATH: link = StringBytes::Encode(env->isolate(), static_cast(req->ptr), - req_wrap->encoding_); + req_wrap->encoding_, + &error); if (link.IsEmpty()) { + // TODO(addaleax): Use `error` itself here. argv[0] = UVException(env->isolate(), UV_EINVAL, req_wrap->syscall(), @@ -275,7 +285,7 @@ void After(uv_fs_t *req) { req->path, req_wrap->data()); } else { - argv[1] = link; + argv[1] = link.ToLocalChecked(); } break; @@ -306,10 +316,13 @@ void After(uv_fs_t *req) { break; } - Local filename = StringBytes::Encode(env->isolate(), - ent.name, - req_wrap->encoding_); + MaybeLocal filename = + StringBytes::Encode(env->isolate(), + ent.name, + req_wrap->encoding_, + &error); if (filename.IsEmpty()) { + // TODO(addaleax): Use `error` itself here. argv[0] = UVException(env->isolate(), UV_EINVAL, req_wrap->syscall(), @@ -318,7 +331,7 @@ void After(uv_fs_t *req) { req_wrap->data()); break; } - name_argv[name_idx++] = filename; + name_argv[name_idx++] = filename.ToLocalChecked(); if (name_idx >= arraysize(name_argv)) { fn->Call(env->context(), names, name_idx, name_argv) @@ -681,16 +694,20 @@ static void ReadLink(const FunctionCallbackInfo& args) { } else { SYNC_CALL(readlink, *path, *path) const char* link_path = static_cast(SYNC_REQ.ptr); - Local rc = StringBytes::Encode(env->isolate(), - link_path, - encoding); + + Local error; + MaybeLocal rc = StringBytes::Encode(env->isolate(), + link_path, + encoding, + &error); if (rc.IsEmpty()) { + // TODO(addaleax): Use `error` itself here. return env->ThrowUVException(UV_EINVAL, "readlink", "Invalid character encoding for link", *path); } - args.GetReturnValue().Set(rc); + args.GetReturnValue().Set(rc.ToLocalChecked()); } } @@ -852,16 +869,20 @@ static void RealPath(const FunctionCallbackInfo& args) { } else { SYNC_CALL(realpath, *path, *path); const char* link_path = static_cast(SYNC_REQ.ptr); - Local rc = StringBytes::Encode(env->isolate(), - link_path, - encoding); + + Local error; + MaybeLocal rc = StringBytes::Encode(env->isolate(), + link_path, + encoding, + &error); if (rc.IsEmpty()) { + // TODO(addaleax): Use `error` itself here. return env->ThrowUVException(UV_EINVAL, "realpath", "Invalid character encoding for path", *path); } - args.GetReturnValue().Set(rc); + args.GetReturnValue().Set(rc.ToLocalChecked()); } } @@ -903,17 +924,20 @@ static void ReadDir(const FunctionCallbackInfo& args) { if (r != 0) return env->ThrowUVException(r, "readdir", "", *path); - Local filename = StringBytes::Encode(env->isolate(), - ent.name, - encoding); + Local error; + MaybeLocal filename = StringBytes::Encode(env->isolate(), + ent.name, + encoding, + &error); if (filename.IsEmpty()) { + // TODO(addaleax): Use `error` itself here. return env->ThrowUVException(UV_EINVAL, "readdir", "Invalid character encoding for filename", *path); } - name_v[name_idx++] = filename; + name_v[name_idx++] = filename.ToLocalChecked(); if (name_idx >= arraysize(name_v)) { fn->Call(env->context(), names, name_idx, name_v) @@ -1367,14 +1391,18 @@ static void Mkdtemp(const FunctionCallbackInfo& args) { } else { SYNC_CALL(mkdtemp, *tmpl, *tmpl); const char* path = static_cast(SYNC_REQ.path); - Local rc = StringBytes::Encode(env->isolate(), path, encoding); + + Local error; + MaybeLocal rc = + StringBytes::Encode(env->isolate(), path, encoding, &error); if (rc.IsEmpty()) { + // TODO(addaleax): Use `error` itself here. return env->ThrowUVException(UV_EINVAL, "mkdtemp", "Invalid character encoding for filename", *tmpl); } - args.GetReturnValue().Set(rc); + args.GetReturnValue().Set(rc.ToLocalChecked()); } } diff --git a/src/node_os.cc b/src/node_os.cc index fad19469532ee0..c1d1de971f4a09 100644 --- a/src/node_os.cc +++ b/src/node_os.cc @@ -357,36 +357,43 @@ static void GetUserInfo(const FunctionCallbackInfo& args) { return env->ThrowUVException(err, "uv_os_get_passwd"); } + Local error; + Local uid = Number::New(env->isolate(), pwd.uid); Local gid = Number::New(env->isolate(), pwd.gid); - Local username = StringBytes::Encode(env->isolate(), - pwd.username, - encoding); - Local homedir = StringBytes::Encode(env->isolate(), - pwd.homedir, - encoding); - Local shell; + MaybeLocal username = StringBytes::Encode(env->isolate(), + pwd.username, + encoding, + &error); + MaybeLocal homedir = StringBytes::Encode(env->isolate(), + pwd.homedir, + encoding, + &error); + MaybeLocal shell; if (pwd.shell == NULL) shell = Null(env->isolate()); else - shell = StringBytes::Encode(env->isolate(), pwd.shell, encoding); + shell = StringBytes::Encode(env->isolate(), pwd.shell, encoding, &error); uv_os_free_passwd(&pwd); if (username.IsEmpty()) { + // TODO(addaleax): Use `error` itself here. return env->ThrowUVException(UV_EINVAL, "uv_os_get_passwd", "Invalid character encoding for username"); } if (homedir.IsEmpty()) { + // TODO(addaleax): Use `error` itself here. return env->ThrowUVException(UV_EINVAL, "uv_os_get_passwd", "Invalid character encoding for homedir"); } if (shell.IsEmpty()) { + // TODO(addaleax): Use `error` itself here. return env->ThrowUVException(UV_EINVAL, "uv_os_get_passwd", "Invalid character encoding for shell"); @@ -396,9 +403,9 @@ static void GetUserInfo(const FunctionCallbackInfo& args) { entry->Set(env->uid_string(), uid); entry->Set(env->gid_string(), gid); - entry->Set(env->username_string(), username); - entry->Set(env->homedir_string(), homedir); - entry->Set(env->shell_string(), shell); + entry->Set(env->username_string(), username.ToLocalChecked()); + entry->Set(env->homedir_string(), homedir.ToLocalChecked()); + entry->Set(env->shell_string(), shell.ToLocalChecked()); args.GetReturnValue().Set(entry); } diff --git a/src/string_bytes.cc b/src/string_bytes.cc index 0954b0f244ec10..636d7da25d3291 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -35,14 +35,26 @@ // 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::EscapableHandleScope; using v8::HandleScope; using v8::Isolate; using v8::Local; using v8::MaybeLocal; -using v8::Object; using v8::String; using v8::Value; @@ -68,46 +80,56 @@ class ExternString: public ResourceType { return length() * sizeof(*data()); } - static Local NewFromCopy(Isolate* isolate, - const TypeName* data, - size_t length) { - EscapableHandleScope scope(isolate); - + static MaybeLocal NewFromCopy(Isolate* isolate, + const TypeName* data, + size_t length, + Local* error) { if (length == 0) - return scope.Escape(String::Empty(isolate)); + return String::Empty(isolate); + + if (length < EXTERN_APEX) + return NewSimpleFromCopy(isolate, data, length, error); TypeName* new_data = node::UncheckedMalloc(length); if (new_data == nullptr) { - return Local(); + *error = SB_MALLOC_FAILED_ERROR; + return MaybeLocal(); } memcpy(new_data, data, length * sizeof(*new_data)); - return scope.Escape(ExternString::New(isolate, - new_data, - length)); + return ExternString::New(isolate, + new_data, + length, + error); } // uses "data" for external resource, and will be free'd on gc - static Local New(Isolate* isolate, - const TypeName* data, - size_t length) { - EscapableHandleScope scope(isolate); - + static MaybeLocal New(Isolate* isolate, + TypeName* data, + size_t length, + Local* error) { if (length == 0) - return scope.Escape(String::Empty(isolate)); + return String::Empty(isolate); + + if (length < EXTERN_APEX) { + MaybeLocal str = NewSimpleFromCopy(isolate, data, length, error); + free(data); + return str; + } ExternString* h_str = new ExternString(isolate, data, length); - MaybeLocal str = NewExternal(isolate, h_str); + MaybeLocal str = NewExternal(isolate, h_str); isolate->AdjustAmountOfExternalAllocatedMemory(h_str->byte_length()); if (str.IsEmpty()) { delete h_str; - return Local(); + *error = SB_STRING_TOO_LONG_ERROR; + return MaybeLocal(); } - return scope.Escape(str.ToLocalChecked()); + return str.ToLocalChecked(); } inline Isolate* isolate() const { return isolate_; } @@ -115,8 +137,14 @@ class ExternString: public ResourceType { private: ExternString(Isolate* isolate, const TypeName* data, size_t length) : isolate_(isolate), data_(data), length_(length) { } - static MaybeLocal NewExternal(Isolate* isolate, - ExternString* h_str); + static MaybeLocal NewExternal(Isolate* isolate, + ExternString* h_str); + + // This method does not actually create ExternString instances. + static MaybeLocal NewSimpleFromCopy(Isolate* isolate, + const TypeName* data, + size_t length, + Local* error); Isolate* isolate_; const TypeName* data_; @@ -131,16 +159,51 @@ typedef ExternString -MaybeLocal ExternOneByteString::NewExternal( +MaybeLocal ExternOneByteString::NewExternal( Isolate* isolate, ExternOneByteString* h_str) { - return String::NewExternalOneByte(isolate, h_str); + return String::NewExternalOneByte(isolate, h_str).FromMaybe(Local()); } template <> -MaybeLocal ExternTwoByteString::NewExternal( +MaybeLocal ExternTwoByteString::NewExternal( Isolate* isolate, ExternTwoByteString* h_str) { - return String::NewExternalTwoByte(isolate, h_str); + return String::NewExternalTwoByte(isolate, h_str).FromMaybe(Local()); +} + +template <> +MaybeLocal ExternOneByteString::NewSimpleFromCopy(Isolate* isolate, + const char* data, + size_t length, + Local* error) { + MaybeLocal str = + String::NewFromOneByte(isolate, + reinterpret_cast(data), + v8::NewStringType::kNormal, + length); + if (str.IsEmpty()) { + *error = SB_STRING_TOO_LONG_ERROR; + return MaybeLocal(); + } + return str.ToLocalChecked(); +} + + +template <> +MaybeLocal ExternTwoByteString::NewSimpleFromCopy(Isolate* isolate, + const uint16_t* data, + size_t length, + Local* error) { + MaybeLocal str = + String::NewFromTwoByte(isolate, + data, + v8::NewStringType::kNormal, + length); + if (str.IsEmpty()) { + *error = SB_STRING_TOO_LONG_ERROR; + return MaybeLocal(); + } + return str.ToLocalChecked(); } } // anonymous namespace @@ -610,97 +673,93 @@ 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; \ + return MaybeLocal(); \ + } \ + } while (0) -Local StringBytes::Encode(Isolate* isolate, - const char* buf, - size_t buflen, - enum encoding encoding) { - EscapableHandleScope scope(isolate); +MaybeLocal StringBytes::Encode(Isolate* isolate, + const char* buf, + size_t buflen, + enum encoding encoding, + Local* error) { CHECK_NE(encoding, UCS2); - CHECK_LE(buflen, Buffer::kMaxLength); - if (!buflen && encoding != BUFFER) - return scope.Escape(String::Empty(isolate)); + CHECK_BUFLEN_IN_RANGE(buflen); + + *error = Local(); + if (!buflen && encoding != BUFFER) { + return String::Empty(isolate); + } + + MaybeLocal val; - Local val; switch (encoding) { case BUFFER: { - Local vbuf = - Buffer::Copy(isolate, buf, buflen).ToLocalChecked(); - return scope.Escape(vbuf); + auto maybe_buf = Buffer::Copy(isolate, buf, buflen); + if (maybe_buf.IsEmpty()) { + *error = SB_BUFFER_CREATION_ERROR; + return MaybeLocal(); + } + return maybe_buf.ToLocalChecked(); } case ASCII: if (contains_non_ascii(buf, buflen)) { char* out = node::UncheckedMalloc(buflen); if (out == nullptr) { - return Local(); + *error = SB_MALLOC_FAILED_ERROR; + return MaybeLocal(); } force_ascii(buf, out, buflen); - if (buflen < EXTERN_APEX) { - val = OneByteString(isolate, out, buflen); - free(out); - } else { - val = ExternOneByteString::New(isolate, out, buflen); - } + return ExternOneByteString::New(isolate, out, buflen, error); } else { - if (buflen < EXTERN_APEX) - val = OneByteString(isolate, buf, buflen); - else - val = ExternOneByteString::NewFromCopy(isolate, buf, buflen); + return ExternOneByteString::NewFromCopy(isolate, buf, buflen, error); } - break; case UTF8: val = String::NewFromUtf8(isolate, buf, - String::kNormalString, + v8::NewStringType::kNormal, buflen); - break; + if (val.IsEmpty()) { + *error = SB_STRING_TOO_LONG_ERROR; + return MaybeLocal(); + } + return val.ToLocalChecked(); case LATIN1: - if (buflen < EXTERN_APEX) - val = OneByteString(isolate, buf, buflen); - else - val = ExternOneByteString::NewFromCopy(isolate, buf, buflen); - break; + return ExternOneByteString::NewFromCopy(isolate, buf, buflen, error); case BASE64: { size_t dlen = base64_encoded_size(buflen); char* dst = node::UncheckedMalloc(dlen); if (dst == nullptr) { - return Local(); + *error = SB_MALLOC_FAILED_ERROR; + return MaybeLocal(); } size_t written = base64_encode(buf, buflen, dst, dlen); CHECK_EQ(written, dlen); - if (dlen < EXTERN_APEX) { - val = OneByteString(isolate, dst, dlen); - free(dst); - } else { - val = ExternOneByteString::New(isolate, dst, dlen); - } - break; + return ExternOneByteString::New(isolate, dst, dlen, error); } case HEX: { size_t dlen = buflen * 2; char* dst = node::UncheckedMalloc(dlen); if (dst == nullptr) { - return Local(); + *error = SB_MALLOC_FAILED_ERROR; + return MaybeLocal(); } size_t written = hex_encode(buf, buflen, dst, dlen); CHECK_EQ(written, dlen); - if (dlen < EXTERN_APEX) { - val = OneByteString(isolate, dst, dlen); - free(dst); - } else { - val = ExternOneByteString::New(isolate, dst, dlen); - } - break; + return ExternOneByteString::New(isolate, dst, dlen, error); } default: @@ -708,13 +767,17 @@ Local StringBytes::Encode(Isolate* isolate, break; } - return scope.Escape(val); + UNREACHABLE(); } -Local StringBytes::Encode(Isolate* isolate, - const uint16_t* buf, - size_t buflen) { +MaybeLocal StringBytes::Encode(Isolate* isolate, + const uint16_t* buf, + size_t buflen, + Local* error) { + CHECK_BUFLEN_IN_RANGE(buflen); + *error = Local(); + // Node's "ucs2" encoding expects LE character data inside a // Buffer, so we need to reorder on BE platforms. See // http://nodejs.org/api/buffer.html regarding Node's "ucs2" @@ -727,24 +790,15 @@ Local StringBytes::Encode(Isolate* isolate, buf = &dst[0]; } - Local val; - if (buflen < EXTERN_APEX) { - val = String::NewFromTwoByte(isolate, - buf, - String::kNormalString, - buflen); - } else { - val = ExternTwoByteString::NewFromCopy(isolate, buf, buflen); - } - - return val; + return ExternTwoByteString::NewFromCopy(isolate, buf, buflen, error); } -Local StringBytes::Encode(Isolate* isolate, - const char* buf, - enum encoding encoding) { +MaybeLocal StringBytes::Encode(Isolate* isolate, + const char* buf, + enum encoding encoding, + Local* error) { const size_t len = strlen(buf); - Local ret; + MaybeLocal ret; if (encoding == UCS2) { // In Node, UCS2 means utf16le. The data must be in little-endian // order and must be aligned on 2-bytes. This returns an empty @@ -763,9 +817,9 @@ Local StringBytes::Encode(Isolate* isolate, } ret = vec.empty() ? static_cast< Local >(String::Empty(isolate)) - : StringBytes::Encode(isolate, &vec[0], vec.size()); + : StringBytes::Encode(isolate, &vec[0], vec.size(), error); } else { - ret = StringBytes::Encode(isolate, buf, len, encoding); + ret = StringBytes::Encode(isolate, buf, len, encoding, error); } return ret; } diff --git a/src/string_bytes.h b/src/string_bytes.h index 81561e6e7067cc..e11c16d65f2afb 100644 --- a/src/string_bytes.h +++ b/src/string_bytes.h @@ -105,19 +105,22 @@ class StringBytes { // Take the bytes in the src, and turn it into a Buffer or String. // Don't call with encoding=UCS2. - static v8::Local Encode(v8::Isolate* isolate, - const char* buf, - size_t buflen, - enum encoding encoding); + static v8::MaybeLocal Encode(v8::Isolate* isolate, + const char* buf, + size_t buflen, + enum encoding encoding, + v8::Local* error); // The input buffer should be in host endianness. - static v8::Local Encode(v8::Isolate* isolate, - const uint16_t* buf, - size_t buflen); - - static v8::Local Encode(v8::Isolate* isolate, - const char* buf, - enum encoding encoding); + static v8::MaybeLocal Encode(v8::Isolate* isolate, + const uint16_t* buf, + size_t buflen, + v8::Local* error); + + static v8::MaybeLocal Encode(v8::Isolate* isolate, + const char* buf, + enum encoding encoding, + v8::Local* error); private: static size_t WriteUCS2(char* buf,