src: use RAII for uv_process_options_t

PR-URL: https://github.com/nodejs/node/pull/59945
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit is contained in:
iknoom 2025-09-20 15:33:45 +09:00 committed by Node.js GitHub Bot
parent cff138c6b1
commit 3e1adceca4
3 changed files with 64 additions and 73 deletions

View File

@ -116,9 +116,10 @@ class ProcessWrap : public HandleWrap {
return Just(stream);
}
static Maybe<void> ParseStdioOptions(Environment* env,
Local<Object> js_options,
uv_process_options_t* options) {
static Maybe<void> ParseStdioOptions(
Environment* env,
Local<Object> js_options,
std::vector<uv_stdio_container_t>* options_stdio) {
Local<Context> context = env->context();
Local<String> stdio_key = env->stdio_string();
Local<Value> stdios_val;
@ -132,8 +133,7 @@ class ProcessWrap : public HandleWrap {
Local<Array> stdios = stdios_val.As<Array>();
uint32_t len = stdios->Length();
options->stdio = new uv_stdio_container_t[len];
options->stdio_count = len;
options_stdio->resize(len);
for (uint32_t i = 0; i < len; i++) {
Local<Value> val;
@ -147,23 +147,23 @@ class ProcessWrap : public HandleWrap {
}
if (type->StrictEquals(env->ignore_string())) {
options->stdio[i].flags = UV_IGNORE;
(*options_stdio)[i].flags = UV_IGNORE;
} else if (type->StrictEquals(env->pipe_string())) {
options->stdio[i].flags = static_cast<uv_stdio_flags>(
(*options_stdio)[i].flags = static_cast<uv_stdio_flags>(
UV_CREATE_PIPE | UV_READABLE_PIPE | UV_WRITABLE_PIPE);
if (!StreamForWrap(env, stdio).To(&options->stdio[i].data.stream)) {
if (!StreamForWrap(env, stdio).To(&(*options_stdio)[i].data.stream)) {
return Nothing<void>();
}
} else if (type->StrictEquals(env->overlapped_string())) {
options->stdio[i].flags = static_cast<uv_stdio_flags>(
UV_CREATE_PIPE | UV_READABLE_PIPE | UV_WRITABLE_PIPE |
UV_OVERLAPPED_PIPE);
if (!StreamForWrap(env, stdio).To(&options->stdio[i].data.stream)) {
(*options_stdio)[i].flags =
static_cast<uv_stdio_flags>(UV_CREATE_PIPE | UV_READABLE_PIPE |
UV_WRITABLE_PIPE | UV_OVERLAPPED_PIPE);
if (!StreamForWrap(env, stdio).To(&(*options_stdio)[i].data.stream)) {
return Nothing<void>();
}
} else if (type->StrictEquals(env->wrap_string())) {
options->stdio[i].flags = UV_INHERIT_STREAM;
if (!StreamForWrap(env, stdio).To(&options->stdio[i].data.stream)) {
(*options_stdio)[i].flags = UV_INHERIT_STREAM;
if (!StreamForWrap(env, stdio).To(&(*options_stdio)[i].data.stream)) {
return Nothing<void>();
}
} else {
@ -174,8 +174,8 @@ class ProcessWrap : public HandleWrap {
}
CHECK(fd_value->IsNumber());
int fd = FromV8Value<int>(fd_value);
options->stdio[i].flags = UV_INHERIT_FD;
options->stdio[i].data.fd = fd;
(*options_stdio)[i].flags = UV_INHERIT_FD;
(*options_stdio)[i].data.fd = fd;
}
}
return JustVoid();
@ -199,8 +199,6 @@ class ProcessWrap : public HandleWrap {
options.exit_cb = OnExit;
// TODO(bnoordhuis) is this possible to do without mallocing ?
// options.file
Local<Value> file_v;
if (!js_options->Get(context, env->file_string()).ToLocal(&file_v)) {
@ -251,23 +249,26 @@ class ProcessWrap : public HandleWrap {
if (!js_options->Get(context, env->args_string()).ToLocal(&argv_v)) {
return;
}
if (!argv_v.IsEmpty() && argv_v->IsArray()) {
std::vector<char*> options_args;
std::vector<std::string> args_vals;
if (argv_v->IsArray()) {
Local<Array> js_argv = argv_v.As<Array>();
int argc = js_argv->Length();
CHECK_LT(argc, INT_MAX); // Check for overflow.
// Heap allocate to detect errors. +1 is for nullptr.
options.args = new char*[argc + 1];
args_vals.reserve(argc);
options_args.resize(argc + 1);
for (int i = 0; i < argc; i++) {
Local<Value> val;
if (!js_argv->Get(context, i).ToLocal(&val)) {
return;
}
node::Utf8Value arg(env->isolate(), val);
options.args[i] = strdup(*arg);
CHECK_NOT_NULL(options.args[i]);
args_vals.emplace_back(arg.ToString());
options_args[i] = const_cast<char*>(args_vals.back().c_str());
CHECK_NOT_NULL(options_args[i]);
}
options.args[argc] = nullptr;
options_args[argc] = nullptr;
options.args = options_args.data();
}
// options.cwd
@ -286,27 +287,35 @@ class ProcessWrap : public HandleWrap {
if (!js_options->Get(context, env->env_pairs_string()).ToLocal(&env_v)) {
return;
}
if (!env_v.IsEmpty() && env_v->IsArray()) {
std::vector<char*> options_env;
std::vector<std::string> env_vals;
if (env_v->IsArray()) {
Local<Array> env_opt = env_v.As<Array>();
int envc = env_opt->Length();
CHECK_LT(envc, INT_MAX); // Check for overflow.
options.env = new char*[envc + 1]; // Heap allocated to detect errors.
env_vals.reserve(envc);
options_env.resize(envc + 1);
for (int i = 0; i < envc; i++) {
Local<Value> val;
if (!env_opt->Get(context, i).ToLocal(&val)) {
return;
}
node::Utf8Value pair(env->isolate(), val);
options.env[i] = strdup(*pair);
CHECK_NOT_NULL(options.env[i]);
env_vals.emplace_back(pair.ToString());
options_env[i] = const_cast<char*>(env_vals.back().c_str());
CHECK_NOT_NULL(options_env[i]);
}
options.env[envc] = nullptr;
options_env[envc] = nullptr;
options.env = options_env.data();
}
// options.stdio
if (ParseStdioOptions(env, js_options, &options).IsNothing()) {
std::vector<uv_stdio_container_t> options_stdio;
if (ParseStdioOptions(env, js_options, &options_stdio).IsNothing()) {
return;
}
options.stdio = options_stdio.data();
options.stdio_count = options_stdio.size();
// options.windowsHide
Local<Value> hide_v;
@ -361,18 +370,6 @@ class ProcessWrap : public HandleWrap {
}
}
if (options.args) {
for (int i = 0; options.args[i]; i++) free(options.args[i]);
delete [] options.args;
}
if (options.env) {
for (int i = 0; options.env[i]; i++) free(options.env[i]);
delete [] options.env;
}
delete[] options.stdio;
args.GetReturnValue().Set(err);
}

View File

@ -403,7 +403,6 @@ void SyncProcessRunner::Spawn(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(result);
}
SyncProcessRunner::SyncProcessRunner(Environment* env)
: max_buffer_(0),
timeout_(0),
@ -412,14 +411,9 @@ SyncProcessRunner::SyncProcessRunner(Environment* env)
uv_loop_(nullptr),
stdio_count_(0),
uv_stdio_containers_(nullptr),
stdio_pipes_initialized_(false),
uv_process_options_(),
file_buffer_(nullptr),
args_buffer_(nullptr),
env_buffer_(nullptr),
cwd_buffer_(nullptr),
uv_process_(),
killed_(false),
@ -436,19 +430,12 @@ SyncProcessRunner::SyncProcessRunner(Environment* env)
lifecycle_(kUninitialized),
env_(env) {
}
env_(env) {}
SyncProcessRunner::~SyncProcessRunner() {
CHECK_EQ(lifecycle_, kHandlesClosed);
stdio_pipes_.clear();
delete[] file_buffer_;
delete[] args_buffer_;
delete[] cwd_buffer_;
delete[] env_buffer_;
delete[] uv_stdio_containers_;
}
@ -808,12 +795,14 @@ Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
Local<Object> js_options = js_value.As<Object>();
Local<Value> js_file;
const char* file_buffer;
if (!js_options->Get(context, env()->file_string()).ToLocal(&js_file) ||
!CopyJsString(js_file, &file_buffer_).To(&r)) {
!CopyJsString(js_file, &file_buffer).To(&r)) {
return Nothing<int>();
}
if (r < 0) return Just(r);
uv_process_options_.file = file_buffer_;
file_buffer_.reset(file_buffer);
uv_process_options_.file = file_buffer_.get();
// Undocumented feature of Win32 CreateProcess API allows spawning
// batch files directly but is potentially insecure because arguments
@ -825,23 +814,27 @@ Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
#endif
Local<Value> js_args;
char* args_buffer;
if (!js_options->Get(context, env()->args_string()).ToLocal(&js_args) ||
!CopyJsStringArray(js_args, &args_buffer_).To(&r)) {
!CopyJsStringArray(js_args, &args_buffer).To(&r)) {
return Nothing<int>();
}
if (r < 0) return Just(r);
uv_process_options_.args = reinterpret_cast<char**>(args_buffer_);
args_buffer_.reset(args_buffer);
uv_process_options_.args = reinterpret_cast<char**>(args_buffer_.get());
Local<Value> js_cwd;
if (!js_options->Get(context, env()->cwd_string()).ToLocal(&js_cwd)) {
return Nothing<int>();
}
if (!js_cwd->IsNullOrUndefined()) {
if (!CopyJsString(js_cwd, &cwd_buffer_).To(&r)) {
const char* cwd_buffer;
if (!CopyJsString(js_cwd, &cwd_buffer).To(&r)) {
return Nothing<int>();
}
if (r < 0) return Just(r);
uv_process_options_.cwd = cwd_buffer_;
cwd_buffer_.reset(cwd_buffer);
uv_process_options_.cwd = cwd_buffer_.get();
}
Local<Value> js_env_pairs;
@ -850,12 +843,13 @@ Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
return Nothing<int>();
}
if (!js_env_pairs->IsNullOrUndefined()) {
if (!CopyJsStringArray(js_env_pairs, &env_buffer_).To(&r)) {
char* env_buffer;
if (!CopyJsStringArray(js_env_pairs, &env_buffer).To(&r)) {
return Nothing<int>();
}
if (r < 0) return Just(r);
uv_process_options_.env = reinterpret_cast<char**>(env_buffer_);
env_buffer_.reset(env_buffer);
uv_process_options_.env = reinterpret_cast<char**>(env_buffer_.get());
}
Local<Value> js_uid;
if (!js_options->Get(context, env()->uid_string()).ToLocal(&js_uid)) {
@ -982,7 +976,7 @@ Maybe<int> SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {
js_stdio_options = js_value.As<Array>();
stdio_count_ = js_stdio_options->Length();
uv_stdio_containers_ = new uv_stdio_container_t[stdio_count_];
uv_stdio_containers_.resize(stdio_count_);
stdio_pipes_.clear();
stdio_pipes_.resize(stdio_count_);
@ -1007,7 +1001,7 @@ Maybe<int> SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {
}
}
uv_process_options_.stdio = uv_stdio_containers_;
uv_process_options_.stdio = uv_stdio_containers_.data();
uv_process_options_.stdio_count = stdio_count_;
return Just<int>(0);

View File

@ -205,15 +205,15 @@ class SyncProcessRunner {
uv_loop_t* uv_loop_;
uint32_t stdio_count_;
uv_stdio_container_t* uv_stdio_containers_;
std::vector<uv_stdio_container_t> uv_stdio_containers_;
std::vector<std::unique_ptr<SyncProcessStdioPipe>> stdio_pipes_;
bool stdio_pipes_initialized_;
uv_process_options_t uv_process_options_;
const char* file_buffer_;
char* args_buffer_;
char* env_buffer_;
const char* cwd_buffer_;
std::unique_ptr<const char[]> file_buffer_;
std::unique_ptr<char[]> args_buffer_;
std::unique_ptr<char[]> env_buffer_;
std::unique_ptr<const char[]> cwd_buffer_;
uv_process_t uv_process_;
bool killed_;