From e760ab7620653a554c989d34c887e4a282b55a9c Mon Sep 17 00:00:00 2001 From: legendecas Date: Thu, 16 Mar 2023 01:17:51 +0800 Subject: [PATCH] src: bootstrap prepare stack trace callback in shadow realm Bootstrap per-realm callbacks like `prepare_stack_trace_callback` in the ShadowRealm. This enables stack trace decoration in the ShadowRealm. --- lib/internal/bootstrap/node.js | 25 ++--------- lib/internal/bootstrap/realm.js | 29 ++++++++++++ src/api/environment.cc | 17 ++++--- src/node_errors.cc | 10 ++--- src/node_realm.cc | 17 +++---- src/node_shadow_realm.cc | 15 ++++--- .../test-shadow-realm-prepare-stack-trace.js | 45 +++++++++++++++++++ 7 files changed, 106 insertions(+), 52 deletions(-) create mode 100644 lib/internal/bootstrap/realm.js create mode 100644 test/parallel/test-shadow-realm-prepare-stack-trace.js diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 657bad0d3eb7ff..00e361c5262cf5 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -1,6 +1,6 @@ // Hello, and welcome to hacking node.js! // -// This file is invoked by `Realm::BootstrapNode()` in `src/node_realm.cc`, +// This file is invoked by `Realm::BootstrapRealm()` in `src/node_realm.cc`, // and is responsible for setting up Node.js core before main scripts // under `lib/internal/main/` are executed. // @@ -35,6 +35,8 @@ // - `lib/internal/bootstrap/loaders.js`: this sets up internal binding and // module loaders, including `process.binding()`, `process._linkedBinding()`, // `internalBinding()` and `BuiltinModule`. +// - `lib/internal/bootstrap/realm.js`: this sets up per-realm internal states +// and callbacks, including `prepare_stack_trace_callback`. // // The initialization done in this script is included in both the main thread // and the worker threads. After this, further initialization is done based @@ -52,8 +54,6 @@ // passed by `BuiltinLoader::CompileAndCall()`. /* global process, require, internalBinding, primordials */ -setupPrepareStackTrace(); - const { FunctionPrototypeCall, JSONParse, @@ -336,25 +336,6 @@ process.emitWarning = emitWarning; // Note: only after this point are the timers effective } -function setupPrepareStackTrace() { - const { - setEnhanceStackForFatalException, - setPrepareStackTraceCallback, - } = internalBinding('errors'); - const { - prepareStackTrace, - fatalExceptionStackEnhancers: { - beforeInspector, - afterInspector, - }, - } = require('internal/errors'); - // Tell our PrepareStackTraceCallback passed to the V8 API - // to call prepareStackTrace(). - setPrepareStackTraceCallback(prepareStackTrace); - // Set the function used to enhance the error stack for printing - setEnhanceStackForFatalException(beforeInspector, afterInspector); -} - function setupProcessObject() { const EventEmitter = require('events'); const origProcProto = ObjectGetPrototypeOf(process); diff --git a/lib/internal/bootstrap/realm.js b/lib/internal/bootstrap/realm.js new file mode 100644 index 00000000000000..efeda7df426e1c --- /dev/null +++ b/lib/internal/bootstrap/realm.js @@ -0,0 +1,29 @@ +'use strict'; + +/** + * This file is executed in every realm that is created by Node.js, including + * the context of main thread, worker threads, and ShadowRealms. + * Only per-realm internal states and callbacks should be bootstrapped in this + * file and no globals should be exposed to the user code. + */ + +setupPrepareStackTrace(); + +function setupPrepareStackTrace() { + const { + setEnhanceStackForFatalException, + setPrepareStackTraceCallback, + } = internalBinding('errors'); + const { + prepareStackTrace, + fatalExceptionStackEnhancers: { + beforeInspector, + afterInspector, + }, + } = require('internal/errors'); + // Tell our PrepareStackTraceCallback passed to the V8 API + // to call prepareStackTrace(). + setPrepareStackTraceCallback(prepareStackTrace); + // Set the function used to enhance the error stack for printing + setEnhanceStackForFatalException(beforeInspector, afterInspector); +} diff --git a/src/api/environment.cc b/src/api/environment.cc index 12672603fcdc46..b82324484da2c8 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -67,17 +67,20 @@ MaybeLocal PrepareStackTraceCallback(Local context, if (env == nullptr) { return exception->ToString(context).FromMaybe(Local()); } - // TODO(legendecas): Per-realm prepareStackTrace callback. - // If we are in a Realm that is not the principal Realm (e.g. ShadowRealm), - // skip the prepareStackTrace callback to avoid passing the JS objects ( - // the exception and trace) across the realm boundary with the - // `Error.prepareStackTrace` override. Realm* current_realm = Realm::GetCurrent(context); + Local prepare; if (current_realm != nullptr && current_realm->kind() != Realm::Kind::kPrincipal) { - return exception->ToString(context).FromMaybe(Local()); + // If we are in a Realm that is not the principal Realm (e.g. ShadowRealm), + // call the realm specific prepareStackTrace callback to avoid passing the + // JS objects (the exception and trace) across the realm boundary with the + // `Error.prepareStackTrace` override. + prepare = current_realm->prepare_stack_trace_callback(); + } else { + // The context is created with ContextifyContext, call the principal + // realm's prepareStackTrace callback. + prepare = env->principal_realm()->prepare_stack_trace_callback(); } - Local prepare = env->prepare_stack_trace_callback(); if (prepare.IsEmpty()) { return exception->ToString(context).FromMaybe(Local()); } diff --git a/src/node_errors.cc b/src/node_errors.cc index ec9ae0f13eac0a..bc6348bbb41f76 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -960,9 +960,9 @@ void PerIsolateMessageListener(Local message, Local error) { } void SetPrepareStackTraceCallback(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + Realm* realm = Realm::GetCurrent(args); CHECK(args[0]->IsFunction()); - env->set_prepare_stack_trace_callback(args[0].As()); + realm->set_prepare_stack_trace_callback(args[0].As()); } static void SetSourceMapsEnabled(const FunctionCallbackInfo& args) { @@ -987,11 +987,11 @@ static void SetMaybeCacheGeneratedSourceMap( static void SetEnhanceStackForFatalException( const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + Realm* realm = Realm::GetCurrent(args); CHECK(args[0]->IsFunction()); CHECK(args[1]->IsFunction()); - env->set_enhance_fatal_stack_before_inspector(args[0].As()); - env->set_enhance_fatal_stack_after_inspector(args[1].As()); + realm->set_enhance_fatal_stack_before_inspector(args[0].As()); + realm->set_enhance_fatal_stack_after_inspector(args[1].As()); } // Side effect-free stringification that will never throw exceptions. diff --git a/src/node_realm.cc b/src/node_realm.cc index a8cd9b9a55da2f..6975be218d6d4e 100644 --- a/src/node_realm.cc +++ b/src/node_realm.cc @@ -335,11 +335,10 @@ void PrincipalRealm::MemoryInfo(MemoryTracker* tracker) const { } MaybeLocal PrincipalRealm::BootstrapRealm() { - EscapableHandleScope scope(isolate_); - - MaybeLocal result = ExecuteBootstrapper("internal/bootstrap/node"); + HandleScope scope(isolate_); - if (result.IsEmpty()) { + if (ExecuteBootstrapper("internal/bootstrap/realm").IsEmpty() || + ExecuteBootstrapper("internal/bootstrap/node").IsEmpty()) { return MaybeLocal(); } @@ -356,9 +355,7 @@ MaybeLocal PrincipalRealm::BootstrapRealm() { auto thread_switch_id = env_->is_main_thread() ? "internal/bootstrap/switches/is_main_thread" : "internal/bootstrap/switches/is_not_main_thread"; - result = ExecuteBootstrapper(thread_switch_id); - - if (result.IsEmpty()) { + if (ExecuteBootstrapper(thread_switch_id).IsEmpty()) { return MaybeLocal(); } @@ -366,9 +363,7 @@ MaybeLocal PrincipalRealm::BootstrapRealm() { env_->owns_process_state() ? "internal/bootstrap/switches/does_own_process_state" : "internal/bootstrap/switches/does_not_own_process_state"; - result = ExecuteBootstrapper(process_state_switch_id); - - if (result.IsEmpty()) { + if (ExecuteBootstrapper(process_state_switch_id).IsEmpty()) { return MaybeLocal(); } @@ -380,7 +375,7 @@ MaybeLocal PrincipalRealm::BootstrapRealm() { return MaybeLocal(); } - return scope.EscapeMaybe(result); + return v8::True(isolate_); } } // namespace node diff --git a/src/node_shadow_realm.cc b/src/node_shadow_realm.cc index bf5c7c94676c55..197cfb35c0a193 100644 --- a/src/node_shadow_realm.cc +++ b/src/node_shadow_realm.cc @@ -91,21 +91,22 @@ PER_REALM_STRONG_PERSISTENT_VALUES(V) #undef V v8::MaybeLocal ShadowRealm::BootstrapRealm() { - EscapableHandleScope scope(isolate_); - MaybeLocal result = v8::True(isolate_); + HandleScope scope(isolate_); // Skip "internal/bootstrap/node" as it installs node globals and per-isolate - // callbacks like PrepareStackTraceCallback. + // callbacks. + if (ExecuteBootstrapper("internal/bootstrap/realm").IsEmpty()) { + return MaybeLocal(); + } if (!env_->no_browser_globals()) { - result = ExecuteBootstrapper("internal/bootstrap/web/exposed-wildcard"); - - if (result.IsEmpty()) { + if (ExecuteBootstrapper("internal/bootstrap/web/exposed-wildcard") + .IsEmpty()) { return MaybeLocal(); } } - return result; + return v8::True(isolate_); } } // namespace shadow_realm diff --git a/test/parallel/test-shadow-realm-prepare-stack-trace.js b/test/parallel/test-shadow-realm-prepare-stack-trace.js new file mode 100644 index 00000000000000..0c681c12beaa7e --- /dev/null +++ b/test/parallel/test-shadow-realm-prepare-stack-trace.js @@ -0,0 +1,45 @@ +// Flags: --experimental-shadow-realm +'use strict'; + +require('../common'); +const assert = require('assert'); + +{ + // Validates inner Error.prepareStackTrace can not leak into the outer realm. + const shadowRealm = new ShadowRealm(); + + const stack = shadowRealm.evaluate(` +Error.prepareStackTrace = (error, trace) => { + globalThis.leaked = 'inner'; + return String(error); +}; + +try { + throw new Error('boom'); +} catch (e) { + e.stack; +} +`); + assert.strictEqual(stack, 'Error: boom'); + assert.strictEqual('leaked' in globalThis, false); +} + +{ + // Validates stacks can be generated in the ShadowRealm. + const shadowRealm = new ShadowRealm(); + + const stack = shadowRealm.evaluate(` +function myFunc() { + throw new Error('boom'); +} + +try { + myFunc(); +} catch (e) { + e.stack; +} +`); + const lines = stack.split('\n'); + assert.strictEqual(lines[0], 'Error: boom'); + assert.match(lines[1], /^ {4}at myFunc \(.*\)/); +}