src: use proper errors as coming from StringBytes

The previous errors were incorrect here, as the code
only failed in situations where strings exceeded size limits or
an OOM situation was encountered, not for invalid encodings
(which aren’t even detected explicitly).

Unfortunately, these situations are hard to test for.

PR-URL: https://github.com/nodejs/node/pull/14579
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

3 files changed, 17 insertions, 71 deletions

diff --git a/src/node_file.cc b/src/node_file.cc
index ee69915ddc..f72226a6e6 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -244,13 +244,7 @@ void After(uv_fs_t *req) {
                                    req_wrap->encoding_,
                                    &error);
         if (link.IsEmpty()) {
-          // TODO(addaleax): Use `error` itself here.
-          argv[0] = UVException(env->isolate(),
-                                UV_EINVAL,
-                                req_wrap->syscall(),
-                                "Invalid character encoding for filename",
-                                req->path,
-                                req_wrap->data());
+          argv[0] = error;
         } else {
           argv[1] = link.ToLocalChecked();
         }
@@ -263,13 +257,7 @@ void After(uv_fs_t *req) {
                                    req_wrap->encoding_,
                                    &error);
         if (link.IsEmpty()) {
-          // TODO(addaleax): Use `error` itself here.
-          argv[0] = UVException(env->isolate(),
-                                UV_EINVAL,
-                                req_wrap->syscall(),
-                                "Invalid character encoding for link",
-                                req->path,
-                                req_wrap->data());
+          argv[0] = error;
         } else {
           argv[1] = link.ToLocalChecked();
         }
@@ -281,13 +269,7 @@ void After(uv_fs_t *req) {
                                    req_wrap->encoding_,
                                    &error);
         if (link.IsEmpty()) {
-          // TODO(addaleax): Use `error` itself here.
-          argv[0] = UVException(env->isolate(),
-                                UV_EINVAL,
-                                req_wrap->syscall(),
-                                "Invalid character encoding for link",
-                                req->path,
-                                req_wrap->data());
+          argv[0] = error;
         } else {
           argv[1] = link.ToLocalChecked();
         }
@@ -326,13 +308,7 @@ void After(uv_fs_t *req) {
                                     req_wrap->encoding_,
                                     &error);
             if (filename.IsEmpty()) {
-              // TODO(addaleax): Use `error` itself here.
-              argv[0] = UVException(env->isolate(),
-                                    UV_EINVAL,
-                                    req_wrap->syscall(),
-                                    "Invalid character encoding for filename",
-                                    req->path,
-                                    req_wrap->data());
+              argv[0] = error;
               break;
             }
             name_argv[name_idx++] = filename.ToLocalChecked();
@@ -711,11 +687,8 @@ static void ReadLink(const FunctionCallbackInfo<Value>& args) {
                                                encoding,
                                                &error);
     if (rc.IsEmpty()) {
-      // TODO(addaleax): Use `error` itself here.
-      return env->ThrowUVException(UV_EINVAL,
-                                   "readlink",
-                                   "Invalid character encoding for link",
-                                   *path);
+      env->isolate()->ThrowException(error);
+      return;
     }
     args.GetReturnValue().Set(rc.ToLocalChecked());
   }
@@ -886,11 +859,8 @@ static void RealPath(const FunctionCallbackInfo<Value>& args) {
                                                encoding,
                                                &error);
     if (rc.IsEmpty()) {
-      // TODO(addaleax): Use `error` itself here.
-      return env->ThrowUVException(UV_EINVAL,
-                                   "realpath",
-                                   "Invalid character encoding for path",
-                                   *path);
+      env->isolate()->ThrowException(error);
+      return;
     }
     args.GetReturnValue().Set(rc.ToLocalChecked());
   }
@@ -940,11 +910,8 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
                                                        encoding,
                                                        &error);
       if (filename.IsEmpty()) {
-        // TODO(addaleax): Use `error` itself here.
-        return env->ThrowUVException(UV_EINVAL,
-                                     "readdir",
-                                     "Invalid character encoding for filename",
-                                     *path);
+        env->isolate()->ThrowException(error);
+        return;
       }
 
       name_v[name_idx++] = filename.ToLocalChecked();
@@ -1405,11 +1372,8 @@ static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
     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);
+      env->isolate()->ThrowException(error);
+      return;
     }
     args.GetReturnValue().Set(rc.ToLocalChecked());
   }
diff --git a/src/node_os.cc b/src/node_os.cc
index c1d1de971f..c71ca401ed 100644
--- a/src/node_os.cc
+++ b/src/node_os.cc
@@ -376,27 +376,11 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
   else
     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");
+  if (username.IsEmpty() || homedir.IsEmpty() || shell.IsEmpty()) {
+    CHECK(!error.IsEmpty());
+    uv_os_free_passwd(&pwd);
+    env->isolate()->ThrowException(error);
+    return;
   }
 
   Local<Object> entry = Object::New(env->isolate());
diff --git a/src/string_bytes.cc b/src/string_bytes.cc
index 5aa3b8ca77..9df42f3bfa 100644
--- a/src/string_bytes.cc
+++ b/src/string_bytes.cc
@@ -686,7 +686,6 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
   CHECK_NE(encoding, UCS2);
   CHECK_BUFLEN_IN_RANGE(buflen);
 
-  *error = Local<Value>();
   if (!buflen && encoding != BUFFER) {
     return String::Empty(isolate);
   }
@@ -772,7 +771,6 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
                                       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