diff --git a/lib/child_process.js b/lib/child_process.js index ab9ceb5bad30..cde92831801c 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -352,6 +352,8 @@ function normalizeSpawnArguments(file, args, options) { if (Array.isArray(args)) { args = args.slice(0); + for (var i = 0; i < args.length; i++) + args[i] = `${args[i]}`; } else if (args !== undefined && (args === null || typeof args !== 'object')) { throw new TypeError('Incorrect value of args option'); diff --git a/src/env.cc b/src/env.cc index 40f0c9ebd66a..2dd1635411fb 100644 --- a/src/env.cc +++ b/src/env.cc @@ -127,7 +127,8 @@ void Environment::PrintSyncTrace() const { const int column = stack_frame->GetColumn(); if (stack_frame->IsEval()) { - if (stack_frame->GetScriptId() == Message::kNoScriptIdInfo) { + if (script_name.IsInvalidated() || + stack_frame->GetScriptId() == Message::kNoScriptIdInfo) { fprintf(stderr, " at [eval]:%i:%i\n", line_number, column); } else { fprintf(stderr, @@ -139,13 +140,16 @@ void Environment::PrintSyncTrace() const { break; } - if (fn_name_s.length() == 0) { - fprintf(stderr, " at %s:%i:%i\n", *script_name, line_number, column); + if (fn_name_s.IsInvalidated() || fn_name_s.length() == 0) { + fprintf(stderr, + " at %s:%i:%i\n", + script_name.IsInvalidated() ? "" : *script_name, + line_number, column); } else { fprintf(stderr, " at %s (%s:%i:%i)\n", *fn_name_s, - *script_name, + script_name.IsInvalidated() ? "" : *script_name, line_number, column); } diff --git a/src/node.cc b/src/node.cc index fda6c3f25773..521bc118bce4 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1485,6 +1485,8 @@ enum encoding ParseEncoding(Isolate* isolate, return default_encoding; node::Utf8Value encoding(isolate, encoding_v); + if (encoding.IsInvalidated()) + return default_encoding; return ParseEncoding(*encoding, default_encoding); } @@ -1555,10 +1557,14 @@ void AppendExceptionLine(Environment* env, // Print (filename):(line number): (message). node::Utf8Value filename(env->isolate(), message->GetScriptResourceName()); + if (filename.IsInvalidated()) + return; const char* filename_string = *filename; int linenum = message->GetLineNumber(); // Print line of source code. node::Utf8Value sourceline(env->isolate(), message->GetSourceLine()); + if (sourceline.IsInvalidated()) + return; const char* sourceline_string = *sourceline; // Because of how node modules work, all scripts are wrapped with a @@ -1667,6 +1673,10 @@ static void ReportException(Environment* env, } node::Utf8Value trace(env->isolate(), trace_value); + if (trace.IsInvalidated()) { + // Something is wrong; abort. + return; + } // range errors have a trace member set to undefined if (trace.length() > 0 && !trace_value->IsUndefined()) { @@ -1674,6 +1684,7 @@ static void ReportException(Environment* env, PrintErrorString("%s\n", *trace); } else { node::Utf8Value arrow_string(env->isolate(), arrow); + CHECK(!arrow_string.IsInvalidated()); PrintErrorString("%s\n%s\n", *arrow_string, *trace); } } else { @@ -1699,17 +1710,25 @@ static void ReportException(Environment* env, PrintErrorString("%s\n", *message ? *message : ""); } else { - node::Utf8Value name_string(env->isolate(), name); - node::Utf8Value message_string(env->isolate(), message); + const char* name_string = ""; + node::Utf8Value name_val(env->isolate(), name); + if (!name_val.IsInvalidated()) + name_string = *name_val; + + const char* message_string = ""; + node::Utf8Value message_val(env->isolate(), message); + if (!message_val.IsInvalidated()) + message_string = *message_val; if (arrow.IsEmpty() || !arrow->IsString() || decorated) { - PrintErrorString("%s: %s\n", *name_string, *message_string); + PrintErrorString("%s: %s\n", name_string, message_string); } else { - node::Utf8Value arrow_string(env->isolate(), arrow); + node::Utf8Value arrow_val(env->isolate(), arrow); + CHECK(!arrow_val.IsInvalidated()); PrintErrorString("%s\n%s: %s\n", - *arrow_string, - *name_string, - *message_string); + *arrow_val, + name_string, + message_string); } } } @@ -1858,6 +1877,8 @@ static void Chdir(const FunctionCallbackInfo& args) { } node::Utf8Value path(args.GetIsolate(), args[0]); + CHECK(!path.IsInvalidated()); + int err = uv_chdir(*path); if (err) { return env->ThrowUVException(err, "uv_chdir"); @@ -1904,6 +1925,7 @@ static void Umask(const FunctionCallbackInfo& args) { } else { oct = 0; node::Utf8Value str(env->isolate(), args[0]); + CHECK(!str.IsInvalidated()); // Parse the octal string. for (size_t i = 0; i < str.length(); i++) { @@ -2011,6 +2033,8 @@ static uid_t uid_by_name(Isolate* isolate, Local value) { return static_cast(value->Uint32Value()); } else { node::Utf8Value name(isolate, value); + if (name.IsInvalidated()) + return uid_not_found; return uid_by_name(*name); } } @@ -2021,6 +2045,8 @@ static gid_t gid_by_name(Isolate* isolate, Local value) { return static_cast(value->Uint32Value()); } else { node::Utf8Value name(isolate, value); + if (name.IsInvalidated()) + return gid_not_found; return gid_by_name(*name); } } @@ -2206,6 +2232,9 @@ static void InitGroups(const FunctionCallbackInfo& args) { } node::Utf8Value arg0(env->isolate(), args[0]); + if (arg0.IsInvalidated()) + return; + gid_t extra_group; bool must_free; char* user; @@ -2437,6 +2466,8 @@ void DLOpen(const FunctionCallbackInfo& args) { Local module = args[0]->ToObject(env->isolate()); // Cast node::Utf8Value filename(env->isolate(), args[1]); // Cast + if (filename.IsInvalidated()) + return; const bool is_dlopen_error = uv_dlopen(*filename, &lib); // Objects containing v14 or later modules will have registered themselves @@ -2643,6 +2674,8 @@ static void Binding(const FunctionCallbackInfo& args) { Local module = args[0]->ToString(env->isolate()); node::Utf8Value module_v(env->isolate(), module); + if (module_v.IsInvalidated()) + return; Local cache = env->binding_cache_object(); Local exports; @@ -2695,6 +2728,8 @@ static void LinkedBinding(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args.GetIsolate()); Local module_name = args[0]->ToString(env->isolate()); + if (module_name.IsEmpty()) + return; Local cache = env->binding_cache_object(); Local exports_v = cache->Get(module_name); @@ -2703,6 +2738,7 @@ static void LinkedBinding(const FunctionCallbackInfo& args) { return args.GetReturnValue().Set(exports_v.As()); node::Utf8Value module_name_v(env->isolate(), module_name); + CHECK(!module_name_v.IsInvalidated()); node_module* mod = get_linked_module(*module_name_v); if (mod == nullptr) { @@ -2748,6 +2784,8 @@ static void ProcessTitleSetter(Local property, Local value, const PropertyCallbackInfo& info) { node::Utf8Value title(info.GetIsolate(), value); + if (title.IsInvalidated()) + return; // TODO(piscisaureus): protect with a lock uv_set_process_title(*title); } @@ -2761,12 +2799,14 @@ static void EnvGetter(Local property, } #ifdef __POSIX__ node::Utf8Value key(isolate, property); + CHECK(!key.IsInvalidated()); const char* val = getenv(*key); if (val) { return info.GetReturnValue().Set(String::NewFromUtf8(isolate, val)); } #else // _WIN32 node::TwoByteValue key(isolate, property); + CHECK(!key.IsInvalidated()); WCHAR buffer[32767]; // The maximum size allowed for environment variables. DWORD result = GetEnvironmentVariableW(reinterpret_cast(*key), buffer, @@ -2789,11 +2829,19 @@ static void EnvSetter(Local property, const PropertyCallbackInfo& info) { #ifdef __POSIX__ node::Utf8Value key(info.GetIsolate(), property); + if (key.IsInvalidated()) + return; node::Utf8Value val(info.GetIsolate(), value); + if (val.IsInvalidated()) + return; setenv(*key, *val, 1); #else // _WIN32 node::TwoByteValue key(info.GetIsolate(), property); + if (key.IsInvalidated()) + return; node::TwoByteValue val(info.GetIsolate(), value); + if (val.IsInvalidated()) + return; WCHAR* key_ptr = reinterpret_cast(*key); // Environment variables that start with '=' are read-only. if (key_ptr[0] != L'=') { @@ -2811,10 +2859,12 @@ static void EnvQuery(Local property, if (property->IsString()) { #ifdef __POSIX__ node::Utf8Value key(info.GetIsolate(), property); + CHECK(!key.IsInvalidated()); if (getenv(*key)) rc = 0; #else // _WIN32 node::TwoByteValue key(info.GetIsolate(), property); + CHECK(!key.IsInvalidated()); WCHAR* key_ptr = reinterpret_cast(*key); if (GetEnvironmentVariableW(key_ptr, nullptr, 0) > 0 || GetLastError() == ERROR_SUCCESS) { @@ -2838,9 +2888,11 @@ static void EnvDeleter(Local property, if (property->IsString()) { #ifdef __POSIX__ node::Utf8Value key(info.GetIsolate(), property); + CHECK(!key.IsInvalidated()); unsetenv(*key); #else node::TwoByteValue key(info.GetIsolate(), property); + CHECK(!key.IsInvalidated()); WCHAR* key_ptr = reinterpret_cast(*key); SetEnvironmentVariableW(key_ptr, nullptr); #endif @@ -3441,6 +3493,7 @@ static void RawDebug(const FunctionCallbackInfo& args) { CHECK(args.Length() == 1 && args[0]->IsString() && "must be called with a single string"); node::Utf8Value message(args.GetIsolate(), args[0]); + CHECK(!message.IsInvalidated()); PrintErrorString("%s\n", *message); fflush(stderr); } diff --git a/src/node_i18n.cc b/src/node_i18n.cc index 284f9b0b6c80..8fac98c5f5ee 100644 --- a/src/node_i18n.cc +++ b/src/node_i18n.cc @@ -401,6 +401,7 @@ static void GetVersion(const FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 1); CHECK(args[0]->IsString()); Utf8Value val(env->isolate(), args[0]); + CHECK(!val.IsInvalidated()); UErrorCode status = U_ZERO_ERROR; char buf[U_MAX_VERSION_STRING_LENGTH] = ""; // Possible output buffer. const char* versionString = GetVersion(*val, buf, &status); @@ -513,6 +514,7 @@ static void ToUnicode(const FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 1); CHECK(args[0]->IsString()); Utf8Value val(env->isolate(), args[0]); + CHECK(!val.IsInvalidated()); // optional arg bool lenient = args[1]->BooleanValue(env->context()).FromJust(); @@ -535,6 +537,7 @@ static void ToASCII(const FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 1); CHECK(args[0]->IsString()); Utf8Value val(env->isolate(), args[0]); + CHECK(!val.IsInvalidated()); // optional arg bool lenient = args[1]->BooleanValue(env->context()).FromJust(); @@ -609,6 +612,8 @@ static void GetStringWidth(const FunctionCallbackInfo& args) { } TwoByteValue value(env->isolate(), args[0]); + if (value.IsInvalidated()) + return; // reinterpret_cast is required by windows to compile UChar* str = reinterpret_cast(*value); static_assert(sizeof(*str) == sizeof(**value), diff --git a/src/node_url.cc b/src/node_url.cc index 1aae55711585..efc560843dcf 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -41,7 +41,8 @@ using v8::Value; { \ Local val = GET(env, obj, #name); \ if (val->IsString()) { \ - Utf8Value value(env->isolate(), val.As()); \ + Utf8Value value(env->isolate(), val); \ + CHECK(!value.IsInvalidated()); \ data->name = *value; \ data->flags |= flag; \ } \ @@ -509,7 +510,8 @@ namespace url { for (int32_t n = 0; n < len; n++) { Local val = ary->Get(env->context(), n).ToLocalChecked(); if (val->IsString()) { - Utf8Value value(env->isolate(), val.As()); + Utf8Value value(env->isolate(), val); + CHECK(!value.IsInvalidated()); vec->push_back(std::string(*value, value.length())); } } @@ -562,6 +564,7 @@ namespace url { Local scheme = GET(env, context_obj, "scheme"); if (scheme->IsString()) { Utf8Value value(env->isolate(), scheme); + CHECK(!value.IsInvalidated()); context->scheme.assign(*value, value.length()); } Local port = GET(env, context_obj, "port"); @@ -1297,7 +1300,6 @@ namespace url { static void Parse(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK_GE(args.Length(), 5); - CHECK(args[0]->IsString()); CHECK(args[2]->IsUndefined() || args[2]->IsNull() || args[2]->IsObject()); @@ -1306,6 +1308,7 @@ namespace url { args[3]->IsObject()); CHECK(args[4]->IsFunction()); Utf8Value input(env->isolate(), args[0]); + CHECK(!input.IsInvalidated()); enum url_parse_state state_override = kUnknownState; if (args[1]->IsNumber()) { state_override = static_cast( @@ -1323,8 +1326,8 @@ namespace url { static void EncodeAuthSet(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK_GE(args.Length(), 1); - CHECK(args[0]->IsString()); Utf8Value value(env->isolate(), args[0]); + CHECK(!value.IsInvalidated()); std::string output; const size_t len = value.length(); output.reserve(len); @@ -1341,10 +1344,10 @@ namespace url { static void ToUSVString(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK_GE(args.Length(), 2); - CHECK(args[0]->IsString()); CHECK(args[1]->IsNumber()); TwoByteValue value(env->isolate(), args[0]); + CHECK(!value.IsInvalidated()); const size_t n = value.length(); const int64_t start = args[1]->IntegerValue(env->context()).FromJust(); @@ -1376,8 +1379,8 @@ namespace url { static void DomainToASCII(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK_GE(args.Length(), 1); - CHECK(args[0]->IsString()); Utf8Value value(env->isolate(), args[0]); + CHECK(!value.IsInvalidated()); url_host host{{""}, HOST_TYPE_DOMAIN}; ParseHost(&host, *value, value.length()); @@ -1396,8 +1399,8 @@ namespace url { static void DomainToUnicode(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK_GE(args.Length(), 1); - CHECK(args[0]->IsString()); Utf8Value value(env->isolate(), args[0]); + CHECK(!value.IsInvalidated()); url_host host{{""}, HOST_TYPE_DOMAIN}; ParseHost(&host, *value, value.length(), true); diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 430bbe3b8a37..b51db7d5f41f 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -814,9 +814,7 @@ int SyncProcessRunner::ParseStdioOptions(Local js_value) { HandleScope scope(env()->isolate()); Local js_stdio_options; - if (!js_value->IsArray()) - return UV_EINVAL; - + CHECK(js_value->IsArray()); js_stdio_options = js_value.As(); stdio_count_ = js_stdio_options->Length(); @@ -946,10 +944,8 @@ int SyncProcessRunner::CopyJsString(Local js_value, size_t size, written; char* buffer; - if (js_value->IsString()) - js_string = js_value.As(); - else - js_string = js_value->ToString(env()->isolate()); + CHECK(js_value->IsString()); + js_string = js_value.As(); // Include space for null terminator byte. size = StringBytes::StorageSize(isolate, js_string, UTF8) + 1; @@ -973,19 +969,10 @@ int SyncProcessRunner::CopyJsStringArray(Local js_value, char** list; char* buffer; - if (!js_value->IsArray()) - return UV_EINVAL; - - js_array = js_value.As()->Clone().As(); + CHECK(js_value->IsArray()); + js_array = js_value.As(); length = js_array->Length(); - // Convert all array elements to string. Modify the js object itself if - // needed - it's okay since we cloned the original object. - for (uint32_t i = 0; i < length; i++) { - if (!js_array->Get(i)->IsString()) - js_array->Set(i, js_array->Get(i)->ToString(env()->isolate())); - } - // Index has a pointer to every string element, plus one more for a final // null pointer. list_size = (length + 1) * sizeof *list; @@ -994,7 +981,9 @@ int SyncProcessRunner::CopyJsStringArray(Local js_value, // after every string. Align strings to cache lines. data_size = 0; for (uint32_t i = 0; i < length; i++) { - data_size += StringBytes::StorageSize(isolate, js_array->Get(i), UTF8) + 1; + auto el = js_array->Get(i); + CHECK(el->IsString()); + data_size += StringBytes::StorageSize(isolate, el, UTF8) + 1; data_size = ROUND_UP(data_size, sizeof(void*)); } diff --git a/src/util.cc b/src/util.cc index 4a89b3a42f24..4c5c5c8c634e 100644 --- a/src/util.cc +++ b/src/util.cc @@ -27,19 +27,22 @@ namespace node { -using v8::Isolate; +using v8::Context; using v8::Local; using v8::String; using v8::Value; template -static void MakeUtf8String(Isolate* isolate, +static void MakeUtf8String(Local context, Local value, T* target) { - Local string = value->ToString(isolate); - if (string.IsEmpty()) + Local string; + if (!value->ToString(context).ToLocal(&string)) { + target->Invalidate(); return; + } + auto isolate = context->GetIsolate(); const size_t storage = StringBytes::StorageSize(isolate, string, UTF8) + 1; target->AllocateSufficientStorage(storage); const int flags = @@ -48,22 +51,27 @@ static void MakeUtf8String(Isolate* isolate, target->SetLengthAndZeroTerminate(length); } -Utf8Value::Utf8Value(Isolate* isolate, Local value) { - if (value.IsEmpty()) +Utf8Value::Utf8Value(Local context, Local value) { + if (value.IsEmpty()) { + Invalidate(); return; + } - MakeUtf8String(isolate, value, this); + MakeUtf8String(context, value, this); } -TwoByteValue::TwoByteValue(Isolate* isolate, Local value) { +TwoByteValue::TwoByteValue(Local context, Local value) { if (value.IsEmpty()) { + Invalidate(); return; } - Local string = value->ToString(isolate); - if (string.IsEmpty()) + Local string; + if (!value->ToString(context).ToLocal(&string)) { + Invalidate(); return; + } // Allocate enough space to include the null terminator const size_t storage = string->Length() + 1; @@ -74,7 +82,7 @@ TwoByteValue::TwoByteValue(Isolate* isolate, Local value) { SetLengthAndZeroTerminate(length); } -BufferValue::BufferValue(Isolate* isolate, Local value) { +BufferValue::BufferValue(Local context, Local value) { // Slightly different take on Utf8Value. If value is a String, // it will return a Utf8 encoded string. If value is a Buffer, // it will copy the data out of the Buffer as is. @@ -85,7 +93,7 @@ BufferValue::BufferValue(Isolate* isolate, Local value) { } if (value->IsString()) { - MakeUtf8String(isolate, value, this); + MakeUtf8String(context, value, this); } else if (Buffer::HasInstance(value)) { const size_t len = Buffer::Length(value); // Leave place for the terminating '\0' byte. diff --git a/src/util.h b/src/util.h index f43ccef8b840..428e55a9bf1f 100644 --- a/src/util.h +++ b/src/util.h @@ -419,17 +419,29 @@ class MaybeStackBuffer { class Utf8Value : public MaybeStackBuffer { public: - explicit Utf8Value(v8::Isolate* isolate, v8::Local value); + explicit Utf8Value(v8::Local context, + v8::Local value); + explicit Utf8Value(v8::Isolate* isolate, v8::Local value) + : Utf8Value(isolate->GetCurrentContext(), value) { + } }; class TwoByteValue : public MaybeStackBuffer { public: - explicit TwoByteValue(v8::Isolate* isolate, v8::Local value); + explicit TwoByteValue(v8::Local context, + v8::Local value); + explicit TwoByteValue(v8::Isolate* isolate, v8::Local value) + : TwoByteValue(isolate->GetCurrentContext(), value) { + } }; class BufferValue : public MaybeStackBuffer { public: - explicit BufferValue(v8::Isolate* isolate, v8::Local value); + explicit BufferValue(v8::Local context, + v8::Local value); + explicit BufferValue(v8::Isolate* isolate, v8::Local value) + : BufferValue(isolate->GetCurrentContext(), value) { + } }; #define THROW_AND_RETURN_UNLESS_BUFFER(env, obj) \ diff --git a/test/parallel/test-child-process-spawnsync-validation-errors.js b/test/parallel/test-child-process-spawnsync-validation-errors.js index 8260e15512d1..0fb62ff3f7ed 100644 --- a/test/parallel/test-child-process-spawnsync-validation-errors.js +++ b/test/parallel/test-child-process-spawnsync-validation-errors.js @@ -200,3 +200,34 @@ if (!common.isWindows) { fail('killSignal', {}, typeErr); fail('killSignal', noop, typeErr); } + +{ + // Validate the args argument + const err = /^TypeError: Incorrect value of args option$/; + + function pass(value) { + // Run the command with the specified option. Since it's not a real command, + // spawnSync() should run successfully but return an ENOENT error. + const child = spawnSync('not_a_real_command', value); + assert.strictEqual(child.error.code, 'ENOENT'); + } + + function fail(value, message = err) { + assert.throws(() => { + spawnSync('not_a_real_command', value); + }, message); + } + + pass([]); + pass(['a']); + pass({}); // in which case the args argument is actually options + pass(undefined); + fail(null); + fail('a'); + fail(42); + fail(false); + fail(Symbol()); + fail([Symbol()], /^TypeError: Cannot convert a Symbol value to a string$/); + fail([{ toString() { throw new TypeError('stringifier'); } }], + /^TypeError: stringifier$/); +}