Skip to content

Commit

Permalink
src: fix empty-named env var assertion failure
Browse files Browse the repository at this point in the history
Setting an environment variable with an empty name on Windows resulted
in an assertion failure, because it was checked for an '=' sign at the
beginning without verifying the length was greater than 0.

Fixes: #32920
Refs: #27310

PR-URL: #32921
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
  • Loading branch information
bl-ue authored and targos committed May 13, 2020
1 parent 9ce6e36 commit 60db9af
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/node_env_var.cc
Expand Up @@ -106,7 +106,7 @@ void RealEnvStore::Set(Isolate* isolate,
node::Utf8Value val(isolate, value);

#ifdef _WIN32
if (key[0] == L'=') return;
if (key.length() > 0 && key[0] == L'=') return;
#endif
uv_os_setenv(*key, *val);
}
Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-process-env.js
Expand Up @@ -106,3 +106,11 @@ if (common.isWindows) {
const keys = Object.keys(process.env);
assert.ok(keys.length > 0);
}

// Setting environment variables on Windows with empty names should not cause
// an assertion failure.
// https://github.com/nodejs/node/issues/32920
{
process.env[''] = '';
assert.strictEqual(process.env[''], undefined);
}

0 comments on commit 60db9af

Please sign in to comment.