Skip to content

Commit

Permalink
src: use proper errors as coming from StringBytes
Browse files Browse the repository at this point in the history 
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: #14579
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
@addaleax
addaleax committed Aug 23, 2017
1 parent 67269fd commit 116447a
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 71 deletions.
60 changes: 12 additions & 48 deletions src/node_file.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand All @@ -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();
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
}
Expand Down
26 changes: 5 additions & 21 deletions src/node_os.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 0 additions & 2 deletions src/string_bytes.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 116447a

Please sign in to comment.