Skip to content

Commit 784c6d4

Browse files
addaleaxaddaleax
committed
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: #14579 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
1 parent 7e54424 commit 784c6d4

File tree

3 files changed

+17
-71
lines changed

3 files changed

+17
-71
lines changed

‎src/node_file.cc

+12-48
Original file line numberDiff line numberDiff line change
@@ -244,13 +244,7 @@ void After(uv_fs_t *req) {
244244
req_wrap->encoding_,
245245
&error);
246246
if (link.IsEmpty()) {
247-
// TODO(addaleax): Use `error` itself here.
248-
argv[0] = UVException(env->isolate(),
249-
UV_EINVAL,
250-
req_wrap->syscall(),
251-
"Invalid character encoding for filename",
252-
req->path,
253-
req_wrap->data());
247+
argv[0] = error;
254248
} else {
255249
argv[1] = link.ToLocalChecked();
256250
}
@@ -263,13 +257,7 @@ void After(uv_fs_t *req) {
263257
req_wrap->encoding_,
264258
&error);
265259
if (link.IsEmpty()) {
266-
// TODO(addaleax): Use `error` itself here.
267-
argv[0] = UVException(env->isolate(),
268-
UV_EINVAL,
269-
req_wrap->syscall(),
270-
"Invalid character encoding for link",
271-
req->path,
272-
req_wrap->data());
260+
argv[0] = error;
273261
} else {
274262
argv[1] = link.ToLocalChecked();
275263
}
@@ -281,13 +269,7 @@ void After(uv_fs_t *req) {
281269
req_wrap->encoding_,
282270
&error);
283271
if (link.IsEmpty()) {
284-
// TODO(addaleax): Use `error` itself here.
285-
argv[0] = UVException(env->isolate(),
286-
UV_EINVAL,
287-
req_wrap->syscall(),
288-
"Invalid character encoding for link",
289-
req->path,
290-
req_wrap->data());
272+
argv[0] = error;
291273
} else {
292274
argv[1] = link.ToLocalChecked();
293275
}
@@ -326,13 +308,7 @@ void After(uv_fs_t *req) {
326308
req_wrap->encoding_,
327309
&error);
328310
if (filename.IsEmpty()) {
329-
// TODO(addaleax): Use `error` itself here.
330-
argv[0] = UVException(env->isolate(),
331-
UV_EINVAL,
332-
req_wrap->syscall(),
333-
"Invalid character encoding for filename",
334-
req->path,
335-
req_wrap->data());
311+
argv[0] = error;
336312
break;
337313
}
338314
name_argv[name_idx++] = filename.ToLocalChecked();
@@ -711,11 +687,8 @@ static void ReadLink(const FunctionCallbackInfo<Value>& args) {
711687
encoding,
712688
&error);
713689
if (rc.IsEmpty()) {
714-
// TODO(addaleax): Use `error` itself here.
715-
return env->ThrowUVException(UV_EINVAL,
716-
"readlink",
717-
"Invalid character encoding for link",
718-
*path);
690+
env->isolate()->ThrowException(error);
691+
return;
719692
}
720693
args.GetReturnValue().Set(rc.ToLocalChecked());
721694
}
@@ -886,11 +859,8 @@ static void RealPath(const FunctionCallbackInfo<Value>& args) {
886859
encoding,
887860
&error);
888861
if (rc.IsEmpty()) {
889-
// TODO(addaleax): Use `error` itself here.
890-
return env->ThrowUVException(UV_EINVAL,
891-
"realpath",
892-
"Invalid character encoding for path",
893-
*path);
862+
env->isolate()->ThrowException(error);
863+
return;
894864
}
895865
args.GetReturnValue().Set(rc.ToLocalChecked());
896866
}
@@ -940,11 +910,8 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
940910
encoding,
941911
&error);
942912
if (filename.IsEmpty()) {
943-
// TODO(addaleax): Use `error` itself here.
944-
return env->ThrowUVException(UV_EINVAL,
945-
"readdir",
946-
"Invalid character encoding for filename",
947-
*path);
913+
env->isolate()->ThrowException(error);
914+
return;
948915
}
949916

950917
name_v[name_idx++] = filename.ToLocalChecked();
@@ -1405,11 +1372,8 @@ static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
14051372
MaybeLocal<Value> rc =
14061373
StringBytes::Encode(env->isolate(), path, encoding, &error);
14071374
if (rc.IsEmpty()) {
1408-
// TODO(addaleax): Use `error` itself here.
1409-
return env->ThrowUVException(UV_EINVAL,
1410-
"mkdtemp",
1411-
"Invalid character encoding for filename",
1412-
*tmpl);
1375+
env->isolate()->ThrowException(error);
1376+
return;
14131377
}
14141378
args.GetReturnValue().Set(rc.ToLocalChecked());
14151379
}

‎src/node_os.cc

+5-21
Original file line numberDiff line numberDiff line change
@@ -376,27 +376,11 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
376376
else
377377
shell = StringBytes::Encode(env->isolate(), pwd.shell, encoding, &error);
378378

379-
uv_os_free_passwd(&pwd);
380-
381-
if (username.IsEmpty()) {
382-
// TODO(addaleax): Use `error` itself here.
383-
return env->ThrowUVException(UV_EINVAL,
384-
"uv_os_get_passwd",
385-
"Invalid character encoding for username");
386-
}
387-
388-
if (homedir.IsEmpty()) {
389-
// TODO(addaleax): Use `error` itself here.
390-
return env->ThrowUVException(UV_EINVAL,
391-
"uv_os_get_passwd",
392-
"Invalid character encoding for homedir");
393-
}
394-
395-
if (shell.IsEmpty()) {
396-
// TODO(addaleax): Use `error` itself here.
397-
return env->ThrowUVException(UV_EINVAL,
398-
"uv_os_get_passwd",
399-
"Invalid character encoding for shell");
379+
if (username.IsEmpty() || homedir.IsEmpty() || shell.IsEmpty()) {
380+
CHECK(!error.IsEmpty());
381+
uv_os_free_passwd(&pwd);
382+
env->isolate()->ThrowException(error);
383+
return;
400384
}
401385

402386
Local<Object> entry = Object::New(env->isolate());

‎src/string_bytes.cc

-2
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,6 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
686686
CHECK_NE(encoding, UCS2);
687687
CHECK_BUFLEN_IN_RANGE(buflen);
688688

689-
*error = Local<Value>();
690689
if (!buflen && encoding != BUFFER) {
691690
return String::Empty(isolate);
692691
}
@@ -772,7 +771,6 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
772771
size_t buflen,
773772
Local<Value>* error) {
774773
CHECK_BUFLEN_IN_RANGE(buflen);
775-
*error = Local<Value>();
776774

777775
// Node's "ucs2" encoding expects LE character data inside a
778776
// Buffer, so we need to reorder on BE platforms. See

0 commit comments

Comments
 (0)