From 92ca50636c8836a90248c2449206ad0eb96d43c1 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 30 Jan 2019 22:21:07 +0800 Subject: [PATCH] lib: save primordials during bootstrap and use it in builtins This patches changes the `safe_globals` internal module into a script that gets run during bootstrap and saves JavaScript builtins (primordials) into an object that is available for all other builtin modules to access lexically later. PR-URL: https://github.com/nodejs/node/pull/25816 Refs: https://github.com/nodejs/node/issues/18795 Reviewed-By: Bradley Farias Reviewed-By: Anna Henningsen Reviewed-By: Gus Caplan --- lib/.eslintrc.yaml | 1 + lib/internal/bootstrap/cache.js | 1 + lib/internal/bootstrap/loaders.js | 76 ++++++++------------- lib/internal/bootstrap/primordials.js | 84 ++++++++++++++++++++++++ lib/internal/error-serdes.js | 14 ++-- lib/internal/modules/esm/module_job.js | 6 +- lib/internal/modules/esm/module_map.js | 4 +- lib/internal/modules/esm/translators.js | 9 ++- lib/internal/policy/manifest.js | 17 +++-- lib/internal/safe_globals.js | 25 ------- lib/internal/trace_events_async_hooks.js | 2 +- lib/internal/vm/source_text_module.js | 2 +- lib/url.js | 2 +- node.gyp | 2 +- src/env.h | 2 + src/node.cc | 40 +++++++---- src/node_native_module.cc | 3 +- 17 files changed, 185 insertions(+), 105 deletions(-) create mode 100644 lib/internal/bootstrap/primordials.js delete mode 100644 lib/internal/safe_globals.js diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml index 81a764f0f85b81..181fb9cead2090 100644 --- a/lib/.eslintrc.yaml +++ b/lib/.eslintrc.yaml @@ -46,3 +46,4 @@ globals: DCHECK_LT: false DCHECK_NE: false internalBinding: false + primordials: false diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js index bed20106060193..5d5c441e65a7b7 100644 --- a/lib/internal/bootstrap/cache.js +++ b/lib/internal/bootstrap/cache.js @@ -22,6 +22,7 @@ const cannotBeRequired = [ 'internal/test/binding', + 'internal/bootstrap/primordials', 'internal/bootstrap/loaders', 'internal/bootstrap/node' ]; diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 7078707b83adb1..1007b41069c723 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -40,33 +40,20 @@ 'use strict'; // This file is compiled as if it's wrapped in a function with arguments -// passed by node::LoadEnvironment() +// passed by node::RunBootstrapping() /* global process, getBinding, getLinkedBinding, getInternalBinding */ -/* global debugBreak, experimentalModules, exposeInternals */ +/* global experimentalModules, exposeInternals, primordials */ -if (debugBreak) - debugger; // eslint-disable-line no-debugger - -const { - apply: ReflectApply, - deleteProperty: ReflectDeleteProperty, - get: ReflectGet, - getOwnPropertyDescriptor: ReflectGetOwnPropertyDescriptor, - has: ReflectHas, - set: ReflectSet, -} = Reflect; const { - prototype: { - hasOwnProperty: ObjectHasOwnProperty, - }, - create: ObjectCreate, - defineProperty: ObjectDefineProperty, - keys: ObjectKeys, -} = Object; + Reflect, + Object, + ObjectPrototype, + SafeSet +} = primordials; // Set up process.moduleLoadList. const moduleLoadList = []; -ObjectDefineProperty(process, 'moduleLoadList', { +Object.defineProperty(process, 'moduleLoadList', { value: moduleLoadList, configurable: true, enumerable: true, @@ -78,7 +65,7 @@ ObjectDefineProperty(process, 'moduleLoadList', { // that are whitelisted for access via process.binding()... This is used // to provide a transition path for modules that are being moved over to // internalBinding. -const internalBindingWhitelist = [ +const internalBindingWhitelist = new SafeSet([ 'async_wrap', 'buffer', 'cares_wrap', @@ -108,20 +95,17 @@ const internalBindingWhitelist = [ 'uv', 'v8', 'zlib' -]; -// We will use a lazy loaded SafeSet in internalBindingWhitelistHas -// for checking existence in this list. -let internalBindingWhitelistSet; +]); // Set up process.binding() and process._linkedBinding(). { - const bindingObj = ObjectCreate(null); + const bindingObj = Object.create(null); process.binding = function binding(module) { module = String(module); // Deprecated specific process.binding() modules, but not all, allow // selective fallback to internalBinding for the deprecated ones. - if (internalBindingWhitelistHas(module)) { + if (internalBindingWhitelist.has(module)) { return internalBinding(module); } let mod = bindingObj[module]; @@ -144,7 +128,7 @@ let internalBindingWhitelistSet; // Set up internalBinding() in the closure. let internalBinding; { - const bindingObj = ObjectCreate(null); + const bindingObj = Object.create(null); internalBinding = function internalBinding(module) { let mod = bindingObj[module]; if (typeof mod !== 'object') { @@ -245,8 +229,8 @@ NativeModule.requireWithFallbackInDeps = function(request) { }; const getOwn = (target, property, receiver) => { - return ReflectApply(ObjectHasOwnProperty, target, [property]) ? - ReflectGet(target, property, receiver) : + return Reflect.apply(ObjectPrototype.hasOwnProperty, target, [property]) ? + Reflect.get(target, property, receiver) : undefined; }; @@ -255,12 +239,12 @@ const getOwn = (target, property, receiver) => { // as the entire namespace (module.exports) and wrapped in a proxy such // that APMs and other behavior are still left intact. NativeModule.prototype.proxifyExports = function() { - this.exportKeys = ObjectKeys(this.exports); + this.exportKeys = Object.keys(this.exports); const update = (property, value) => { if (this.reflect !== undefined && - ReflectApply(ObjectHasOwnProperty, - this.reflect.exports, [property])) + Reflect.apply(ObjectPrototype.hasOwnProperty, + this.reflect.exports, [property])) this.reflect.exports[property].set(value); }; @@ -269,12 +253,12 @@ NativeModule.prototype.proxifyExports = function() { defineProperty: (target, prop, descriptor) => { // Use `Object.defineProperty` instead of `Reflect.defineProperty` // to throw the appropriate error if something goes wrong. - ObjectDefineProperty(target, prop, descriptor); + Object.defineProperty(target, prop, descriptor); if (typeof descriptor.get === 'function' && - !ReflectHas(handler, 'get')) { + !Reflect.has(handler, 'get')) { handler.get = (target, prop, receiver) => { - const value = ReflectGet(target, prop, receiver); - if (ReflectApply(ObjectHasOwnProperty, target, [prop])) + const value = Reflect.get(target, prop, receiver); + if (Reflect.apply(ObjectPrototype.hasOwnProperty, target, [prop])) update(prop, value); return value; }; @@ -283,15 +267,15 @@ NativeModule.prototype.proxifyExports = function() { return true; }, deleteProperty: (target, prop) => { - if (ReflectDeleteProperty(target, prop)) { + if (Reflect.deleteProperty(target, prop)) { update(prop, undefined); return true; } return false; }, set: (target, prop, value, receiver) => { - const descriptor = ReflectGetOwnPropertyDescriptor(target, prop); - if (ReflectSet(target, prop, value, receiver)) { + const descriptor = Reflect.getOwnPropertyDescriptor(target, prop); + if (Reflect.set(target, prop, value, receiver)) { if (descriptor && typeof descriptor.set === 'function') { for (const key of this.exportKeys) { update(key, getOwn(target, key, receiver)); @@ -319,7 +303,7 @@ NativeModule.prototype.compile = function() { NativeModule.require; const fn = compileFunction(id); - fn(this.exports, requireFn, this, process, internalBinding); + fn(this.exports, requireFn, this, process, internalBinding, primordials); if (experimentalModules && this.canBeRequiredByUsers) { this.proxifyExports(); @@ -344,13 +328,5 @@ if (process.env.NODE_V8_COVERAGE) { } } -function internalBindingWhitelistHas(name) { - if (!internalBindingWhitelistSet) { - const { SafeSet } = NativeModule.require('internal/safe_globals'); - internalBindingWhitelistSet = new SafeSet(internalBindingWhitelist); - } - return internalBindingWhitelistSet.has(name); -} - // This will be passed to internal/bootstrap/node.js. return loaderExports; diff --git a/lib/internal/bootstrap/primordials.js b/lib/internal/bootstrap/primordials.js new file mode 100644 index 00000000000000..2a97e74542a964 --- /dev/null +++ b/lib/internal/bootstrap/primordials.js @@ -0,0 +1,84 @@ +'use strict'; + +/* global breakAtBootstrap, primordials */ + +// This file subclasses and stores the JS builtins that come from the VM +// so that Node.js's builtin modules do not need to later look these up from +// the global proxy, which can be mutated by users. + +// TODO(joyeecheung): we can restrict access to these globals in builtin +// modules through the JS linter, for example: ban access such as `Object` +// (which falls back to a lookup in the global proxy) in favor of +// `primordials.Object` where `primordials` is a lexical variable passed +// by the native module compiler. + +if (breakAtBootstrap) { + debugger; // eslint-disable-line no-debugger +} + +function copyProps(src, dest) { + for (const key of Reflect.ownKeys(src)) { + if (!Reflect.getOwnPropertyDescriptor(dest, key)) { + Reflect.defineProperty( + dest, + key, + Reflect.getOwnPropertyDescriptor(src, key)); + } + } +} + +function makeSafe(unsafe, safe) { + copyProps(unsafe.prototype, safe.prototype); + copyProps(unsafe, safe); + Object.setPrototypeOf(safe.prototype, null); + Object.freeze(safe.prototype); + Object.freeze(safe); + return safe; +} + +// Subclass the constructors because we need to use their prototype +// methods later. +primordials.SafeMap = makeSafe( + Map, + class SafeMap extends Map {} +); +primordials.SafeWeakMap = makeSafe( + WeakMap, + class SafeWeakMap extends WeakMap {} +); +primordials.SafeSet = makeSafe( + Set, + class SafeSet extends Set {} +); +primordials.SafePromise = makeSafe( + Promise, + class SafePromise extends Promise {} +); + +// Create copies of the namespace objects +[ + 'JSON', + 'Math', + 'Reflect' +].forEach((name) => { + const target = primordials[name] = Object.create(null); + copyProps(global[name], target); +}); + +// Create copies of intrinsic objects +[ + 'Array', + 'Date', + 'Function', + 'Object', + 'RegExp', + 'String' +].forEach((name) => { + const target = primordials[name] = Object.create(null); + copyProps(global[name], target); + const proto = primordials[name + 'Prototype'] = Object.create(null); + copyProps(global[name].prototype, proto); +}); + +Object.setPrototypeOf(primordials, null); +Object.freeze(primordials); diff --git a/lib/internal/error-serdes.js b/lib/internal/error-serdes.js index d6e15ac77397cf..7b4b416b80c670 100644 --- a/lib/internal/error-serdes.js +++ b/lib/internal/error-serdes.js @@ -2,7 +2,13 @@ const Buffer = require('buffer').Buffer; const { serialize, deserialize } = require('v8'); -const { SafeSet } = require('internal/safe_globals'); +const { + SafeSet, + Object, + ObjectPrototype, + FunctionPrototype, + ArrayPrototype +} = primordials; const kSerializedError = 0; const kSerializedObject = 1; @@ -14,9 +20,9 @@ const GetOwnPropertyNames = Object.getOwnPropertyNames; const DefineProperty = Object.defineProperty; const Assign = Object.assign; const ObjectPrototypeToString = - Function.prototype.call.bind(Object.prototype.toString); -const ForEach = Function.prototype.call.bind(Array.prototype.forEach); -const Call = Function.prototype.call.bind(Function.prototype.call); + FunctionPrototype.call.bind(ObjectPrototype.toString); +const ForEach = FunctionPrototype.call.bind(ArrayPrototype.forEach); +const Call = FunctionPrototype.call.bind(FunctionPrototype.call); const errors = { Error, TypeError, RangeError, URIError, SyntaxError, ReferenceError, EvalError diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index e3fb91ca6ff505..e900babefd6261 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -1,7 +1,11 @@ 'use strict'; const { ModuleWrap } = internalBinding('module_wrap'); -const { SafeSet, SafePromise } = require('internal/safe_globals'); +const { + SafeSet, + SafePromise +} = primordials; + const { decorateErrorStack } = require('internal/util'); const assert = require('assert'); const resolvedPromise = SafePromise.resolve(); diff --git a/lib/internal/modules/esm/module_map.js b/lib/internal/modules/esm/module_map.js index a9d0c23c0e5ee9..1c1cd54a3d79ed 100644 --- a/lib/internal/modules/esm/module_map.js +++ b/lib/internal/modules/esm/module_map.js @@ -1,7 +1,9 @@ 'use strict'; const ModuleJob = require('internal/modules/esm/module_job'); -const { SafeMap } = require('internal/safe_globals'); +const { + SafeMap +} = primordials; const debug = require('util').debuglog('esm'); const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes; const { validateString } = require('internal/validators'); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 776b8d584df21f..25552cff0e8f47 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -12,14 +12,19 @@ const createDynamicModule = require( 'internal/modules/esm/create_dynamic_module'); const fs = require('fs'); const { _makeLong } = require('path'); -const { SafeMap } = require('internal/safe_globals'); +const { + SafeMap, + JSON, + FunctionPrototype, + StringPrototype +} = primordials; const { URL } = require('url'); const { debuglog, promisify } = require('util'); const esmLoader = require('internal/process/esm_loader'); const readFileAsync = promisify(fs.readFile); const readFileSync = fs.readFileSync; -const StringReplace = Function.call.bind(String.prototype.replace); +const StringReplace = FunctionPrototype.call.bind(StringPrototype.replace); const JsonParse = JSON.parse; const debug = debuglog('esm'); diff --git a/lib/internal/policy/manifest.js b/lib/internal/policy/manifest.js index 67b6d7b9d9b6c5..6c777a7c78b9c6 100644 --- a/lib/internal/policy/manifest.js +++ b/lib/internal/policy/manifest.js @@ -6,16 +6,21 @@ const { } = require('internal/errors').codes; const debug = require('util').debuglog('policy'); const SRI = require('internal/policy/sri'); -const { SafeWeakMap } = require('internal/safe_globals'); +const { + SafeWeakMap, + FunctionPrototype, + Object, + RegExpPrototype +} = primordials; const crypto = require('crypto'); const { Buffer } = require('buffer'); const { URL } = require('url'); const { createHash, timingSafeEqual } = crypto; -const HashUpdate = Function.call.bind(crypto.Hash.prototype.update); -const HashDigest = Function.call.bind(crypto.Hash.prototype.digest); -const BufferEquals = Function.call.bind(Buffer.prototype.equals); -const BufferToString = Function.call.bind(Buffer.prototype.toString); -const RegExpTest = Function.call.bind(RegExp.prototype.test); +const HashUpdate = FunctionPrototype.call.bind(crypto.Hash.prototype.update); +const HashDigest = FunctionPrototype.call.bind(crypto.Hash.prototype.digest); +const BufferEquals = FunctionPrototype.call.bind(Buffer.prototype.equals); +const BufferToString = FunctionPrototype.call.bind(Buffer.prototype.toString); +const RegExpTest = FunctionPrototype.call.bind(RegExpPrototype.test); const { entries } = Object; const kIntegrities = new SafeWeakMap(); const kReactions = new SafeWeakMap(); diff --git a/lib/internal/safe_globals.js b/lib/internal/safe_globals.js deleted file mode 100644 index 109409d535495d..00000000000000 --- a/lib/internal/safe_globals.js +++ /dev/null @@ -1,25 +0,0 @@ -'use strict'; - -const copyProps = (unsafe, safe) => { - for (const key of Reflect.ownKeys(unsafe)) { - if (!Object.getOwnPropertyDescriptor(safe, key)) { - Object.defineProperty( - safe, - key, - Object.getOwnPropertyDescriptor(unsafe, key)); - } - } -}; -const makeSafe = (unsafe, safe) => { - copyProps(unsafe.prototype, safe.prototype); - copyProps(unsafe, safe); - Object.setPrototypeOf(safe.prototype, null); - Object.freeze(safe.prototype); - Object.freeze(safe); - return safe; -}; - -exports.SafeMap = makeSafe(Map, class SafeMap extends Map {}); -exports.SafeWeakMap = makeSafe(WeakMap, class SafeWeakMap extends WeakMap {}); -exports.SafeSet = makeSafe(Set, class SafeSet extends Set {}); -exports.SafePromise = makeSafe(Promise, class SafePromise extends Promise {}); diff --git a/lib/internal/trace_events_async_hooks.js b/lib/internal/trace_events_async_hooks.js index 0cf5d517c2bf9f..b947cbb6152b4b 100644 --- a/lib/internal/trace_events_async_hooks.js +++ b/lib/internal/trace_events_async_hooks.js @@ -3,7 +3,7 @@ const { trace } = internalBinding('trace_events'); const async_wrap = internalBinding('async_wrap'); const async_hooks = require('async_hooks'); -const { SafeMap, SafeSet } = require('internal/safe_globals'); +const { SafeMap, SafeSet } = primordials; // Use small letters such that chrome://tracing groups by the name. // The behavior is not only useful but the same as the events emitted using diff --git a/lib/internal/vm/source_text_module.js b/lib/internal/vm/source_text_module.js index 8b107a9b74866f..6840b4281f46f1 100644 --- a/lib/internal/vm/source_text_module.js +++ b/lib/internal/vm/source_text_module.js @@ -17,7 +17,7 @@ const { customInspectSymbol, emitExperimentalWarning } = require('internal/util'); -const { SafePromise } = require('internal/safe_globals'); +const { SafePromise } = primordials; const { validateInt32, validateUint32, diff --git a/lib/url.js b/lib/url.js index 569733bfc4b0d7..0e02dbc1312101 100644 --- a/lib/url.js +++ b/lib/url.js @@ -23,7 +23,7 @@ const { toASCII } = require('internal/idna'); const { hexTable } = require('internal/querystring'); -const { SafeSet } = require('internal/safe_globals'); +const { SafeSet } = primordials; const { ERR_INVALID_ARG_TYPE diff --git a/node.gyp b/node.gyp index 81a0ade8f7ac83..ae5578b72ec952 100644 --- a/node.gyp +++ b/node.gyp @@ -27,6 +27,7 @@ 'node_intermediate_lib_type%': 'static_library', 'library_files': [ 'lib/internal/per_context.js', + 'lib/internal/bootstrap/primordials.js', 'lib/internal/bootstrap/cache.js', 'lib/internal/bootstrap/loaders.js', 'lib/internal/bootstrap/node.js', @@ -151,7 +152,6 @@ 'lib/internal/modules/esm/module_job.js', 'lib/internal/modules/esm/module_map.js', 'lib/internal/modules/esm/translators.js', - 'lib/internal/safe_globals.js', 'lib/internal/net.js', 'lib/internal/options.js', 'lib/internal/policy/manifest.js', diff --git a/src/env.h b/src/env.h index 4134efcb5db9c5..51775ff1a00155 100644 --- a/src/env.h +++ b/src/env.h @@ -254,6 +254,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2; V(port2_string, "port2") \ V(port_string, "port") \ V(preference_string, "preference") \ + V(primordials_string, "primordials") \ V(priority_string, "priority") \ V(process_string, "process") \ V(promise_string, "promise") \ @@ -364,6 +365,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2; V(performance_entry_template, v8::Function) \ V(pipe_constructor_template, v8::FunctionTemplate) \ V(process_object, v8::Object) \ + V(primordials, v8::Object) \ V(promise_reject_callback, v8::Function) \ V(promise_wrap_template, v8::ObjectTemplate) \ V(sab_lifetimepartner_constructor_template, v8::FunctionTemplate) \ diff --git a/src/node.cc b/src/node.cc index d3a3d299ac5268..8593daeefcfd35 100644 --- a/src/node.cc +++ b/src/node.cc @@ -259,18 +259,37 @@ MaybeLocal RunBootstrapping(Environment* env) { global->Set(context, FIXED_ONE_BYTE_STRING(env->isolate(), "global"), global) .FromJust(); + // Store primordials + env->set_primordials(Object::New(isolate)); + std::vector> primordials_params = { + FIXED_ONE_BYTE_STRING(isolate, "breakAtBootstrap"), + env->primordials_string() + }; + std::vector> primordials_args = { + Boolean::New(isolate, + env->options()->debug_options().break_node_first_line), + env->primordials() + }; + MaybeLocal primordials_ret = + ExecuteBootstrapper(env, + "internal/bootstrap/primordials", + &primordials_params, + &primordials_args); + if (primordials_ret.IsEmpty()) { + return MaybeLocal(); + } + // Create binding loaders std::vector> loaders_params = { env->process_string(), FIXED_ONE_BYTE_STRING(isolate, "getBinding"), FIXED_ONE_BYTE_STRING(isolate, "getLinkedBinding"), FIXED_ONE_BYTE_STRING(isolate, "getInternalBinding"), - // --inspect-brk-node - FIXED_ONE_BYTE_STRING(isolate, "debugBreak"), // --experimental-modules FIXED_ONE_BYTE_STRING(isolate, "experimentalModules"), // --expose-internals - FIXED_ONE_BYTE_STRING(isolate, "exposeInternals")}; + FIXED_ONE_BYTE_STRING(isolate, "exposeInternals"), + env->primordials_string()}; std::vector> loaders_args = { process, env->NewFunctionTemplate(binding::GetBinding) @@ -282,12 +301,9 @@ MaybeLocal RunBootstrapping(Environment* env) { env->NewFunctionTemplate(binding::GetInternalBinding) ->GetFunction(context) .ToLocalChecked(), - Boolean::New(isolate, - env->options()->debug_options().break_node_first_line), - Boolean::New(isolate, - env->options()->experimental_modules), - Boolean::New(isolate, - env->options()->expose_internals)}; + Boolean::New(isolate, env->options()->experimental_modules), + Boolean::New(isolate, env->options()->expose_internals), + env->primordials()}; // Bootstrap internal loaders MaybeLocal loader_exports = ExecuteBootstrapper( @@ -311,11 +327,13 @@ MaybeLocal RunBootstrapping(Environment* env) { std::vector> node_params = { env->process_string(), FIXED_ONE_BYTE_STRING(isolate, "loaderExports"), - FIXED_ONE_BYTE_STRING(isolate, "isMainThread")}; + FIXED_ONE_BYTE_STRING(isolate, "isMainThread"), + env->primordials_string()}; std::vector> node_args = { process, loader_exports_obj, - Boolean::New(isolate, env->is_main_thread())}; + Boolean::New(isolate, env->is_main_thread()), + env->primordials()}; MaybeLocal result = ExecuteBootstrapper( env, "internal/bootstrap/node", &node_params, &node_args); diff --git a/src/node_native_module.cc b/src/node_native_module.cc index 675495e34bff5a..662aad31d5d159 100644 --- a/src/node_native_module.cc +++ b/src/node_native_module.cc @@ -181,7 +181,8 @@ MaybeLocal NativeModuleLoader::CompileAsModule(Environment* env, env->require_string(), env->module_string(), env->process_string(), - env->internal_binding_string()}; + env->internal_binding_string(), + env->primordials_string()}; return per_process::native_module_loader.LookupAndCompile( env->context(), id, ¶meters, env); }