src: do proper StringBytes error handling

- 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 <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

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);
+    return this.utf8Slice(0, this.length);
-  } else {
-    const len = this.length;
-    if (len === 0)
-      return '';
-
-    if (!start || start < 0)
-      start = 0;
-    else if (start >= len)
-      return '';
-
-    if (end === undefined || end > len)
-      end = len;
-    else if (end <= 0)
-      return '';
-
-    start |= 0;
-    end |= 0;
-
-    if (end <= start)
-      return '';
-    result = stringSlice(this, encoding, start, end);
   }
-  if (result === undefined)
+
-    throw new Error('"toString()" failed');
+  const len = this.length;
-  return result;
+  if (len === 0)
+    return '';
+
+  if (!start || start < 0)
+    start = 0;
+  else if (start >= len)
+    return '';
+
+  if (end === undefined || end > len)
+    end = len;
+  else if (end <= 0)
+    return '';
+
+  start |= 0;
+  end |= 0;
+
+  if (end <= start)
+    return '';
+  return stringSlice(this, encoding, start, end);
 };
 
 

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<Value> fn = StringBytes::Encode(env->isolate(),
+    Local<Value> error;
-                                          filename,
+    MaybeLocal<Value> fn = StringBytes::Encode(env->isolate(),
-                                          wrap->encoding_);
+                                               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();
     }
   }
 

src/node.cc

@@ -1570,11 +1570,15 @@ Local<Value> Encode(Isolate* isolate,
                     size_t len,
                     enum encoding encoding) {
   CHECK_NE(encoding, UCS2);
-  return StringBytes::Encode(isolate, buf, len, encoding);
+  Local<Value> error;
+  return StringBytes::Encode(isolate, buf, len, encoding, &error)
+      .ToLocalChecked();
 }
 
 Local<Value> Encode(Isolate* isolate, const uint16_t* buf, size_t len) {
-  return StringBytes::Encode(isolate, buf, len);
+  Local<Value> error;
+  return StringBytes::Encode(isolate, buf, len, &error)
+      .ToLocalChecked();
 }
 
 // Returns -1 if the handle was not valid for decoding

src/node_buffer.cc

@@ -465,14 +465,26 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
 
   SLICE_START_END(args[0], args[1], ts_obj_length)
 
-  args.GetReturnValue().Set(
+  Local<Value> error;
-      StringBytes::Encode(isolate, ts_obj_data + start, length, encoding));
+  MaybeLocal<Value> 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<UCS2>(const FunctionCallbackInfo<Value>& 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<UCS2>(const FunctionCallbackInfo<Value>& args) {
     buf = reinterpret_cast<const uint16_t*>(data);
   }
 
-  args.GetReturnValue().Set(StringBytes::Encode(env->isolate(), buf, length));
+  Local<Value> error;
+  MaybeLocal<Value> 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());
 }
 
 

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<Value>& args) {
     md_len = 0;
   }
 
-  Local<Value> rc = StringBytes::Encode(env->isolate(),
+  Local<Value> error;
-                                        reinterpret_cast<const char*>(md_value),
+  MaybeLocal<Value> rc =
-                                        md_len,
+      StringBytes::Encode(env->isolate(),
-                                        encoding);
+                          reinterpret_cast<const char*>(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<Value>& args) {
   EVP_MD_CTX_cleanup(&hash->mdctx_);
   hash->finalized_ = true;
 
-  Local<Value> rc = StringBytes::Encode(env->isolate(),
+  Local<Value> error;
-                                        reinterpret_cast<const char*>(md_value),
+  MaybeLocal<Value> rc =
-                                        md_len,
+      StringBytes::Encode(env->isolate(),
-                                        encoding);
+                          reinterpret_cast<const char*>(md_value),
-  args.GetReturnValue().Set(rc);
+                          md_len,
+                          encoding,
+                          &error);
+  if (rc.IsEmpty()) {
+    CHECK(!error.IsEmpty());
+    env->isolate()->ThrowException(error);
+    return;
+  }
+  args.GetReturnValue().Set(rc.ToLocalChecked());
 }
 
 

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<Value> argv[2];
-  Local<Value> link;
+  MaybeLocal<Value> link;
+  Local<Value> 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<const char*>(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<const char*>(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<const char*>(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<Value> filename = StringBytes::Encode(env->isolate(),
+            MaybeLocal<Value> filename =
-                                                        ent.name,
+                StringBytes::Encode(env->isolate(),
-                                                        req_wrap->encoding_);
+                                    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<Value>& args) {
   } else {
     SYNC_CALL(readlink, *path, *path)
     const char* link_path = static_cast<const char*>(SYNC_REQ.ptr);
-    Local<Value> rc = StringBytes::Encode(env->isolate(),
+
-                                          link_path,
+    Local<Value> error;
-                                          encoding);
+    MaybeLocal<Value> 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<Value>& args) {
   } else {
     SYNC_CALL(realpath, *path, *path);
     const char* link_path = static_cast<const char*>(SYNC_REQ.ptr);
-    Local<Value> rc = StringBytes::Encode(env->isolate(),
+
-                                          link_path,
+    Local<Value> error;
-                                          encoding);
+    MaybeLocal<Value> 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<Value>& args) {
       if (r != 0)
         return env->ThrowUVException(r, "readdir", "", *path);
 
-      Local<Value> filename = StringBytes::Encode(env->isolate(),
+      Local<Value> error;
-                                                  ent.name,
+      MaybeLocal<Value> filename = StringBytes::Encode(env->isolate(),
-                                                  encoding);
+                                                       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<Value>& args) {
   } else {
     SYNC_CALL(mkdtemp, *tmpl, *tmpl);
     const char* path = static_cast<const char*>(SYNC_REQ.path);
-    Local<Value> rc = StringBytes::Encode(env->isolate(), path, encoding);
+
+    Local<Value> error;
+    MaybeLocal<Value> 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());
   }
 }
 

src/node_os.cc

@@ -357,36 +357,43 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
     return env->ThrowUVException(err, "uv_os_get_passwd");
   }
 
+  Local<Value> error;
+
   Local<Value> uid = Number::New(env->isolate(), pwd.uid);
   Local<Value> gid = Number::New(env->isolate(), pwd.gid);
-  Local<Value> username = StringBytes::Encode(env->isolate(),
+  MaybeLocal<Value> username = StringBytes::Encode(env->isolate(),
-                                              pwd.username,
+                                                   pwd.username,
-                                              encoding);
+                                                   encoding,
-  Local<Value> homedir = StringBytes::Encode(env->isolate(),
+                                                   &error);
-                                             pwd.homedir,
+  MaybeLocal<Value> homedir = StringBytes::Encode(env->isolate(),
-                                             encoding);
+                                                  pwd.homedir,
-  Local<Value> shell;
+                                                  encoding,
+                                                  &error);
+  MaybeLocal<Value> 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<Value>& args) {
 
   entry->Set(env->uid_string(), uid);
   entry->Set(env->gid_string(), gid);
-  entry->Set(env->username_string(), username);
+  entry->Set(env->username_string(), username.ToLocalChecked());
-  entry->Set(env->homedir_string(), homedir);
+  entry->Set(env->homedir_string(), homedir.ToLocalChecked());
-  entry->Set(env->shell_string(), shell);
+  entry->Set(env->shell_string(), shell.ToLocalChecked());
 
   args.GetReturnValue().Set(entry);
 }

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<String> NewFromCopy(Isolate* isolate,
+  static MaybeLocal<Value> NewFromCopy(Isolate* isolate,
-                                   const TypeName* data,
+                                       const TypeName* data,
-                                   size_t length) {
+                                       size_t length,
-    EscapableHandleScope scope(isolate);
+                                       Local<Value>* 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<TypeName>(length);
     if (new_data == nullptr) {
-      return Local<String>();
+      *error = SB_MALLOC_FAILED_ERROR;
+      return MaybeLocal<Value>();
     }
     memcpy(new_data, data, length * sizeof(*new_data));
 
-    return scope.Escape(ExternString<ResourceType, TypeName>::New(isolate,
+    return ExternString<ResourceType, TypeName>::New(isolate,
-                                                                  new_data,
+                                                     new_data,
-                                                                  length));
+                                                     length,
+                                                     error);
   }
 
   // uses "data" for external resource, and will be free'd on gc
-  static Local<String> New(Isolate* isolate,
+  static MaybeLocal<Value> New(Isolate* isolate,
-                           const TypeName* data,
+                               TypeName* data,
-                           size_t length) {
+                               size_t length,
-    EscapableHandleScope scope(isolate);
+                               Local<Value>* error) {
-
     if (length == 0)
-      return scope.Escape(String::Empty(isolate));
+      return String::Empty(isolate);
+
+    if (length < EXTERN_APEX) {
+      MaybeLocal<Value> str = NewSimpleFromCopy(isolate, data, length, error);
+      free(data);
+      return str;
+    }
 
     ExternString* h_str = new ExternString<ResourceType, TypeName>(isolate,
                                                                    data,
                                                                    length);
-    MaybeLocal<String> str = NewExternal(isolate, h_str);
+    MaybeLocal<Value> str = NewExternal(isolate, h_str);
     isolate->AdjustAmountOfExternalAllocatedMemory(h_str->byte_length());
 
     if (str.IsEmpty()) {
       delete h_str;
-      return Local<String>();
+      *error = SB_STRING_TOO_LONG_ERROR;
+      return MaybeLocal<Value>();
     }
 
-    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<String> NewExternal(Isolate* isolate,
+  static MaybeLocal<Value> NewExternal(Isolate* isolate,
-                                        ExternString* h_str);
+                                       ExternString* h_str);
+
+  // This method does not actually create ExternString instances.
+  static MaybeLocal<Value> NewSimpleFromCopy(Isolate* isolate,
+                                             const TypeName* data,
+                                             size_t length,
+                                             Local<Value>* error);
 
   Isolate* isolate_;
   const TypeName* data_;
@@ -131,16 +159,51 @@ typedef ExternString<String::ExternalStringResource,
 
 
 template <>
-MaybeLocal<String> ExternOneByteString::NewExternal(
+MaybeLocal<Value> ExternOneByteString::NewExternal(
     Isolate* isolate, ExternOneByteString* h_str) {
-  return String::NewExternalOneByte(isolate, h_str);
+  return String::NewExternalOneByte(isolate, h_str).FromMaybe(Local<Value>());
 }
 
 
 template <>
-MaybeLocal<String> ExternTwoByteString::NewExternal(
+MaybeLocal<Value> ExternTwoByteString::NewExternal(
     Isolate* isolate, ExternTwoByteString* h_str) {
-  return String::NewExternalTwoByte(isolate, h_str);
+  return String::NewExternalTwoByte(isolate, h_str).FromMaybe(Local<Value>());
+}
+
+template <>
+MaybeLocal<Value> ExternOneByteString::NewSimpleFromCopy(Isolate* isolate,
+                                                         const char* data,
+                                                         size_t length,
+                                                         Local<Value>* error) {
+  MaybeLocal<String> str =
+      String::NewFromOneByte(isolate,
+                             reinterpret_cast<const uint8_t*>(data),
+                             v8::NewStringType::kNormal,
+                             length);
+  if (str.IsEmpty()) {
+    *error = SB_STRING_TOO_LONG_ERROR;
+    return MaybeLocal<Value>();
+  }
+  return str.ToLocalChecked();
+}
+
+
+template <>
+MaybeLocal<Value> ExternTwoByteString::NewSimpleFromCopy(Isolate* isolate,
+                                                         const uint16_t* data,
+                                                         size_t length,
+                                                         Local<Value>* error) {
+  MaybeLocal<String> str =
+      String::NewFromTwoByte(isolate,
+                             data,
+                             v8::NewStringType::kNormal,
+                             length);
+  if (str.IsEmpty()) {
+    *error = SB_STRING_TOO_LONG_ERROR;
+    return MaybeLocal<Value>();
+  }
+  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<Value>();                                     \
+    }                                                                 \
+  } while (0)
 
-Local<Value> StringBytes::Encode(Isolate* isolate,
-                                 const char* buf,
-                                 size_t buflen,
-                                 enum encoding encoding) {
-  EscapableHandleScope scope(isolate);
 
+MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
+                                      const char* buf,
+                                      size_t buflen,
+                                      enum encoding encoding,
+                                      Local<Value>* error) {
   CHECK_NE(encoding, UCS2);
-  CHECK_LE(buflen, Buffer::kMaxLength);
+  CHECK_BUFLEN_IN_RANGE(buflen);
-  if (!buflen && encoding != BUFFER)
+
-    return scope.Escape(String::Empty(isolate));
+  *error = Local<Value>();
+  if (!buflen && encoding != BUFFER) {
+    return String::Empty(isolate);
+  }
+
+  MaybeLocal<String> val;
 
-  Local<String> val;
   switch (encoding) {
     case BUFFER:
       {
-        Local<Object> vbuf =
+        auto maybe_buf = Buffer::Copy(isolate, buf, buflen);
-            Buffer::Copy(isolate, buf, buflen).ToLocalChecked();
+        if (maybe_buf.IsEmpty()) {
-        return scope.Escape(vbuf);
+          *error = SB_BUFFER_CREATION_ERROR;
+          return MaybeLocal<Value>();
+        }
+        return maybe_buf.ToLocalChecked();
       }
 
     case ASCII:
       if (contains_non_ascii(buf, buflen)) {
         char* out = node::UncheckedMalloc(buflen);
         if (out == nullptr) {
-          return Local<String>();
+          *error = SB_MALLOC_FAILED_ERROR;
+          return MaybeLocal<Value>();
         }
         force_ascii(buf, out, buflen);
-        if (buflen < EXTERN_APEX) {
+        return ExternOneByteString::New(isolate, out, buflen, error);
-          val = OneByteString(isolate, out, buflen);
-          free(out);
-        } else {
-          val = ExternOneByteString::New(isolate, out, buflen);
-        }
       } else {
-        if (buflen < EXTERN_APEX)
+        return ExternOneByteString::NewFromCopy(isolate, buf, buflen, error);
-          val = OneByteString(isolate, buf, buflen);
-        else
-          val = ExternOneByteString::NewFromCopy(isolate, buf, buflen);
       }
-      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<Value>();
+      }
+      return val.ToLocalChecked();
 
     case LATIN1:
-      if (buflen < EXTERN_APEX)
+      return ExternOneByteString::NewFromCopy(isolate, buf, buflen, error);
-        val = OneByteString(isolate, buf, buflen);
-      else
-        val = ExternOneByteString::NewFromCopy(isolate, buf, buflen);
-      break;
 
     case BASE64: {
       size_t dlen = base64_encoded_size(buflen);
       char* dst = node::UncheckedMalloc(dlen);
       if (dst == nullptr) {
-        return Local<String>();
+        *error = SB_MALLOC_FAILED_ERROR;
+        return MaybeLocal<Value>();
       }
 
       size_t written = base64_encode(buf, buflen, dst, dlen);
       CHECK_EQ(written, dlen);
 
-      if (dlen < EXTERN_APEX) {
+      return ExternOneByteString::New(isolate, dst, dlen, error);
-        val = OneByteString(isolate, dst, dlen);
-        free(dst);
-      } else {
-        val = ExternOneByteString::New(isolate, dst, dlen);
-      }
-      break;
     }
 
     case HEX: {
       size_t dlen = buflen * 2;
       char* dst = node::UncheckedMalloc(dlen);
       if (dst == nullptr) {
-        return Local<String>();
+        *error = SB_MALLOC_FAILED_ERROR;
+        return MaybeLocal<Value>();
       }
       size_t written = hex_encode(buf, buflen, dst, dlen);
       CHECK_EQ(written, dlen);
 
-      if (dlen < EXTERN_APEX) {
+      return ExternOneByteString::New(isolate, dst, dlen, error);
-        val = OneByteString(isolate, dst, dlen);
-        free(dst);
-      } else {
-        val = ExternOneByteString::New(isolate, dst, dlen);
-      }
-      break;
     }
 
     default:
@@ -708,13 +767,17 @@ Local<Value> StringBytes::Encode(Isolate* isolate,
       break;
   }
 
-  return scope.Escape(val);
+  UNREACHABLE();
 }
 
 
-Local<Value> StringBytes::Encode(Isolate* isolate,
+MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
-                                 const uint16_t* buf,
+                                      const uint16_t* buf,
-                                 size_t buflen) {
+                                      size_t buflen,
+                                      Local<Value>* error) {
+  CHECK_BUFLEN_IN_RANGE(buflen);
+  *error = Local<Value>();
+
   // 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<Value> StringBytes::Encode(Isolate* isolate,
     buf = &dst[0];
   }
 
-  Local<String> val;
+  return ExternTwoByteString::NewFromCopy(isolate, buf, buflen, error);
-  if (buflen < EXTERN_APEX) {
-    val = String::NewFromTwoByte(isolate,
-                                 buf,
-                                 String::kNormalString,
-                                 buflen);
-  } else {
-    val = ExternTwoByteString::NewFromCopy(isolate, buf, buflen);
-  }
-
-  return val;
 }
 
-Local<Value> StringBytes::Encode(Isolate* isolate,
+MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
-                                 const char* buf,
+                                      const char* buf,
-                                 enum encoding encoding) {
+                                      enum encoding encoding,
+                                      Local<Value>* error) {
   const size_t len = strlen(buf);
-  Local<Value> ret;
+  MaybeLocal<Value> 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<Value> StringBytes::Encode(Isolate* isolate,
     }
     ret = vec.empty() ?
         static_cast< Local<Value> >(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;
 }

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<v8::Value> Encode(v8::Isolate* isolate,
+  static v8::MaybeLocal<v8::Value> Encode(v8::Isolate* isolate,
-                                     const char* buf,
+                                          const char* buf,
-                                     size_t buflen,
+                                          size_t buflen,
-                                     enum encoding encoding);
+                                          enum encoding encoding,
+                                          v8::Local<v8::Value>* error);
 
   // The input buffer should be in host endianness.
-  static v8::Local<v8::Value> Encode(v8::Isolate* isolate,
+  static v8::MaybeLocal<v8::Value> Encode(v8::Isolate* isolate,
-                                     const uint16_t* buf,
+                                          const uint16_t* buf,
-                                     size_t buflen);
+                                          size_t buflen,
+                                          v8::Local<v8::Value>* error);
 
-  static v8::Local<v8::Value> Encode(v8::Isolate* isolate,
+  static v8::MaybeLocal<v8::Value> Encode(v8::Isolate* isolate,
-                                     const char* buf,
+                                          const char* buf,
-                                     enum encoding encoding);
+                                          enum encoding encoding,
+                                          v8::Local<v8::Value>* error);
 
  private:
   static size_t WriteUCS2(char* buf,