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 <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
  • Loading branch information
bl-ue authored and BethGriggs committed Apr 28, 2020
1 parent 03fce45 commit 23aba12
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
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,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);
DateTimeConfigurationChangeNotification(isolate, key);
Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-process-env.js
Original file line number Diff line number Diff line change
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 23aba12

Please sign in to comment.