src: make EnvDelete behave like the delete operator

According to TC39 specification, the delete operator returns false or throws in strict mode, if the property is non-configurable. It returns true in all other cases. Process.env can never have non-configurable properties, thus EnvDelete must always return true. This is independent of strict mode. Fixes: #7960 PR-URL: #7975 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Author: fhinkel

src/node.cc

@@ -2774,23 +2774,18 @@ static void EnvQuery(Local<String> property,
 
 static void EnvDeleter(Local<String> property,
  const PropertyCallbackInfo<Boolean>& info) {
- bool rc = true;
 #ifdef __POSIX__
  node::Utf8Value key(info.GetIsolate(), property);
- rc = getenv(*key) != nullptr;
- if (rc)
- unsetenv(*key);
+ unsetenv(*key);
 #else
  String::Value key(property);
  WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
- if (key_ptr[0] == L'=' || !SetEnvironmentVariableW(key_ptr, nullptr)) {
- // Deletion failed. Return true if the key wasn't there in the first place,
- // false if it is still there.
- rc = GetEnvironmentVariableW(key_ptr, nullptr, 0) == 0 &&
- GetLastError() != ERROR_SUCCESS;
- }
+ SetEnvironmentVariableW(key_ptr, nullptr);
 #endif
- info.GetReturnValue().Set(rc);
+
+ // process.env never has non-configurable properties, so always
+ // return true like the tc39 delete operator.
+ info.GetReturnValue().Set(true);
 }
 
 

test/parallel/test-process-env.js

@@ -2,11 +2,8 @@
 /* eslint-disable max-len */
 
 require('../common');
-// first things first, set the timezone; see tzset(3)
-process.env.TZ = 'Europe/Amsterdam';
-
-var assert = require('assert');
-var spawn = require('child_process').spawn;
+const assert = require('assert');
+const spawn = require('child_process').spawn;
 
 /* For the moment we are not going to support setting the timezone via the
  * environment variables. The problem is that various V8 platform backends
@@ -16,6 +13,8 @@ var spawn = require('child_process').spawn;
 https://github.com/joyent/node/blob/08782931205bc4f6d28102ebc29fd806e8ccdf1f/deps/v8/src/platform-linux.cc#L339-345
 https://github.com/joyent/node/blob/08782931205bc4f6d28102ebc29fd806e8ccdf1f/deps/v8/src/platform-win32.cc#L590-596
 
+// first things first, set the timezone; see tzset(3)
+process.env.TZ = 'Europe/Amsterdam';
 
 // time difference between Greenwich and Amsterdam is +2 hours in the summer
 date = new Date('Fri, 10 Sep 1982 03:15:00 GMT');
@@ -27,28 +26,28 @@ assert.equal(5, date.getHours());
 // changes in environment should be visible to child processes
 if (process.argv[2] == 'you-are-the-child') {
  // failed assertion results in process exiting with status code 1
- assert.equal(false, 'NODE_PROCESS_ENV_DELETED' in process.env);
- assert.equal(42, process.env.NODE_PROCESS_ENV);
- assert.equal('asdf', process.env.hasOwnProperty);
+ assert.strictEqual(false, 'NODE_PROCESS_ENV_DELETED' in process.env);
+ assert.strictEqual('42', process.env.NODE_PROCESS_ENV);
+ assert.strictEqual('asdf', process.env.hasOwnProperty);
  var hasOwnProperty = Object.prototype.hasOwnProperty;
  const has = hasOwnProperty.call(process.env, 'hasOwnProperty');
- assert.equal(true, has);
+ assert.strictEqual(true, has);
  process.exit(0);
 } else {
- assert.equal(Object.prototype.hasOwnProperty, process.env.hasOwnProperty);
+ assert.strictEqual(Object.prototype.hasOwnProperty, process.env.hasOwnProperty);
  const has = process.env.hasOwnProperty('hasOwnProperty');
- assert.equal(false, has);
+ assert.strictEqual(false, has);
 
  process.env.hasOwnProperty = 'asdf';
 
  process.env.NODE_PROCESS_ENV = 42;
- assert.equal(42, process.env.NODE_PROCESS_ENV);
+ assert.strictEqual('42', process.env.NODE_PROCESS_ENV);
 
  process.env.NODE_PROCESS_ENV_DELETED = 42;
- assert.equal(true, 'NODE_PROCESS_ENV_DELETED' in process.env);
+ assert.strictEqual(true, 'NODE_PROCESS_ENV_DELETED' in process.env);
 
  delete process.env.NODE_PROCESS_ENV_DELETED;
- assert.equal(false, 'NODE_PROCESS_ENV_DELETED' in process.env);
+ assert.strictEqual(false, 'NODE_PROCESS_ENV_DELETED' in process.env);
 
  var child = spawn(process.argv[0], [process.argv[1], 'you-are-the-child']);
  child.stdout.on('data', function(data) { console.log(data.toString()); });
@@ -59,3 +58,8 @@ if (process.argv[2] == 'you-are-the-child') {
  }
  });
 }
+
+// delete should return true except for non-configurable properties
+// https://github.com/nodejs/node/issues/7960
+delete process.env.NON_EXISTING_VARIABLE;
+assert.strictEqual(true, delete process.env.NON_EXISTING_VARIABLE);