From 61cc5141b08dc23f84bf600fe1aff3c19ca064fe Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 15 Aug 2026 23:39:19 +0000 Subject: [PATCH] child_process: build the default env block in one native pass When spawn()/spawnSync() are called without options.env, normalizeSpawnArguments() copied process.env with a spread and then walked the copy to build the KEY=value array uv_spawn() takes. Spreading the process.env proxy costs one enumerator callback plus a query and a getter interceptor per variable, each doing a linear getenv() scan and allocating; with a couple of hundred variables that was the single largest JS-side cost of spawning a process. Add KVStore::Pairs() (Enumerate() + Get() by default, one uv_os_environ() pass for the real environment, skipping hidden variables on Windows exactly like Enumerate() does), expose it as process_wrap.getEnvPairs(), and use it for the default-environment case. A user supplied options.env and the permission model case keep the existing code. On Windows the same sort/first-wins-case-insensitive filter is applied to the pairs. The variables copyProcessEnvToEnv() propagates are part of the real environment by definition, and its entries cannot contain null bytes, so those steps only remain on the options.env path. Signed-off-by: Shelley Vohr --- lib/child_process.js | 145 ++++++++++++------ src/node_env_var.cc | 60 ++++++++ src/process_wrap.cc | 13 ++ src/util.h | 4 + .../test-child-process-default-env.js | 76 +++++++++ 5 files changed, 248 insertions(+), 50 deletions(-) create mode 100644 test/parallel/test-child-process-default-env.js diff --git a/lib/child_process.js b/lib/child_process.js index 1fdc520d31e8..32317b6621c4 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -40,6 +40,7 @@ const { RegExpPrototypeExec, SafeSet, StringPrototypeIncludes, + StringPrototypeIndexOf, StringPrototypeSlice, StringPrototypeToUpperCase, SymbolDispose, @@ -93,6 +94,8 @@ const { stdioStringToArray, } = child_process; +const { getEnvPairs } = internalBinding('process_wrap'); + const MAX_BUFFER = 1024 * 1024; const permission = require('internal/process/permission'); @@ -697,60 +700,74 @@ function normalizeSpawnArguments(file, args, options) { ArrayPrototypeUnshift(args, file); } - // Shallow copy to guarantee changes won't impact process.env - const env = options.env || { ...process.env }; - const envPairs = []; - - // process.env.NODE_V8_COVERAGE always propagates, making it possible to - // collect coverage for programs that spawn with white-listed environment. - copyProcessEnvToEnv(env, 'NODE_V8_COVERAGE', options.env); - - if (isZOS) { - // The following environment variables must always propagate if set. - copyProcessEnvToEnv(env, '_BPXK_AUTOCVT', options.env); - copyProcessEnvToEnv(env, '_CEE_RUNOPTS', options.env); - copyProcessEnvToEnv(env, '_TAG_REDIR_ERR', options.env); - copyProcessEnvToEnv(env, '_TAG_REDIR_IN', options.env); - copyProcessEnvToEnv(env, '_TAG_REDIR_OUT', options.env); - copyProcessEnvToEnv(env, 'STEPLIB', options.env); - copyProcessEnvToEnv(env, 'LIBPATH', options.env); - copyProcessEnvToEnv(env, '_EDC_SIG_DFLT', options.env); - copyProcessEnvToEnv(env, '_EDC_SUSV3', options.env); - } + let envPairs; + if (!options.env && !permission.isEnabled()) { + // Default: the child inherits this process's environment. Take it as + // 'KEY=value' strings in one native pass over the environment block + // instead of copying process.env (one interceptor round trip and one + // getenv() scan per variable). Everything copyProcessEnvToEnv() would + // propagate below is part of it by definition, and entries of the real + // environment block cannot contain null bytes. + envPairs = getEnvPairs(); + if (process.platform === 'win32') { + envPairs = dedupeWindowsEnvPairs(envPairs); + } + } else { + // Shallow copy to guarantee changes won't impact process.env + const env = options.env || { ...process.env }; + envPairs = []; + + // process.env.NODE_V8_COVERAGE always propagates, making it possible to + // collect coverage for programs that spawn with white-listed environment. + copyProcessEnvToEnv(env, 'NODE_V8_COVERAGE', options.env); + + if (isZOS) { + // The following environment variables must always propagate if set. + copyProcessEnvToEnv(env, '_BPXK_AUTOCVT', options.env); + copyProcessEnvToEnv(env, '_CEE_RUNOPTS', options.env); + copyProcessEnvToEnv(env, '_TAG_REDIR_ERR', options.env); + copyProcessEnvToEnv(env, '_TAG_REDIR_IN', options.env); + copyProcessEnvToEnv(env, '_TAG_REDIR_OUT', options.env); + copyProcessEnvToEnv(env, 'STEPLIB', options.env); + copyProcessEnvToEnv(env, 'LIBPATH', options.env); + copyProcessEnvToEnv(env, '_EDC_SIG_DFLT', options.env); + copyProcessEnvToEnv(env, '_EDC_SUSV3', options.env); + } - if (permission.isEnabled()) { - copyPermissionModelFlagsToEnv(env, 'NODE_OPTIONS', args); - } + if (permission.isEnabled()) { + copyPermissionModelFlagsToEnv(env, 'NODE_OPTIONS', args); + } - let envKeys = []; - // Prototype values are intentionally included. - for (const key in env) { - ArrayPrototypePush(envKeys, key); - } + let envKeys = []; + // Prototype values are intentionally included. + for (const key in env) { + ArrayPrototypePush(envKeys, key); + } - if (process.platform === 'win32') { - // On Windows env keys are case insensitive. Filter out duplicates, - // keeping only the first one (in lexicographic order) - const sawKey = new SafeSet(); - envKeys = ArrayPrototypeFilter( - ArrayPrototypeSort(envKeys), - (key) => { - const uppercaseKey = StringPrototypeToUpperCase(key); - if (sawKey.has(uppercaseKey)) { - return false; - } - sawKey.add(uppercaseKey); - return true; - }, - ); - } + if (process.platform === 'win32') { + // On Windows env keys are case insensitive. Filter out duplicates, + // keeping only the first one (in lexicographic order) + const sawKey = new SafeSet(); + envKeys = ArrayPrototypeFilter( + ArrayPrototypeSort(envKeys), + (key) => { + const uppercaseKey = StringPrototypeToUpperCase(key); + if (sawKey.has(uppercaseKey)) { + return false; + } + sawKey.add(uppercaseKey); + return true; + }, + ); + } - for (const key of envKeys) { - const value = env[key]; - if (value !== undefined) { - validateArgumentNullCheck(key, `options.env['${key}']`); - validateArgumentNullCheck(value, `options.env['${key}']`); - ArrayPrototypePush(envPairs, `${key}=${value}`); + for (const key of envKeys) { + const value = env[key]; + if (value !== undefined) { + validateArgumentNullCheck(key, `options.env['${key}']`); + validateArgumentNullCheck(value, `options.env['${key}']`); + ArrayPrototypePush(envPairs, `${key}=${value}`); + } } } @@ -768,6 +785,34 @@ function normalizeSpawnArguments(file, args, options) { }; } +/** + * Windows environment variable names are case-insensitive: keep only the + * first entry for each name in lexicographic order of the names, exactly like + * the key filtering applied to a user-supplied `options.env`. + * @param {string[]} envPairs 'KEY=value' strings + * @returns {string[]} + */ +function dedupeWindowsEnvPairs(envPairs) { + const keyed = []; + for (let i = 0; i < envPairs.length; i++) { + const pair = envPairs[i]; + // Names never start with '=' here (hidden variables are not enumerated), + // so the first '=' ends the name. + const eq = StringPrototypeIndexOf(pair, '='); + ArrayPrototypePush(keyed, { key: eq === -1 ? pair : StringPrototypeSlice(pair, 0, eq), pair }); + } + ArrayPrototypeSort(keyed, (a, b) => (a.key < b.key ? -1 : a.key > b.key ? 1 : 0)); + const sawKey = new SafeSet(); + const result = []; + for (let i = 0; i < keyed.length; i++) { + const uppercaseKey = StringPrototypeToUpperCase(keyed[i].key); + if (sawKey.has(uppercaseKey)) continue; + sawKey.add(uppercaseKey); + ArrayPrototypePush(result, keyed[i].pair); + } + return result; +} + function abortChildProcess(child, killSignal, reason) { if (!child) return; diff --git a/src/node_env_var.cc b/src/node_env_var.cc index 25e405be86d2..8a16f9ec136d 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -15,6 +15,7 @@ using v8::Boolean; using v8::Context; using v8::DontDelete; using v8::DontEnum; +using v8::EscapableHandleScope; using v8::FunctionTemplate; using v8::HandleScope; using v8::IndexedPropertyHandlerConfiguration; @@ -47,6 +48,7 @@ class RealEnvStore final : public KVStore { int32_t Query(const char* key) const override; void Delete(Isolate* isolate, Local key) override; MaybeLocal Enumerate(Isolate* isolate) const override; + MaybeLocal Pairs(Isolate* isolate) const override; }; class MapKVStore final : public KVStore { @@ -222,6 +224,64 @@ MaybeLocal RealEnvStore::Enumerate(Isolate* isolate) const { return env_v.ToArray(); } +MaybeLocal RealEnvStore::Pairs(Isolate* isolate) const { + Mutex::ScopedLock lock(per_process::env_var_mutex); + uv_env_item_t* items; + int count; + + auto cleanup = OnScopeLeave([&]() { uv_os_free_environ(items, count); }); + CHECK_EQ(uv_os_environ(&items, &count), 0); + + MaybeStackBuffer, 256> pairs_v(count); + int pairs_v_index = 0; + std::string pair; + for (int i = 0; i < count; i++) { +#ifdef _WIN32 + // If the key starts with '=' it is a hidden environment variable. + // Enumerate() skips these, so a copy of process.env never had them. + if (items[i].name[0] == '=') continue; +#endif + pair.assign(items[i].name); + pair += '='; + pair += items[i].value; + Local str; + if (!ToV8Value(isolate->GetCurrentContext(), pair, isolate).ToLocal(&str)) { + return {}; + } + pairs_v[pairs_v_index++] = str; + } + + return Array::New(isolate, pairs_v.out(), pairs_v_index); +} + +MaybeLocal KVStore::Pairs(Isolate* isolate) const { + EscapableHandleScope scope(isolate); + Local context = isolate->GetCurrentContext(); + Local keys; + if (!Enumerate(isolate).ToLocal(&keys)) return {}; + uint32_t keys_length = keys->Length(); + MaybeStackBuffer, 256> pairs(keys_length); + uint32_t pairs_index = 0; + std::string pair; + for (uint32_t i = 0; i < keys_length; i++) { + Local key; + if (!keys->Get(context, i).ToLocal(&key)) return {}; + if (!key->IsString()) continue; + Utf8Value key_utf8(isolate, key); + // A key that disappeared between Enumerate() and Get() is skipped, like an + // undefined value is when copying process.env in JS. + std::optional value = Get(*key_utf8); + if (!value.has_value()) continue; + pair.assign(*key_utf8, key_utf8.length()); + pair += '='; + pair += *value; + Local str; + if (!ToV8Value(context, pair, isolate).ToLocal(&str)) return {}; + pairs[pairs_index++] = str; + } + return scope.Escape(Array::New(isolate, pairs.out(), pairs_index)); +} + std::shared_ptr KVStore::Clone(Isolate* isolate) const { HandleScope handle_scope(isolate); Local context = isolate->GetCurrentContext(); diff --git a/src/process_wrap.cc b/src/process_wrap.cc index 1277581784d8..8ca42a784dee 100644 --- a/src/process_wrap.cc +++ b/src/process_wrap.cc @@ -79,6 +79,7 @@ class ProcessWrap : public HandleWrap { SetProtoMethod(isolate, constructor, "kill", Kill); SetConstructorFunction(context, target, "Process", constructor); + SetMethodNoSideEffect(context, target, "getEnvPairs", GetEnvPairs); Local constants = Object::New(isolate); NODE_DEFINE_CONSTANT(constants, kProcessFlagDetached); @@ -91,6 +92,7 @@ class ProcessWrap : public HandleWrap { registry->Register(New); registry->Register(Spawn); registry->Register(Kill); + registry->Register(GetEnvPairs); } SET_NO_MEMORY_INFO() @@ -342,6 +344,17 @@ class ProcessWrap : public HandleWrap { args.GetReturnValue().Set(err); } + // The current environment as ["KEY=value", ...], i.e. what a spawned + // child inherits by default, produced in one pass over the environment + // block instead of one interceptor round trip per variable. + static void GetEnvPairs(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Local pairs; + if (env->env_vars()->Pairs(env->isolate()).ToLocal(&pairs)) { + args.GetReturnValue().Set(pairs); + } + } + static void Kill(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); ProcessWrap* wrap; diff --git a/src/util.h b/src/util.h index ec7fbad43798..6cb2d5e8900e 100644 --- a/src/util.h +++ b/src/util.h @@ -323,6 +323,10 @@ class KVStore { virtual int32_t Query(const char* key) const = 0; virtual void Delete(v8::Isolate* isolate, v8::Local key) = 0; virtual v8::MaybeLocal Enumerate(v8::Isolate* isolate) const = 0; + // All entries as an array of "KEY=value" strings, in enumeration order — + // the form uv_spawn() consumes. The default implementation is + // Enumerate() + Get(); stores that can produce it in one pass override it. + virtual v8::MaybeLocal Pairs(v8::Isolate* isolate) const; virtual std::shared_ptr Clone(v8::Isolate* isolate) const; virtual v8::Maybe AssignFromObject(v8::Local context, diff --git a/test/parallel/test-child-process-default-env.js b/test/parallel/test-child-process-default-env.js new file mode 100644 index 000000000000..9d51ea337bb3 --- /dev/null +++ b/test/parallel/test-child-process-default-env.js @@ -0,0 +1,76 @@ +'use strict'; +// When no `env` option is given, a child process must inherit exactly the +// parent's current environment: same variables and values, reflecting runtime +// additions/deletions made through process.env. +const common = require('../common'); +const assert = require('assert'); +const { spawn, spawnSync, execFileSync } = require('child_process'); + +// Mutate the environment at runtime in a few ways first. +process.env.TEST_DEFAULT_ENV_ADDED = 'added ünïcödé ✓'; +process.env.TEST_DEFAULT_ENV_EMPTY = ''; +process.env.TEST_DEFAULT_ENV_EQUALS = 'a=b=c'; +process.env.TEST_DEFAULT_ENV_DELETED = 'x'; +delete process.env.TEST_DEFAULT_ENV_DELETED; + +function expectedEnv() { + // What `{ ...process.env }` yields, minus keys whose value is undefined + // (there are none for the real environment, but keep the definition exact). + const copy = { ...process.env }; + for (const key of Object.keys(copy)) { + if (copy[key] === undefined) delete copy[key]; + } + return copy; +} + +const printEnv = ['-e', 'process.stdout.write(JSON.stringify(process.env))']; + +function run(options) { + const child = spawnSync(process.execPath, printEnv, { encoding: 'utf8', ...options }); + assert.strictEqual(child.status, 0, child.stderr || String(child.error)); + return child.stdout; +} + +function check(output, label, expected = expectedEnv()) { + const childEnv = JSON.parse(output); + assert.deepStrictEqual(childEnv, expected, `${label}: contents`); + assert.strictEqual(childEnv.TEST_DEFAULT_ENV_ADDED, 'added ünïcödé ✓'); + assert.strictEqual(childEnv.TEST_DEFAULT_ENV_EMPTY, ''); + assert.strictEqual(childEnv.TEST_DEFAULT_ENV_EQUALS, 'a=b=c'); + assert.ok(!('TEST_DEFAULT_ENV_DELETED' in childEnv)); +} + +// spawnSync, options omitted entirely. +check(run(), 'spawnSync no options'); +// Explicitly undefined / null env behave like the default. +check(run({ env: undefined }), 'spawnSync env undefined'); +check(run({ env: null }), 'spawnSync env null'); +// execFileSync goes through the same normalization. +check(execFileSync(process.execPath, printEnv, { encoding: 'utf8' }), 'execFileSync'); +// A user-supplied env is still passed through as given (not merged). +{ + const env = { ...process.env, ONLY: 'this' }; + delete env.TEST_DEFAULT_ENV_ADDED; + const childEnv = JSON.parse(run({ env })); + assert.strictEqual(childEnv.ONLY, 'this'); + assert.ok(!('TEST_DEFAULT_ENV_ADDED' in childEnv)); +} +// Async spawn (the environment is captured at spawn() time). +{ + const expectedAtSpawn = expectedEnv(); + const child = spawn(process.execPath, printEnv); + let out = ''; + child.stdout.setEncoding('utf8').on('data', (d) => { out += d; }); + let err = ''; + child.stderr.setEncoding('utf8').on('data', (d) => { err += d; }); + child.on('close', common.mustCall((code) => { + assert.strictEqual(code, 0, err); + check(out, 'spawn', expectedAtSpawn); + })); +} +// A variable added after an earlier spawn is seen by a later one (no caching). +process.env.TEST_DEFAULT_ENV_LATE = 'late'; +{ + const childEnv = JSON.parse(run()); + assert.strictEqual(childEnv.TEST_DEFAULT_ENV_LATE, 'late'); +}