X Tutup
The Wayback Machine - https://web.archive.org/web/20210814040306/https://github.com/nodejs/node/pull/36660/files
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process: disallow adding options to process.allowedNodeEnvironmentFlags #36660

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter
Filter file types
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.

Always

Just for now

@@ -6,21 +6,27 @@

const {
ArrayPrototypeEvery,
ArrayPrototypeForEach,
ArrayPrototypeIncludes,
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypeSplice,
BigUint64Array,
Float64Array,
NumberMAX_SAFE_INTEGER,
ObjectDefineProperty,
ObjectFreeze,
ReflectApply,
RegExpPrototypeTest,
SafeSet,
SafeArrayIterator,
Set,
SetPrototypeEntries,
SetPrototypeValues,
StringPrototypeEndsWith,
StringPrototypeReplace,
StringPrototypeSlice,
StringPrototypeStartsWith,
Symbol,
SymbolIterator,
Uint32Array,
} = primordials;

@@ -41,6 +47,8 @@ const {
} = require('internal/validators');
const constants = internalBinding('constants').os.signals;

const kInternal = Symbol('internal properties');

function assert(x, msg) {
if (!x) throw new ERR_ASSERTION(msg || 'assertion error');
}
@@ -293,18 +301,18 @@ function buildAllowedFlags() {

// Save these for comparison against flags provided to
// process.allowedNodeEnvironmentFlags.has() which lack leading dashes.
const nodeFlags = new SafeSet(ArrayPrototypeMap(allowedNodeEnvironmentFlags,
trimLeadingDashes));

class NodeEnvironmentFlagsSet extends SafeSet {
constructor(...args) {
super(...args);

// The super constructor consumes `add`, but
// disallow any future adds.
ObjectDefineProperty(this, 'add', {
value: () => this
});
const nodeFlags = ArrayPrototypeMap(allowedNodeEnvironmentFlags,
trimLeadingDashes);

class NodeEnvironmentFlagsSet extends Set {
constructor(array) {
super();
this[kInternal] = { array };
}

add() {
// No-op, `Set` API compatible

This comment has been minimized.

@jasnell

jasnell Feb 1, 2021
Member

It might seem silly but we need to at least consider whether there needs to be a deprecation cycle on this before it's made non-op.

This comment has been minimized.

@aduh95

aduh95 Feb 1, 2021
Author Contributor

Note that process.allowedNodeEnvironmentFlags.add is already no-op on all Node.js release lines.
We may or may not consider it a breaking change to make Set.prototype.add.call(process.allowedNodeEnvironmentFlags) no-op – IMHO we should not as this never was a documented thing, but no strong feelings.

return this;
}

delete() {
@@ -313,7 +321,7 @@ function buildAllowedFlags() {
}

clear() {
// No-op
// No-op, `Set` API compatible
}

has(key) {
@@ -328,13 +336,39 @@ function buildAllowedFlags() {
key = StringPrototypeReplace(key, replaceUnderscoresRegex, '-');
if (RegExpPrototypeTest(leadingDashesRegex, key)) {
key = StringPrototypeReplace(key, trailingValuesRegex, '');
return super.has(key);
return ArrayPrototypeIncludes(this[kInternal].array, key);
}
return nodeFlags.has(key);
return ArrayPrototypeIncludes(nodeFlags, key);
}
return false;
}

entries() {
this[kInternal].set ??=
new Set(new SafeArrayIterator(this[kInternal].array));
return SetPrototypeEntries(this[kInternal].set);
}

forEach(callback, thisArg = undefined) {
ArrayPrototypeForEach(
this[kInternal].array,
(v) => ReflectApply(callback, thisArg, [v, v, this])
);
}

get size() {
return this[kInternal].array.length;
}

values() {
this[kInternal].set ??=
new Set(new SafeArrayIterator(this[kInternal].array));
return SetPrototypeValues(this[kInternal].set);
}
}
NodeEnvironmentFlagsSet.prototype.keys =
NodeEnvironmentFlagsSet.prototype[SymbolIterator] =
NodeEnvironmentFlagsSet.prototype.values;

ObjectFreeze(NodeEnvironmentFlagsSet.prototype.constructor);
ObjectFreeze(NodeEnvironmentFlagsSet.prototype);
@@ -1,6 +1,6 @@
'use strict';

require('../common');
const common = require('../common');
const assert = require('assert');

// Assert legit flags are allowed, and bogus flags are disallowed
@@ -63,14 +63,41 @@ const assert = require('assert');

process.allowedNodeEnvironmentFlags.add('foo');
assert.strictEqual(process.allowedNodeEnvironmentFlags.has('foo'), false);
process.allowedNodeEnvironmentFlags.forEach((flag) => {
assert.strictEqual(flag === 'foo', false);
});
Set.prototype.add.call(process.allowedNodeEnvironmentFlags, 'foo');
assert.strictEqual(process.allowedNodeEnvironmentFlags.has('foo'), false);

process.allowedNodeEnvironmentFlags.clear();
assert.strictEqual(process.allowedNodeEnvironmentFlags.size > 0, true);
const thisArg = {};
process.allowedNodeEnvironmentFlags.forEach(
common.mustCallAtLeast(function(flag, _, set) {
assert.notStrictEqual(flag, 'foo');
assert.strictEqual(this, thisArg);
assert.strictEqual(set, process.allowedNodeEnvironmentFlags);
}),
thisArg
);

for (const flag of process.allowedNodeEnvironmentFlags.keys()) {
assert.notStrictEqual(flag, 'foo');
}
for (const flag of process.allowedNodeEnvironmentFlags.values()) {
assert.notStrictEqual(flag, 'foo');
}
for (const flag of process.allowedNodeEnvironmentFlags) {
assert.notStrictEqual(flag, 'foo');
}
for (const [flag] of process.allowedNodeEnvironmentFlags.entries()) {
assert.notStrictEqual(flag, 'foo');
}

const size = process.allowedNodeEnvironmentFlags.size;

process.allowedNodeEnvironmentFlags.clear();
assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
Set.prototype.clear.call(process.allowedNodeEnvironmentFlags);
assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);

process.allowedNodeEnvironmentFlags.delete('-r');
assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
Set.prototype.delete.call(process.allowedNodeEnvironmentFlags, '-r');
assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
}
ProTip! Use n and p to navigate between commits in a pull request.
X Tutup