Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 317ae96

Browse filesBrowse files
fhinkelcjihrig
authored andcommitted
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>
1 parent cb0baca commit 317ae96
Copy full SHA for 317ae96

File tree

Expand file treeCollapse file tree

2 files changed

+24
-25
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+24
-25
lines changed
Open diff view settings
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+6-11Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2774,23 +2774,18 @@ static void EnvQuery(Local<String> property,
27742774

27752775
static void EnvDeleter(Local<String> property,
27762776
const PropertyCallbackInfo<Boolean>& info) {
2777-
bool rc = true;
27782777
#ifdef __POSIX__
27792778
node::Utf8Value key(info.GetIsolate(), property);
2780-
rc = getenv(*key) != nullptr;
2781-
if (rc)
2782-
unsetenv(*key);
2779+
unsetenv(*key);
27832780
#else
27842781
String::Value key(property);
27852782
WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
2786-
if (key_ptr[0] == L'=' || !SetEnvironmentVariableW(key_ptr, nullptr)) {
2787-
// Deletion failed. Return true if the key wasn't there in the first place,
2788-
// false if it is still there.
2789-
rc = GetEnvironmentVariableW(key_ptr, nullptr, 0) == 0 &&
2790-
GetLastError() != ERROR_SUCCESS;
2791-
}
2783+
SetEnvironmentVariableW(key_ptr, nullptr);
27922784
#endif
2793-
info.GetReturnValue().Set(rc);
2785+
2786+
// process.env never has non-configurable properties, so always
2787+
// return true like the tc39 delete operator.
2788+
info.GetReturnValue().Set(true);
27942789
}
27952790

27962791

Collapse file

‎test/parallel/test-process-env.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-process-env.js
+18-14Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,8 @@
22
/* eslint-disable max-len */
33

44
require('../common');
5-
// first things first, set the timezone; see tzset(3)
6-
process.env.TZ = 'Europe/Amsterdam';
7-
8-
var assert = require('assert');
9-
var spawn = require('child_process').spawn;
5+
const assert = require('assert');
6+
const spawn = require('child_process').spawn;
107

118
/* For the moment we are not going to support setting the timezone via the
129
* environment variables. The problem is that various V8 platform backends
@@ -16,6 +13,8 @@ var spawn = require('child_process').spawn;
1613
https://github.com/joyent/node/blob/08782931205bc4f6d28102ebc29fd806e8ccdf1f/deps/v8/src/platform-linux.cc#L339-345
1714
https://github.com/joyent/node/blob/08782931205bc4f6d28102ebc29fd806e8ccdf1f/deps/v8/src/platform-win32.cc#L590-596
1815
16+
// first things first, set the timezone; see tzset(3)
17+
process.env.TZ = 'Europe/Amsterdam';
1918
2019
// time difference between Greenwich and Amsterdam is +2 hours in the summer
2120
date = new Date('Fri, 10 Sep 1982 03:15:00 GMT');
@@ -27,28 +26,28 @@ assert.equal(5, date.getHours());
2726
// changes in environment should be visible to child processes
2827
if (process.argv[2] == 'you-are-the-child') {
2928
// failed assertion results in process exiting with status code 1
30-
assert.equal(false, 'NODE_PROCESS_ENV_DELETED' in process.env);
31-
assert.equal(42, process.env.NODE_PROCESS_ENV);
32-
assert.equal('asdf', process.env.hasOwnProperty);
29+
assert.strictEqual(false, 'NODE_PROCESS_ENV_DELETED' in process.env);
30+
assert.strictEqual('42', process.env.NODE_PROCESS_ENV);
31+
assert.strictEqual('asdf', process.env.hasOwnProperty);
3332
var hasOwnProperty = Object.prototype.hasOwnProperty;
3433
const has = hasOwnProperty.call(process.env, 'hasOwnProperty');
35-
assert.equal(true, has);
34+
assert.strictEqual(true, has);
3635
process.exit(0);
3736
} else {
38-
assert.equal(Object.prototype.hasOwnProperty, process.env.hasOwnProperty);
37+
assert.strictEqual(Object.prototype.hasOwnProperty, process.env.hasOwnProperty);
3938
const has = process.env.hasOwnProperty('hasOwnProperty');
40-
assert.equal(false, has);
39+
assert.strictEqual(false, has);
4140

4241
process.env.hasOwnProperty = 'asdf';
4342

4443
process.env.NODE_PROCESS_ENV = 42;
45-
assert.equal(42, process.env.NODE_PROCESS_ENV);
44+
assert.strictEqual('42', process.env.NODE_PROCESS_ENV);
4645

4746
process.env.NODE_PROCESS_ENV_DELETED = 42;
48-
assert.equal(true, 'NODE_PROCESS_ENV_DELETED' in process.env);
47+
assert.strictEqual(true, 'NODE_PROCESS_ENV_DELETED' in process.env);
4948

5049
delete process.env.NODE_PROCESS_ENV_DELETED;
51-
assert.equal(false, 'NODE_PROCESS_ENV_DELETED' in process.env);
50+
assert.strictEqual(false, 'NODE_PROCESS_ENV_DELETED' in process.env);
5251

5352
var child = spawn(process.argv[0], [process.argv[1], 'you-are-the-child']);
5453
child.stdout.on('data', function(data) { console.log(data.toString()); });
@@ -59,3 +58,8 @@ if (process.argv[2] == 'you-are-the-child') {
5958
}
6059
});
6160
}
61+
62+
// delete should return true except for non-configurable properties
63+
// https://github.com/nodejs/node/issues/7960
64+
delete process.env.NON_EXISTING_VARIABLE;
65+
assert.strictEqual(true, delete process.env.NON_EXISTING_VARIABLE);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.