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

Frozen intrinsics experimental flag #25685

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions LICENSE
Original file line number Diff line number Diff line change
Expand Up @@ -1349,6 +1349,24 @@ The externally maintained libraries used by Node.js are:
OR OTHER DEALINGS IN THE SOFTWARE.
"""

- caja, located at lib/internal/freeze_intrinsics.js, is licensed as follows:
"""
Adapted from SES/Caja - Copyright (C) 2011 Google Inc.
Copyright (C) 2018 Agoric

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
"""

- brotli, located at deps/brotli, is licensed as follows:
"""
Copyright (c) 2009, 2010, 2013-2016 by the Brotli Authors.
Expand Down
20 changes: 20 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,25 @@ added: v6.0.0
Force FIPS-compliant crypto on startup. (Cannot be disabled from script code.)
(Same requirements as `--enable-fips`.)

### `--frozen-intrinsics`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bikeshed: Is intrinsics the right name? At least in the context of V8 it usually refers to something else (for me), and builtins (or similar) might feel more natural/easier to grok?

(There’s also primordials, but like intrinsics, I feel like that might not be the most intuitive choice)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be wary of naming here, "builtins" might mean node's builtin modules such as require("fs") to some people, which are not being frozen. The term intrinsics does cover what is being frozen as per spec terminology.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, using language too close to the spec and too far away from developers are familiar with is what I want to avoid :)

I agree that builtins isn’t perfect.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@addaleax what is wrong with being close to spec? To my knowledge developers do not have a unique term for what is being affected/frozen here. Why not use an existing term if one does not exist? Mutating builtin prototypes is often a phrase used but "builtins" themselves are not clear and actively would mislead some. At least "intrinsics" has a very clear definition we can point to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think --frozen-js-builtins might be my preference at this point, but I’m not going to insist on anything. :)

@bmeck I think having an intuitive term is more important than an exact term here, that’s all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've stuck with --frozen-intrinsics for now, but happy to consider other options too.

<!-- YAML
added: REPLACEME
-->

> Stability: 1 - Experimental

Enable experimental frozen intrinsics like `Array` and `Object`.
guybedford marked this conversation as resolved.
Show resolved Hide resolved

Support is currently only provided for the root context and no guarantees are
currently provided that `global.Array` is indeed the default intrinsic
reference.

**Code breakage is highly likely with this flag**, especially since limited
support for subclassing builtins is provided currently due to ECMA-262 bug
https://github.com/tc39/ecma262/pull/1320.

Both of the above may change in future updates, which will be breaking changes.
guybedford marked this conversation as resolved.
Show resolved Hide resolved

### `--http-parser=library`
<!-- YAML
added: v11.4.0
Expand Down Expand Up @@ -687,6 +706,7 @@ Node.js options that are allowed are:
- `--experimental-report`
- `--experimental-vm-modules`
- `--force-fips`
- `--frozen-intrinsics`
guybedford marked this conversation as resolved.
Show resolved Hide resolved
- `--icu-data-dir`
- `--inspect`
- `--inspect-brk`
Expand Down
3 changes: 3 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,9 @@ Force FIPS-compliant crypto on startup
Same requirements as
.Fl -enable-fips .
.
.It Fl -frozen-intrinsics
Enable experimental frozen intrinsics support.
.
.It Fl -http-parser Ns = Ns Ar library
Chooses an HTTP parser library. Available values are
.Sy llhttp
Expand Down
10 changes: 10 additions & 0 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ function prepareMainThreadExecution() {
initializeClusterIPC();

initializeDeprecations();
initializeFrozenIntrinsics();
guybedford marked this conversation as resolved.
Show resolved Hide resolved
initializeESMLoader();
loadPreloadModules();
}
Expand Down Expand Up @@ -211,6 +212,14 @@ function initializeESMLoader() {
}
}

function initializeFrozenIntrinsics() {
if (getOptionValue('--frozen-intrinsics')) {
process.emitWarning('The --frozen-intrinsics flag is experimental',
'ExperimentalWarning');
require('internal/freeze_intrinsics')();
}
}

function loadPreloadModules() {
// For user code, we preload modules if `-r` is passed
const preloadModules = getOptionValue('--require');
Expand All @@ -226,6 +235,7 @@ module.exports = {
prepareMainThreadExecution,
initializeDeprecations,
initializeESMLoader,
initializeFrozenIntrinsics,
loadPreloadModules,
setupTraceCategoryState,
initializeReport
Expand Down
244 changes: 244 additions & 0 deletions lib/internal/freeze_intrinsics.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
// Adapted from SES/Caja - Copyright (C) 2011 Google Inc.
// Copyright (C) 2018 Agoric

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// SPDX-License-Identifier: MIT

// based upon:
// https://github.com/google/caja/blob/master/src/com/google/caja/ses/startSES.js
// https://github.com/google/caja/blob/master/src/com/google/caja/ses/repairES5.js
// https://github.com/tc39/proposal-frozen-realms/blob/91ac390e3451da92b5c27e354b39e52b7636a437/shim/src/deep-freeze.js

/* global WebAssembly, SharedArrayBuffer, console */
'use strict';
module.exports = function() {

const intrinsics = [
// Anonymous Intrinsics
// ThrowTypeError
Object.getOwnPropertyDescriptor(Function.prototype, 'caller').get,
// IteratorPrototype
Object.getPrototypeOf(
Object.getPrototypeOf(new Array()[Symbol.iterator]())
),
// ArrayIteratorPrototype
Object.getPrototypeOf(new Array()[Symbol.iterator]()),
// StringIteratorPrototype
Object.getPrototypeOf(new String()[Symbol.iterator]()),
// MapIteratorPrototype
Object.getPrototypeOf(new Map()[Symbol.iterator]()),
// SetIteratorPrototype
Object.getPrototypeOf(new Set()[Symbol.iterator]()),
// GeneratorFunction
Object.getPrototypeOf(function* () {}),
// AsyncFunction
Object.getPrototypeOf(async function() {}),
// AsyncGeneratorFunction
Object.getPrototypeOf(async function* () {}),
// TypedArray
Object.getPrototypeOf(Uint8Array),

// 18 The Global Object
eval,
isFinite,
isNaN,
parseFloat,
parseInt,
decodeURI,
decodeURIComponent,
encodeURI,
encodeURIComponent,

// 19 Fundamental Objects
Object, // 19.1
Function, // 19.2
Boolean, // 19.3
Symbol, // 19.4

// Disabled pending stack trace mutation handling
// Error, // 19.5
// EvalError,
// RangeError,
// ReferenceError,
// SyntaxError,
// TypeError,
// URIError,

// 20 Numbers and Dates
Number, // 20.1
Math, // 20.2
Date, // 20.3

// 21 Text Processing
String, // 21.1
RegExp, // 21.2

// 22 Indexed Collections
Array, // 22.1

Int8Array,
Uint8Array,
Uint8ClampedArray,
Int16Array,
Uint16Array,
Int32Array,
Uint32Array,
Float32Array,
Float64Array,
BigInt64Array,
BigUint64Array,

// 23 Keyed Collections
Map, // 23.1
Set, // 23.2
WeakMap, // 23.3
WeakSet, // 23.4

// 24 Structured Data
ArrayBuffer, // 24.1
DataView, // 24.3
JSON, // 24.5
Promise, // 25.4

// 26 Reflection
Reflect, // 26.1
Proxy, // 26.2

// B.2.1
escape,
unescape,

// Web compatibility
clearImmediate,
clearInterval,
clearTimeout,
decodeURI,
decodeURIComponent,
encodeURI,
encodeURIComponent,
setImmediate,
setInterval,
setTimeout,

// Other APIs
console,
BigInt,
Atomics,
WebAssembly,
SharedArrayBuffer
];

if (typeof Intl !== 'undefined')
intrinsics.push(Intl);

intrinsics.forEach(deepFreeze);

function deepFreeze(root) {

const { freeze, getOwnPropertyDescriptors, getPrototypeOf } = Object;
const { ownKeys } = Reflect;

// Objects that are deeply frozen.
// It turns out that Error is reachable from WebAssembly so it is
// explicitly added here to ensure it is not frozen
const frozenSet = new WeakSet([Error, Error.prototype]);

/**
* "innerDeepFreeze()" acts like "Object.freeze()", except that:
*
* To deepFreeze an object is to freeze it and all objects transitively
* reachable from it via transitive reflective property and prototype
* traversal.
*/
function innerDeepFreeze(node) {
// Objects that we have frozen in this round.
const freezingSet = new Set();

// If val is something we should be freezing but aren't yet,
// add it to freezingSet.
function enqueue(val) {
if (Object(val) !== val) {
// ignore primitives
return;
}
const type = typeof val;
if (type !== 'object' && type !== 'function') {
// NB: handle for any new cases in future
}
if (frozenSet.has(val) || freezingSet.has(val)) {
// todo use uncurried form
// Ignore if already frozen or freezing
return;
}
freezingSet.add(val); // todo use uncurried form
}

function doFreeze(obj) {
// Immediately freeze the object to ensure reactive
// objects such as proxies won't add properties
// during traversal, before they get frozen.

// Object are verified before being enqueued,
// therefore this is a valid candidate.
// Throws if this fails (strict mode).
freeze(obj);

// We rely upon certain commitments of Object.freeze and proxies here

// Get stable/immutable outbound links before a Proxy has a chance to do
// something sneaky.
const proto = getPrototypeOf(obj);
const descs = getOwnPropertyDescriptors(obj);
enqueue(proto);
ownKeys(descs).forEach((name) => {
// todo uncurried form
// todo: getOwnPropertyDescriptors is guaranteed to return well-formed
// descriptors, but they still inherit from Object.prototype. If
// someone has poisoned Object.prototype to add 'value' or 'get'
// properties, then a simple 'if ("value" in desc)' or 'desc.value'
// test could be confused. We use hasOwnProperty to be sure about
// whether 'value' is present or not, which tells us for sure that
// this is a data property.
const desc = descs[name];
if ('value' in desc) {
// todo uncurried form
enqueue(desc.value);
} else {
enqueue(desc.get);
enqueue(desc.set);
}
});
}

function dequeue() {
// New values added before forEach() has finished will be visited.
freezingSet.forEach(doFreeze); // todo curried forEach
}

function commit() {
// todo curried forEach
// we capture the real WeakSet.prototype.add above, in case someone
// changes it. The two-argument form of forEach passes the second
// argument as the 'this' binding, so we add to the correct set.
freezingSet.forEach(frozenSet.add, frozenSet);
}

enqueue(node);
dequeue();
commit();
}

innerDeepFreeze(root);
return root;
}
};
2 changes: 2 additions & 0 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
const {
initializeDeprecations,
initializeESMLoader,
initializeFrozenIntrinsics,
initializeReport,
loadPreloadModules,
setupTraceCategoryState
Expand Down Expand Up @@ -81,6 +82,7 @@ port.on('message', (message) => {
require('internal/process/policy').setup(manifestSrc, manifestURL);
}
initializeDeprecations();
initializeFrozenIntrinsics();
initializeESMLoader();
loadPreloadModules();
publicWorker.parentPort = publicPort;
Expand Down
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
'lib/internal/error-serdes.js',
'lib/internal/fixed_queue.js',
'lib/internal/freelist.js',
'lib/internal/freeze_intrinsics.js',
'lib/internal/fs/promises.js',
'lib/internal/fs/read_file_context.js',
'lib/internal/fs/streams.js',
Expand Down
4 changes: 4 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
kAllowedInEnvironment);
#endif // NODE_REPORT
AddOption("--expose-internals", "", &EnvironmentOptions::expose_internals);
AddOption("--frozen-intrinsics",
"experimental frozen intrinsics support",
&EnvironmentOptions::frozen_intrinsics,
kAllowedInEnvironment);
AddOption("--http-parser",
"Select which HTTP parser to use; either 'legacy' or 'llhttp' "
"(default: llhttp).",
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class EnvironmentOptions : public Options {
bool experimental_repl_await = false;
bool experimental_vm_modules = false;
bool expose_internals = false;
bool frozen_intrinsics = false;
std::string http_parser = "llhttp";
bool no_deprecation = false;
bool no_force_async_hooks_checks = false;
Expand Down
10 changes: 10 additions & 0 deletions test/parallel/test-freeze-intrinsics.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Flags: --frozen-intrinsics
'use strict';
require('../common');
const assert = require('assert');

try {
Object.defineProperty = 'asdf';
assert(false);
guybedford marked this conversation as resolved.
Show resolved Hide resolved
} catch {
}
Loading