From 784c6d40f88c5a3b4270f2a6d4c7c120b4b12af6 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 2 Aug 2017 00:13:39 +0200 Subject: [PATCH] src: use proper errors as coming from StringBytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-By: Ben Noordhuis --- src/node_file.cc | 60 +++++++++------------------------------------ src/node_os.cc | 26 ++++---------------- src/string_bytes.cc | 2 -- 3 files changed, 17 insertions(+), 71 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index ee69915ddcaf70..f72226a6e6f39c 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& 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& 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& 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& args) { 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); + env->isolate()->ThrowException(error); + return; } args.GetReturnValue().Set(rc.ToLocalChecked()); } diff --git a/src/node_os.cc b/src/node_os.cc index c1d1de971f4a09..c71ca401ed6609 100644 --- a/src/node_os.cc +++ b/src/node_os.cc @@ -376,27 +376,11 @@ static void GetUserInfo(const FunctionCallbackInfo& 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 entry = Object::New(env->isolate()); diff --git a/src/string_bytes.cc b/src/string_bytes.cc index 5aa3b8ca770192..9df42f3bfada01 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -686,7 +686,6 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, CHECK_NE(encoding, UCS2); CHECK_BUFLEN_IN_RANGE(buflen); - *error = Local(); if (!buflen && encoding != BUFFER) { return String::Empty(isolate); } @@ -772,7 +771,6 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, 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