1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src: make EnvDelete behave like the delete operator

Browse Source 
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: https://github.com/nodejs/node/issues/7960
PR-URL: https://github.com/nodejs/node/pull/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>
This commit is contained in:
Franziska Hinkelmann 2016-08-04 09:36:50 +02:00 committed by James M Snell
parent cc3a9e7947
commit ff7a841f4d
2 changed files with 24 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/node.cc
View File

@ -2720,23 +2720,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);
}

32
test/parallel/test-process-env.js
View File

@ -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);