From 0bb17b7acc6199613c1acca37cd4eb8786456519 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 3 Apr 2024 16:17:41 +0530 Subject: [PATCH 01/12] feat: rewrite node:vm --- ext/node/lib.rs | 6 +- ext/node/ops/mod.rs | 2 + ext/node/ops/vm.rs | 128 ++++++++++++++++++++++++++ ext/node/ops/vm_internal.rs | 176 ++++++++++++++++++++++++++++++++++++ ext/node/polyfills/vm.ts | 44 +++++---- 5 files changed, 335 insertions(+), 21 deletions(-) create mode 100644 ext/node/ops/vm.rs create mode 100644 ext/node/ops/vm_internal.rs diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 85abe49e496517..f91ed0b1a3f00b 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -260,7 +260,11 @@ deno_core::extension!(deno_node, ops::winerror::op_node_sys_to_uv_error, ops::v8::op_v8_cached_data_version_tag, ops::v8::op_v8_get_heap_statistics, - ops::v8::op_vm_run_in_new_context, + ops::vm::op_vm_create_script, + ops::vm::op_vm_create_context, + ops::vm::op_vm_script_run_in_context, + ops::vm::op_vm_script_run_in_this_context, + ops::vm::op_vm_is_context, ops::idna::op_node_idna_domain_to_ascii, ops::idna::op_node_idna_domain_to_unicode, ops::idna::op_node_idna_punycode_to_ascii, diff --git a/ext/node/ops/mod.rs b/ext/node/ops/mod.rs index 8aed274bc201a4..c14b63bf4dbacf 100644 --- a/ext/node/ops/mod.rs +++ b/ext/node/ops/mod.rs @@ -10,6 +10,8 @@ pub mod os; pub mod require; pub mod util; pub mod v8; +pub mod vm; +mod vm_internal; pub mod winerror; pub mod worker_threads; pub mod zlib; diff --git a/ext/node/ops/vm.rs b/ext/node/ops/vm.rs new file mode 100644 index 00000000000000..5e89820f3be5a2 --- /dev/null +++ b/ext/node/ops/vm.rs @@ -0,0 +1,128 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use deno_core::error::AnyError; +use deno_core::error::type_error; +use deno_core::op2; +use deno_core::v8; + +use super::vm_internal as i; + +struct Script { + inner: i::ContextifyScript, +} + +impl Script { + fn new( + scope: &mut v8::HandleScope, + source: v8::Local, + ) -> Result { + Ok(Self { + inner: i::ContextifyScript::new(scope, source)?, + }) + } + + fn run_in_this_context<'s>( + &self, + scope: &'s mut v8::HandleScope, + ) -> Result, AnyError> { + let context = scope.get_current_context(); + + let context_scope = &mut v8::ContextScope::new(scope, context); + let mut scope = v8::EscapableHandleScope::new(context_scope); + let result = self + .inner + .eval_machine(&mut scope, context) + .ok_or_else(|| type_error("Failed to run script"))?; + Ok(scope.escape(result)) + } + + fn run_in_context<'s>( + &self, + scope: &mut v8::HandleScope<'s>, + sandbox: v8::Local<'s, v8::Value>, + ) -> Result, AnyError> { + let context = if let Ok(sandbox_obj) = sandbox.try_into() { + let context = i::ContextifyContext::from_sandbox_obj(scope, sandbox_obj); + todo!() + } else { + scope.get_current_context() + }; + + let context_scope = &mut v8::ContextScope::new(scope, context); + let mut scope = v8::EscapableHandleScope::new(context_scope); + let result = self + .inner + .eval_machine(&mut scope, context) + .ok_or_else(|| type_error("Failed to run script"))?; + Ok(scope.escape(result)) + } +} + +#[op2] +pub fn op_vm_create_script<'a>( + scope: &mut v8::HandleScope<'a>, + source: v8::Local<'a, v8::String>, +) -> Result, AnyError> { + let script = Script::new(scope, source)?; + Ok(deno_core::cppgc::make_cppgc_object(scope, script)) +} + +#[op2] +pub fn op_vm_script_run_in_context<'a>( + scope: &mut v8::HandleScope<'a>, + #[cppgc] script: &Script, + sandbox: v8::Local<'a, v8::Value>, +) -> Result, AnyError> { + script.run_in_context(scope, sandbox) +} + +#[op2] +pub fn op_vm_script_run_in_this_context<'a>( + scope: &'a mut v8::HandleScope, + #[cppgc] script: &Script, +) -> Result, AnyError> { + script.run_in_this_context(scope) +} + +#[op2] +pub fn op_vm_create_context( + scope: &mut v8::HandleScope, + sandbox_obj: v8::Local, +) { + // Don't allow contextifying a sandbox multiple times. + + i::ContextifyContext::attach(scope, sandbox_obj); +} + +#[op2] +pub fn op_vm_is_context( + scope: &mut v8::HandleScope, + sandbox_obj: v8::Local, +) -> bool { + todo!() +} + +#[cfg(test)] +mod tests { + use super::*; + use deno_core::v8; + + #[test] + fn test_run_in_this_context() { + let platform = v8::new_default_platform(0, false).make_shared(); + v8::V8::initialize_platform(platform); + v8::V8::initialize(); + + let isolate = &mut v8::Isolate::new(Default::default()); + + let scope = &mut v8::HandleScope::new(isolate); + let context = v8::Context::new(scope); + let scope = &mut v8::ContextScope::new(scope, context); + + let source = v8::String::new(scope, "1 + 2").unwrap(); + let script = Script::new(scope, source).unwrap(); + + let result = script.run_in_this_context(scope).unwrap(); + assert!(result.is_number()); + } +} diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs new file mode 100644 index 00000000000000..98122998fa4cf6 --- /dev/null +++ b/ext/node/ops/vm_internal.rs @@ -0,0 +1,176 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use deno_core::error::AnyError; +use deno_core::error::type_error; +use deno_core::v8; + +pub const PRIVATE_SYMBOL_NAME: v8::OneByteConst = v8::String::create_external_onebyte_const(b"node:contextify:context"); + +/// An unbounded script that can be run in a context. +#[derive(Debug)] +pub struct ContextifyScript { + script: v8::Global, +} + +impl ContextifyScript { + pub fn new( + scope: &mut v8::HandleScope, + source_str: v8::Local, + ) -> Result { + let source = v8::script_compiler::Source::new(source_str, None); + + let unbound_script = v8::script_compiler::compile_unbound_script( + scope, + source, + v8::script_compiler::CompileOptions::NoCompileOptions, + v8::script_compiler::NoCacheReason::NoReason, + ).ok_or_else(|| type_error("Failed to compile script"))?; + let script = v8::Global::new(scope, unbound_script); + Ok(Self { script }) + } + + pub fn eval_machine<'s>( + &self, + scope: &mut v8::HandleScope<'s>, + context: v8::Local, + ) -> Option> { + let tc_scope = &mut v8::TryCatch::new(scope); + + let unbound_script = v8::Local::new(tc_scope, self.script.clone()); + let script = unbound_script.bind_to_current_context(tc_scope); + + // TODO: support `break_on_first_line` arg + // TODO: support `break_on_sigint` and `timeout` args + let result = script.run(tc_scope); + // TODO: support `microtask_queue` arg + + if tc_scope.has_caught() { + // TODO: + // if display_errors { + // + // } + + if !tc_scope.has_terminated() { + tc_scope.rethrow(); + } + + return None; + } + + Some(result.unwrap()) + } +} + +#[derive(Debug)] +pub struct ContextifyContext { + +} + +impl ContextifyContext { + pub fn attach( + scope: &mut v8::HandleScope, + sandbox_obj: v8::Local, + ) { + let tmp = init_global_template(scope); + + let context = create_v8_context(scope, tmp, None); + Self::from_context(scope, context, sandbox_obj); + } + + pub fn from_context( + scope: &mut v8::HandleScope, + v8_context: v8::Local, + sandbox_obj: v8::Local, + ) { + let main_context = scope.get_current_context(); + let new_context_global = v8_context.global(scope); + v8_context.set_security_token(main_context.get_security_token(scope)); + + let wrapper = deno_core::cppgc::make_cppgc_object(scope, Self {}); + + let private_str = v8::String::new_from_onebyte_const(scope, &PRIVATE_SYMBOL_NAME); + let private_symbol = v8::Private::for_api(scope, private_str); + + sandbox_obj + .set_private(scope, private_symbol, wrapper.into()); + } + + pub fn from_sandbox_obj<'a>( + scope: &mut v8::HandleScope, + sandbox_obj: v8::Local, + ) -> Option<&'a Self> { + let private_str = v8::String::new_from_onebyte_const(scope, &PRIVATE_SYMBOL_NAME); + let private_symbol = v8::Private::for_api(scope, private_str); + + sandbox_obj.get_private(scope, private_symbol) + .and_then(|wrapper| deno_core::cppgc::try_unwrap_cppgc_object::(wrapper)) + } +} + +pub const VM_CONTEXT_INDEX: usize = 0; + +fn create_v8_context<'a>( + scope: &mut v8::HandleScope<'a>, + object_template: v8::Local, + snapshot_data: Option<&'static [u8]>, +) -> v8::Local<'a, v8::Context> { + let scope = &mut v8::EscapableHandleScope::new(scope); + + let context = if let Some(_snapshot_data) = snapshot_data { + v8::Context::from_snapshot(scope, VM_CONTEXT_INDEX).unwrap() + } else { + v8::Context::new_from_template(scope, object_template) + }; + + scope.escape(context) +} + +#[derive(Debug, Clone)] +struct SlotContextifyGlobalTemplate(v8::Global); + +fn init_global_template<'a>( + scope: &mut v8::HandleScope<'a>, +) -> v8::Local<'a, v8::ObjectTemplate> { + let mut maybe_object_template_slot = + scope.get_slot::(); + + if maybe_object_template_slot.is_none() { + init_global_template_inner(scope); + maybe_object_template_slot = + scope.get_slot::(); + } + let object_template_slot = maybe_object_template_slot + .expect("ContextifyGlobalTemplate slot should be already populated.") + .clone(); + v8::Local::new(scope, object_template_slot.0) +} + + +extern "C" fn c_noop(info: *const v8::FunctionCallbackInfo) {} + +fn init_global_template_inner(scope: &mut v8::HandleScope) { +let global_func_template = + v8::FunctionTemplate::builder_raw(c_noop).build(scope); + let global_object_template = global_func_template.instance_template(scope); + + let named_property_handler_config = { + let mut config = v8::NamedPropertyHandlerConfiguration::new() + .flags(v8::PropertyHandlerFlags::HAS_NO_SIDE_EFFECT); + config + }; + + let indexed_property_handler_config = { + let mut config = v8::IndexedPropertyHandlerConfiguration::new() + .flags(v8::PropertyHandlerFlags::HAS_NO_SIDE_EFFECT); + config + }; + + global_object_template + .set_named_property_handler(named_property_handler_config); + global_object_template + .set_indexed_property_handler(indexed_property_handler_config); + let contextify_global_template_slot = SlotContextifyGlobalTemplate( + v8::Global::new(scope, global_object_template), + ); + scope.set_slot(contextify_global_template_slot); +} diff --git a/ext/node/polyfills/vm.ts b/ext/node/polyfills/vm.ts index 10000b08c725f0..75938b06627a24 100644 --- a/ext/node/polyfills/vm.ts +++ b/ext/node/polyfills/vm.ts @@ -5,24 +5,27 @@ import { core } from "ext:core/mod.js"; import { notImplemented } from "ext:deno_node/_utils.ts"; -import { op_vm_run_in_new_context } from "ext:core/ops"; +import { + op_vm_create_script, + op_vm_create_context, + op_vm_script_run_in_context, + op_vm_script_run_in_this_context, + op_vm_is_context, +} from "ext:core/ops"; export class Script { - code: string; + #inner; + constructor(code: string, _options = {}) { - this.code = `${code}`; + this.#inner = op_vm_create_script(code); } runInThisContext(_options: any) { - const [result, error] = core.evalContext(this.code, "data:"); - if (error) { - throw error.thrown; - } - return result; + return op_vm_script_run_in_this_context(this.#inner); } - runInContext(_contextifiedObject: any, _options: any) { - notImplemented("Script.prototype.runInContext"); + runInContext(contextifiedObject: any, _options: any) { + return op_vm_script_run_in_context(this.#inner, contextifiedObject); } runInNewContext(contextObject: any, options: any) { @@ -31,7 +34,8 @@ export class Script { "Script.runInNewContext options are currently not supported", ); } - return op_vm_run_in_new_context(this.code, contextObject); + const context = createContext(contextObject); + return this.runInContext(context, options); } createCachedData() { @@ -39,8 +43,9 @@ export class Script { } } -export function createContext(_contextObject: any, _options: any) { - notImplemented("createContext"); +export function createContext(contextObject: any, _options: any) { + op_vm_create_context(contextObject); + return contextObject; } export function createScript(code: string, options: any) { @@ -48,11 +53,11 @@ export function createScript(code: string, options: any) { } export function runInContext( - _code: string, - _contextifiedObject: any, + code: string, + contextifiedObject: any, _options: any, ) { - notImplemented("runInContext"); + return createScript(code).runInContext(contextifiedObject); } export function runInNewContext( @@ -63,7 +68,7 @@ export function runInNewContext( if (options) { console.warn("vm.runInNewContext options are currently not supported"); } - return op_vm_run_in_new_context(code, contextObject); + return createScript(code).runInNewContext(contextObject); } export function runInThisContext( @@ -73,9 +78,8 @@ export function runInThisContext( return createScript(code, options).runInThisContext(options); } -export function isContext(_maybeContext: any) { - // TODO(@littledivy): Currently we do not expose contexts so this is always false. - return false; +export function isContext(maybeContext: any) { + return op_vm_is_context(maybeContext); } export function compileFunction(_code: string, _params: any, _options: any) { From 36f3bc6309be8638017de6dc3938a4544ea1b285 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 3 Apr 2024 19:12:04 +0530 Subject: [PATCH 02/12] wip --- ext/node/ops/vm.rs | 8 ++- ext/node/ops/vm_internal.rs | 116 ++++++++++++++++++++++++++++++++++-- ext/node/polyfills/vm.ts | 4 +- 3 files changed, 119 insertions(+), 9 deletions(-) diff --git a/ext/node/ops/vm.rs b/ext/node/ops/vm.rs index 5e89820f3be5a2..f0ac2148877c5c 100644 --- a/ext/node/ops/vm.rs +++ b/ext/node/ops/vm.rs @@ -42,8 +42,10 @@ impl Script { sandbox: v8::Local<'s, v8::Value>, ) -> Result, AnyError> { let context = if let Ok(sandbox_obj) = sandbox.try_into() { - let context = i::ContextifyContext::from_sandbox_obj(scope, sandbox_obj); - todo!() + let context = + i::ContextifyContext::from_sandbox_obj(scope, sandbox_obj) + .ok_or_else(|| type_error("Invalid sandbox object"))?; + context.context(scope) } else { scope.get_current_context() }; @@ -99,7 +101,7 @@ pub fn op_vm_is_context( scope: &mut v8::HandleScope, sandbox_obj: v8::Local, ) -> bool { - todo!() + false } #[cfg(test)] diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 98122998fa4cf6..3e994acc7db487 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -3,6 +3,7 @@ use deno_core::error::AnyError; use deno_core::error::type_error; use deno_core::v8; +use deno_core::v8::MapFnTo; pub const PRIVATE_SYMBOL_NAME: v8::OneByteConst = v8::String::create_external_onebyte_const(b"node:contextify:context"); @@ -61,9 +62,10 @@ impl ContextifyScript { } } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct ContextifyContext { - + context: v8::Global, + sandbox: v8::Global, } impl ContextifyContext { @@ -86,7 +88,12 @@ impl ContextifyContext { let new_context_global = v8_context.global(scope); v8_context.set_security_token(main_context.get_security_token(scope)); - let wrapper = deno_core::cppgc::make_cppgc_object(scope, Self {}); + let context = v8::Global::new(scope, v8_context); + let sandbox = v8::Global::new(scope, sandbox_obj); + let wrapper = deno_core::cppgc::make_cppgc_object(scope, Self { + context, + sandbox, + }); let private_str = v8::String::new_from_onebyte_const(scope, &PRIVATE_SYMBOL_NAME); let private_symbol = v8::Private::for_api(scope, private_str); @@ -105,6 +112,33 @@ impl ContextifyContext { sandbox_obj.get_private(scope, private_symbol) .and_then(|wrapper| deno_core::cppgc::try_unwrap_cppgc_object::(wrapper)) } + + pub fn context<'a>(&self, scope: &mut v8::HandleScope<'a>) -> v8::Local<'a, v8::Context> { + v8::Local::new(scope, &self.context) + } + + fn global_proxy<'s>( + &self, + scope: &mut v8::HandleScope<'s>, + ) -> v8::Local<'s, v8::Object> { + let ctx = self.context(scope); + ctx.global(scope) + } + + fn sandbox<'a>(&self, scope: &mut v8::HandleScope<'a>) -> v8::Local<'a, v8::Object> { + v8::Local::new(scope, &self.sandbox) + } + + fn get( + scope: &mut v8::HandleScope, + object: v8::Local, + ) -> Option { + let Some(context) = object.get_creation_context(scope) else { + return None; + }; + + context.get_slot::(scope).cloned() + } } pub const VM_CONTEXT_INDEX: usize = 0; @@ -148,14 +182,33 @@ fn init_global_template<'a>( extern "C" fn c_noop(info: *const v8::FunctionCallbackInfo) {} +thread_local! { + pub static GETTER_MAP_FN: v8::GenericNamedPropertyGetterCallback<'static> = property_getter.map_fn_to(); + pub static SETTER_MAP_FN: v8::GenericNamedPropertySetterCallback<'static> = property_setter.map_fn_to(); + pub static DELETER_MAP_FN: v8::GenericNamedPropertyGetterCallback<'static> = property_deleter.map_fn_to(); + pub static ENUMERATOR_MAP_FN: v8::GenericNamedPropertyEnumeratorCallback<'static> = property_enumerator.map_fn_to(); + pub static DEFINER_MAP_FN: v8::GenericNamedPropertyDefinerCallback<'static> = property_definer.map_fn_to(); + pub static DESCRIPTOR_MAP_FN: v8::GenericNamedPropertyGetterCallback<'static> = property_descriptor.map_fn_to(); +} + fn init_global_template_inner(scope: &mut v8::HandleScope) { -let global_func_template = + let global_func_template = v8::FunctionTemplate::builder_raw(c_noop).build(scope); let global_object_template = global_func_template.instance_template(scope); let named_property_handler_config = { let mut config = v8::NamedPropertyHandlerConfiguration::new() .flags(v8::PropertyHandlerFlags::HAS_NO_SIDE_EFFECT); + + config = GETTER_MAP_FN.with(|getter| config.getter_raw(*getter)); + config = SETTER_MAP_FN.with(|setter| config.setter_raw(*setter)); + config = DELETER_MAP_FN.with(|deleter| config.deleter_raw(*deleter)); + config = + ENUMERATOR_MAP_FN.with(|enumerator| config.enumerator_raw(*enumerator)); + config = DEFINER_MAP_FN.with(|definer| config.definer_raw(*definer)); + config = + DESCRIPTOR_MAP_FN.with(|descriptor| config.descriptor_raw(*descriptor)); + config }; @@ -174,3 +227,58 @@ let global_func_template = ); scope.set_slot(contextify_global_template_slot); } + + +fn property_getter<'s>( + scope: &mut v8::HandleScope<'s>, + key: v8::Local<'s, v8::Name>, + args: v8::PropertyCallbackArguments<'s>, + mut rv: v8::ReturnValue, +) { + let ctx = ContextifyContext::get(scope, args.this()); + + let context = ctx.context(scope); + let sandbox = ctx.sandbox(scope); +} + +fn property_setter<'s>( + scope: &mut v8::HandleScope<'s>, + key: v8::Local<'s, v8::Name>, + value: v8::Local<'s, v8::Value>, + args: v8::PropertyCallbackArguments<'s>, + mut rv: v8::ReturnValue, +) { +} + +fn property_deleter<'s>( + scope: &mut v8::HandleScope<'s>, + key: v8::Local<'s, v8::Name>, + args: v8::PropertyCallbackArguments<'s>, + mut rv: v8::ReturnValue, +) { + +} + +fn property_enumerator<'s>( + scope: &mut v8::HandleScope<'s>, + args: v8::PropertyCallbackArguments<'s>, + mut rv: v8::ReturnValue, +) { +} + +fn property_definer<'s>( + scope: &mut v8::HandleScope<'s>, + key: v8::Local<'s, v8::Name>, + descriptor: &v8::PropertyDescriptor, + args: v8::PropertyCallbackArguments<'s>, + mut rv: v8::ReturnValue, +) { +} + +fn property_descriptor<'s>( + scope: &mut v8::HandleScope<'s>, + key: v8::Local<'s, v8::Name>, + args: v8::PropertyCallbackArguments<'s>, + mut rv: v8::ReturnValue, +) { +} diff --git a/ext/node/polyfills/vm.ts b/ext/node/polyfills/vm.ts index 75938b06627a24..c5ced9cbedd827 100644 --- a/ext/node/polyfills/vm.ts +++ b/ext/node/polyfills/vm.ts @@ -39,11 +39,11 @@ export class Script { } createCachedData() { - notImplemented("Script.prototyp.createCachedData"); + notImplemented("Script.prototype.createCachedData"); } } -export function createContext(contextObject: any, _options: any) { +export function createContext(contextObject: any = {}, _options: any) { op_vm_create_context(contextObject); return contextObject; } From 005e07cfb9943a2e77156871ce24584bbd133266 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 3 Apr 2024 19:27:38 +0530 Subject: [PATCH 03/12] piece by piece --- ext/node/ops/vm.rs | 7 +- ext/node/ops/vm_internal.rs | 165 ++++++++++++++++++++++++++++++------ 2 files changed, 141 insertions(+), 31 deletions(-) diff --git a/ext/node/ops/vm.rs b/ext/node/ops/vm.rs index f0ac2148877c5c..3785c32a780ece 100644 --- a/ext/node/ops/vm.rs +++ b/ext/node/ops/vm.rs @@ -1,7 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use deno_core::error::AnyError; use deno_core::error::type_error; +use deno_core::error::AnyError; use deno_core::op2; use deno_core::v8; @@ -42,9 +42,8 @@ impl Script { sandbox: v8::Local<'s, v8::Value>, ) -> Result, AnyError> { let context = if let Ok(sandbox_obj) = sandbox.try_into() { - let context = - i::ContextifyContext::from_sandbox_obj(scope, sandbox_obj) - .ok_or_else(|| type_error("Invalid sandbox object"))?; + let context = i::ContextifyContext::from_sandbox_obj(scope, sandbox_obj) + .ok_or_else(|| type_error("Invalid sandbox object"))?; context.context(scope) } else { scope.get_current_context() diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 3e994acc7db487..44020d4bf2ee3e 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -1,11 +1,12 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use deno_core::error::AnyError; use deno_core::error::type_error; +use deno_core::error::AnyError; use deno_core::v8; use deno_core::v8::MapFnTo; -pub const PRIVATE_SYMBOL_NAME: v8::OneByteConst = v8::String::create_external_onebyte_const(b"node:contextify:context"); +pub const PRIVATE_SYMBOL_NAME: v8::OneByteConst = + v8::String::create_external_onebyte_const(b"node:contextify:context"); /// An unbounded script that can be run in a context. #[derive(Debug)] @@ -25,7 +26,8 @@ impl ContextifyScript { source, v8::script_compiler::CompileOptions::NoCompileOptions, v8::script_compiler::NoCacheReason::NoReason, - ).ok_or_else(|| type_error("Failed to compile script"))?; + ) + .ok_or_else(|| type_error("Failed to compile script"))?; let script = v8::Global::new(scope, unbound_script); Ok(Self { script }) } @@ -40,17 +42,9 @@ impl ContextifyScript { let unbound_script = v8::Local::new(tc_scope, self.script.clone()); let script = unbound_script.bind_to_current_context(tc_scope); - // TODO: support `break_on_first_line` arg - // TODO: support `break_on_sigint` and `timeout` args let result = script.run(tc_scope); - // TODO: support `microtask_queue` arg if tc_scope.has_caught() { - // TODO: - // if display_errors { - // - // } - if !tc_scope.has_terminated() { tc_scope.rethrow(); } @@ -90,30 +84,35 @@ impl ContextifyContext { let context = v8::Global::new(scope, v8_context); let sandbox = v8::Global::new(scope, sandbox_obj); - let wrapper = deno_core::cppgc::make_cppgc_object(scope, Self { - context, - sandbox, - }); + let wrapper = + deno_core::cppgc::make_cppgc_object(scope, Self { context, sandbox }); - let private_str = v8::String::new_from_onebyte_const(scope, &PRIVATE_SYMBOL_NAME); + let private_str = + v8::String::new_from_onebyte_const(scope, &PRIVATE_SYMBOL_NAME); let private_symbol = v8::Private::for_api(scope, private_str); - sandbox_obj - .set_private(scope, private_symbol, wrapper.into()); + sandbox_obj.set_private(scope, private_symbol, wrapper.into()); } pub fn from_sandbox_obj<'a>( scope: &mut v8::HandleScope, sandbox_obj: v8::Local, ) -> Option<&'a Self> { - let private_str = v8::String::new_from_onebyte_const(scope, &PRIVATE_SYMBOL_NAME); + let private_str = + v8::String::new_from_onebyte_const(scope, &PRIVATE_SYMBOL_NAME); let private_symbol = v8::Private::for_api(scope, private_str); - sandbox_obj.get_private(scope, private_symbol) - .and_then(|wrapper| deno_core::cppgc::try_unwrap_cppgc_object::(wrapper)) + sandbox_obj + .get_private(scope, private_symbol) + .and_then(|wrapper| { + deno_core::cppgc::try_unwrap_cppgc_object::(wrapper) + }) } - pub fn context<'a>(&self, scope: &mut v8::HandleScope<'a>) -> v8::Local<'a, v8::Context> { + pub fn context<'a>( + &self, + scope: &mut v8::HandleScope<'a>, + ) -> v8::Local<'a, v8::Context> { v8::Local::new(scope, &self.context) } @@ -125,7 +124,10 @@ impl ContextifyContext { ctx.global(scope) } - fn sandbox<'a>(&self, scope: &mut v8::HandleScope<'a>) -> v8::Local<'a, v8::Object> { + fn sandbox<'a>( + &self, + scope: &mut v8::HandleScope<'a>, + ) -> v8::Local<'a, v8::Object> { v8::Local::new(scope, &self.sandbox) } @@ -179,7 +181,6 @@ fn init_global_template<'a>( v8::Local::new(scope, object_template_slot.0) } - extern "C" fn c_noop(info: *const v8::FunctionCallbackInfo) {} thread_local! { @@ -228,17 +229,35 @@ fn init_global_template_inner(scope: &mut v8::HandleScope) { scope.set_slot(contextify_global_template_slot); } - fn property_getter<'s>( scope: &mut v8::HandleScope<'s>, key: v8::Local<'s, v8::Name>, args: v8::PropertyCallbackArguments<'s>, - mut rv: v8::ReturnValue, + mut ret: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()); + let ctx = ContextifyContext::get(scope, args.this()).unwrap(); let context = ctx.context(scope); let sandbox = ctx.sandbox(scope); + + let tc_scope = &mut v8::TryCatch::new(scope); + let maybe_rv = sandbox.get_real_named_property(tc_scope, key).or_else(|| { + ctx + .global_proxy(tc_scope) + .get_real_named_property(tc_scope, key) + }); + + if let Some(mut rv) = maybe_rv { + if tc_scope.has_caught() && !tc_scope.has_terminated() { + tc_scope.rethrow(); + } + + if rv == sandbox { + rv = ctx.global_proxy(tc_scope).into(); + } + + ret.set(rv); + } } fn property_setter<'s>( @@ -248,6 +267,87 @@ fn property_setter<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { + let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + + let context = ctx.context(scope); + let (attributes, is_declared_on_global_proxy) = match ctx + .global_proxy(scope) + .get_real_named_property_attributes(scope, key) + { + Some(attr) => (attr, true), + None => (v8::PropertyAttribute::NONE, false), + }; + let mut read_only = attributes.is_read_only(); + + let (attributes, is_declared_on_sandbox) = match ctx + .sandbox(scope) + .get_real_named_property_attributes(scope, key) + { + Some(attr) => (attr, true), + None => (v8::PropertyAttribute::NONE, false), + }; + read_only |= attributes.is_read_only(); + + if read_only { + return; + } + + // true for x = 5 + // false for this.x = 5 + // false for Object.defineProperty(this, 'foo', ...) + // false for vmResult.x = 5 where vmResult = vm.runInContext(); + let is_contextual_store = ctx.global_proxy(scope) != args.this(); + + // Indicator to not return before setting (undeclared) function declarations + // on the sandbox in strict mode, i.e. args.ShouldThrowOnError() = true. + // True for 'function f() {}', 'this.f = function() {}', + // 'var f = function()'. + // In effect only for 'function f() {}' because + // var f = function(), is_declared = true + // this.f = function() {}, is_contextual_store = false. + let is_function = value.is_function(); + + let is_declared = is_declared_on_global_proxy || is_declared_on_sandbox; + if !is_declared + && args.should_throw_on_error() + && is_contextual_store + && !is_function + { + return; + } + + if !is_declared && key.is_symbol() { + return; + }; + + if ctx.sandbox(scope).set(scope, key.into(), value).is_none() { + return; + } + + if is_declared_on_sandbox { + if let Some(desc) = + ctx.sandbox(scope).get_own_property_descriptor(scope, key) + { + if !desc.is_undefined() { + let desc_obj: v8::Local = desc.try_into().unwrap(); + // We have to specify the return value for any contextual or get/set + // property + let get_key = + v8::String::new_external_onebyte_static(scope, b"get").unwrap(); + let set_key = + v8::String::new_external_onebyte_static(scope, b"get").unwrap(); + if desc_obj + .has_own_property(scope, get_key.into()) + .unwrap_or(false) + || desc_obj + .has_own_property(scope, set_key.into()) + .unwrap_or(false) + { + rv.set(value); + } + } + } + } } fn property_deleter<'s>( @@ -256,7 +356,18 @@ fn property_deleter<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { + let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + + let context = ctx.context(scope); + let sandbox = ctx.sandbox(scope); + let context_scope = &mut v8::ContextScope::new(scope, context); + if !sandbox + .delete(context_scope, key.into()) + .unwrap_or(false) { + return; + } + rv.set_bool(false); } fn property_enumerator<'s>( From 3d76cbae1f03fcca9b89c7dd81be3044438420e0 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 3 Apr 2024 20:32:07 +0530 Subject: [PATCH 04/12] property callbacks --- ext/node/ops/vm_internal.rs | 194 +++++++++++++++++++++++++++++++++++- 1 file changed, 190 insertions(+), 4 deletions(-) diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 44020d4bf2ee3e..2a59f3a5d7bae6 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -192,6 +192,14 @@ thread_local! { pub static DESCRIPTOR_MAP_FN: v8::GenericNamedPropertyGetterCallback<'static> = property_descriptor.map_fn_to(); } +thread_local! { + pub static INDEXED_GETTER_MAP_FN: v8::IndexedPropertyGetterCallback<'static> = indexed_property_getter.map_fn_to(); + pub static INDEXED_SETTER_MAP_FN: v8::IndexedPropertySetterCallback<'static> = indexed_property_setter.map_fn_to(); + pub static INDEXED_DELETER_MAP_FN: v8::IndexedPropertyGetterCallback<'static> = indexed_property_deleter.map_fn_to(); + pub static INDEXED_DEFINER_MAP_FN: v8::IndexedPropertyDefinerCallback<'static> = indexed_property_definer.map_fn_to(); + pub static INDEXED_DESCRIPTOR_MAP_FN: v8::IndexedPropertyGetterCallback<'static> = indexed_property_descriptor.map_fn_to(); +} + fn init_global_template_inner(scope: &mut v8::HandleScope) { let global_func_template = v8::FunctionTemplate::builder_raw(c_noop).build(scope); @@ -216,6 +224,16 @@ fn init_global_template_inner(scope: &mut v8::HandleScope) { let indexed_property_handler_config = { let mut config = v8::IndexedPropertyHandlerConfiguration::new() .flags(v8::PropertyHandlerFlags::HAS_NO_SIDE_EFFECT); + + config = INDEXED_GETTER_MAP_FN.with(|getter| config.getter_raw(*getter)); + config = INDEXED_SETTER_MAP_FN.with(|setter| config.setter_raw(*setter)); + config = INDEXED_DELETER_MAP_FN.with(|deleter| config.deleter_raw(*deleter)); + config = + ENUMERATOR_MAP_FN.with(|enumerator| config.enumerator_raw(*enumerator)); + config = INDEXED_DEFINER_MAP_FN.with(|definer| config.definer_raw(*definer)); + config = + INDEXED_DESCRIPTOR_MAP_FN.with(|descriptor| config.descriptor_raw(*descriptor)); + config }; @@ -361,9 +379,7 @@ fn property_deleter<'s>( let context = ctx.context(scope); let sandbox = ctx.sandbox(scope); let context_scope = &mut v8::ContextScope::new(scope, context); - if !sandbox - .delete(context_scope, key.into()) - .unwrap_or(false) { + if !sandbox.delete(context_scope, key.into()).unwrap_or(false) { return; } @@ -375,15 +391,95 @@ fn property_enumerator<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { + let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + + let context = ctx.context(scope); + let sandbox = ctx.sandbox(scope); + let context_scope = &mut v8::ContextScope::new(scope, context); + let Some(properties) = sandbox + .get_property_names(context_scope, v8::GetPropertyNamesArgs::default()) + else { + return; + }; + + rv.set(properties.into()); } fn property_definer<'s>( scope: &mut v8::HandleScope<'s>, key: v8::Local<'s, v8::Name>, - descriptor: &v8::PropertyDescriptor, + desc: &v8::PropertyDescriptor, args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { + let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + + let context = ctx.context(scope); + let (attributes, is_declared) = match ctx + .global_proxy(scope) + .get_real_named_property_attributes(scope, key) + { + Some(attr) => (attr, true), + None => (v8::PropertyAttribute::NONE, false), + }; + + let read_only = attributes.is_read_only(); + let dont_delete = attributes.is_dont_delete(); + + // If the property is set on the global as read_only, don't change it on + // the global or sandbox. + if is_declared && read_only && dont_delete { + return; + } + + let sandbox = ctx.sandbox(scope); + let scope = &mut v8::ContextScope::new(scope, context); + + let mut define_prop_on_sandbox = |scope: &mut v8::HandleScope, desc_for_sandbox: &mut v8::PropertyDescriptor| { + if desc.has_enumerable() { + desc_for_sandbox.set_enumerable(desc.enumerable()); + } + + if desc.has_configurable() { + desc_for_sandbox.set_configurable(desc.configurable()); + } + + sandbox.define_property(scope, key.into(), desc_for_sandbox); + }; + + if desc.has_get() || desc.has_set() { + let mut desc_for_sandbox = v8::PropertyDescriptor::new_from_get_set( + if desc.has_get() { + desc.get() + } else { + v8::undefined(scope).into() + }, + if desc.has_set() { + desc.set() + } else { + v8::undefined(scope).into() + }, + ); + + define_prop_on_sandbox(scope, &mut desc_for_sandbox); + } else { + let value = if desc.has_value() { + desc.value() + } else { + v8::undefined(scope).into() + }; + + if desc.has_writable() { + let mut desc_for_sandbox = v8::PropertyDescriptor::new_from_value_writable( + value, + desc.writable(), + ); + define_prop_on_sandbox(scope, &mut desc_for_sandbox); + } else { + let mut desc_for_sandbox = v8::PropertyDescriptor::new_from_value(value); + define_prop_on_sandbox(scope, &mut desc_for_sandbox); + } + } } fn property_descriptor<'s>( @@ -392,4 +488,94 @@ fn property_descriptor<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { + let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + + let context = ctx.context(scope); + let sandbox = ctx.sandbox(scope); + let scope = &mut v8::ContextScope::new(scope, context); + + if sandbox.has_own_property(scope, key).unwrap_or(false) { + if let Some(desc) = sandbox.get_own_property_descriptor(scope, key) { + rv.set(desc); + } + } +} + +fn uint32_to_name<'s>( + scope: &mut v8::HandleScope<'s>, + index: u32, +) -> v8::Local<'s, v8::Name> { + let int = v8::Integer::new_from_unsigned(scope, index); + let u32 = v8::Local::::try_from(int).unwrap(); + u32.to_string(scope).unwrap().into() +} + +fn indexed_property_getter<'s>( + scope: &mut v8::HandleScope<'s>, + index: u32, + args: v8::PropertyCallbackArguments<'s>, + rv: v8::ReturnValue, +) { + let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + + let key = uint32_to_name(scope, index); + property_getter(scope, key, args, rv); +} + +fn indexed_property_setter<'s>( + scope: &mut v8::HandleScope<'s>, + index: u32, + value: v8::Local<'s, v8::Value>, + args: v8::PropertyCallbackArguments<'s>, + rv: v8::ReturnValue, +) { + let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + + let key = uint32_to_name(scope, index); + property_setter(scope, key, value, args, rv); +} + +fn indexed_property_deleter<'s>( + scope: &mut v8::HandleScope<'s>, + index: u32, + args: v8::PropertyCallbackArguments<'s>, + mut rv: v8::ReturnValue, +) { + let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + + let context = ctx.context(scope); + let sandbox = ctx.sandbox(scope); + let context_scope = &mut v8::ContextScope::new(scope, context); + if !sandbox.delete_index(context_scope, index).unwrap_or(false) { + return; + } + + // Delete failed on the sandbox, intercept and do not delete on + // the global object. + rv.set_bool(false); +} + +fn indexed_property_definer<'s>( + scope: &mut v8::HandleScope<'s>, + index: u32, + descriptor: &v8::PropertyDescriptor, + args: v8::PropertyCallbackArguments<'s>, + rv: v8::ReturnValue, +) { + let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + + let key = uint32_to_name(scope, index); + property_definer(scope, key, descriptor, args, rv); +} + +fn indexed_property_descriptor<'s>( + scope: &mut v8::HandleScope<'s>, + index: u32, + args: v8::PropertyCallbackArguments<'s>, + rv: v8::ReturnValue, +) { + let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + + let key = uint32_to_name(scope, index); + property_descriptor(scope, key, args, rv); } From 9a1da579c2954bc243a7d371e28a9d907d6c6ce2 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 3 Apr 2024 21:14:18 +0530 Subject: [PATCH 05/12] works --- ext/node/ops/vm.rs | 9 +++-- ext/node/ops/vm_internal.rs | 71 +++++++++++++++++++++++-------------- tests/unit_node/vm_test.ts | 34 ++++++++++++++++-- 3 files changed, 84 insertions(+), 30 deletions(-) diff --git a/ext/node/ops/vm.rs b/ext/node/ops/vm.rs index 3785c32a780ece..75e5dcd7584009 100644 --- a/ext/node/ops/vm.rs +++ b/ext/node/ops/vm.rs @@ -98,9 +98,14 @@ pub fn op_vm_create_context( #[op2] pub fn op_vm_is_context( scope: &mut v8::HandleScope, - sandbox_obj: v8::Local, + sandbox_obj: v8::Local, ) -> bool { - false + sandbox_obj + .try_into() + .map(|sandbox_obj| { + i::ContextifyContext::is_contextify_context(scope, sandbox_obj) + }) + .unwrap_or(false) } #[cfg(test)] diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 2a59f3a5d7bae6..56068fcf9ed8da 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -86,6 +86,15 @@ impl ContextifyContext { let sandbox = v8::Global::new(scope, sandbox_obj); let wrapper = deno_core::cppgc::make_cppgc_object(scope, Self { context, sandbox }); + let ptr = deno_core::cppgc::try_unwrap_cppgc_object::(wrapper.into()) + .unwrap(); + + unsafe { + v8_context.set_aligned_pointer_in_embedder_data( + 0, + ptr as *const ContextifyContext as _, + ); + } let private_str = v8::String::new_from_onebyte_const(scope, &PRIVATE_SYMBOL_NAME); @@ -109,6 +118,13 @@ impl ContextifyContext { }) } + pub fn is_contextify_context<'a>( + scope: &mut v8::HandleScope, + object: v8::Local, + ) -> bool { + Self::from_sandbox_obj(scope, object).is_some() + } + pub fn context<'a>( &self, scope: &mut v8::HandleScope<'a>, @@ -131,15 +147,16 @@ impl ContextifyContext { v8::Local::new(scope, &self.sandbox) } - fn get( - scope: &mut v8::HandleScope, - object: v8::Local, - ) -> Option { + fn get<'a, 'b, 'c>( + scope: &'b mut v8::HandleScope<'a>, + object: v8::Local<'a, v8::Object>, + ) -> Option<&'c ContextifyContext> { let Some(context) = object.get_creation_context(scope) else { return None; }; - context.get_slot::(scope).cloned() + let context_ptr = context.get_aligned_pointer_from_embedder_data(0); + Some(unsafe { &*(context_ptr as *const ContextifyContext) }) } } @@ -227,12 +244,14 @@ fn init_global_template_inner(scope: &mut v8::HandleScope) { config = INDEXED_GETTER_MAP_FN.with(|getter| config.getter_raw(*getter)); config = INDEXED_SETTER_MAP_FN.with(|setter| config.setter_raw(*setter)); - config = INDEXED_DELETER_MAP_FN.with(|deleter| config.deleter_raw(*deleter)); + config = + INDEXED_DELETER_MAP_FN.with(|deleter| config.deleter_raw(*deleter)); config = ENUMERATOR_MAP_FN.with(|enumerator| config.enumerator_raw(*enumerator)); - config = INDEXED_DEFINER_MAP_FN.with(|definer| config.definer_raw(*definer)); config = - INDEXED_DESCRIPTOR_MAP_FN.with(|descriptor| config.descriptor_raw(*descriptor)); + INDEXED_DEFINER_MAP_FN.with(|definer| config.definer_raw(*definer)); + config = INDEXED_DESCRIPTOR_MAP_FN + .with(|descriptor| config.descriptor_raw(*descriptor)); config }; @@ -435,17 +454,19 @@ fn property_definer<'s>( let sandbox = ctx.sandbox(scope); let scope = &mut v8::ContextScope::new(scope, context); - let mut define_prop_on_sandbox = |scope: &mut v8::HandleScope, desc_for_sandbox: &mut v8::PropertyDescriptor| { - if desc.has_enumerable() { - desc_for_sandbox.set_enumerable(desc.enumerable()); - } + let mut define_prop_on_sandbox = + |scope: &mut v8::HandleScope, + desc_for_sandbox: &mut v8::PropertyDescriptor| { + if desc.has_enumerable() { + desc_for_sandbox.set_enumerable(desc.enumerable()); + } - if desc.has_configurable() { - desc_for_sandbox.set_configurable(desc.configurable()); - } + if desc.has_configurable() { + desc_for_sandbox.set_configurable(desc.configurable()); + } - sandbox.define_property(scope, key.into(), desc_for_sandbox); - }; + sandbox.define_property(scope, key.into(), desc_for_sandbox); + }; if desc.has_get() || desc.has_set() { let mut desc_for_sandbox = v8::PropertyDescriptor::new_from_get_set( @@ -463,17 +484,15 @@ fn property_definer<'s>( define_prop_on_sandbox(scope, &mut desc_for_sandbox); } else { - let value = if desc.has_value() { - desc.value() - } else { - v8::undefined(scope).into() - }; + let value = if desc.has_value() { + desc.value() + } else { + v8::undefined(scope).into() + }; if desc.has_writable() { - let mut desc_for_sandbox = v8::PropertyDescriptor::new_from_value_writable( - value, - desc.writable(), - ); + let mut desc_for_sandbox = + v8::PropertyDescriptor::new_from_value_writable(value, desc.writable()); define_prop_on_sandbox(scope, &mut desc_for_sandbox); } else { let mut desc_for_sandbox = v8::PropertyDescriptor::new_from_value(value); diff --git a/tests/unit_node/vm_test.ts b/tests/unit_node/vm_test.ts index f8bc11b8237824..26d2e5bffe2a7a 100644 --- a/tests/unit_node/vm_test.ts +++ b/tests/unit_node/vm_test.ts @@ -1,5 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -import { isContext, runInNewContext } from "node:vm"; +import { createContext, isContext, runInContext, runInNewContext, Script } from "node:vm"; import { assertEquals, assertThrows } from "@std/assert/mod.ts"; Deno.test({ @@ -10,10 +10,25 @@ Deno.test({ }, }); +Deno.test({ + name: "vm new Script()", + fn() { + const script = new Script(` +function add(a, b) { + return a + b; +} +const x = add(1, 2); +x +`); + + const value = script.runInThisContext(); + assertEquals(value, 3); + }, +}); + Deno.test({ name: "vm runInNewContext sandbox", fn() { - assertThrows(() => runInNewContext("Deno")); // deno-lint-ignore no-var var a = 1; assertThrows(() => runInNewContext("a + 1")); @@ -23,6 +38,21 @@ Deno.test({ }, }); +Deno.test({ + name: "vm createContext", + fn() { + // @ts-expect-error implicit any + globalThis.globalVar = 3; + + const context = { globalVar: 1 }; + createContext(context); + runInContext("globalVar *= 2", context); + assertEquals(context.globalVar, 2); + // @ts-expect-error implicit any + assertEquals(globalThis.globalVar, 3); + }, +}); + // https://github.com/webpack/webpack/blob/87660921808566ef3b8796f8df61bd79fc026108/lib/javascript/JavascriptParser.js#L4329 Deno.test({ name: "vm runInNewContext webpack magic comments", From 6fd804cebfff86db7c005a5171516e2782b0af88 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 5 Apr 2024 15:54:40 +0530 Subject: [PATCH 06/12] re-entrant --- ext/node/ops/vm.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/node/ops/vm.rs b/ext/node/ops/vm.rs index 75e5dcd7584009..416dbf59a1d093 100644 --- a/ext/node/ops/vm.rs +++ b/ext/node/ops/vm.rs @@ -68,7 +68,7 @@ pub fn op_vm_create_script<'a>( Ok(deno_core::cppgc::make_cppgc_object(scope, script)) } -#[op2] +#[op2(reentrant)] pub fn op_vm_script_run_in_context<'a>( scope: &mut v8::HandleScope<'a>, #[cppgc] script: &Script, @@ -77,7 +77,7 @@ pub fn op_vm_script_run_in_context<'a>( script.run_in_context(scope, sandbox) } -#[op2] +#[op2(reentrant)] pub fn op_vm_script_run_in_this_context<'a>( scope: &'a mut v8::HandleScope, #[cppgc] script: &Script, From 5b932111e3412330d49aa40ab51a2e5aa2f2025b Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 5 Apr 2024 22:49:30 +0530 Subject: [PATCH 07/12] stuff --- ext/node/polyfills/vm.ts | 9 ++------- tests/unit_node/vm_test.ts | 8 +++++++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/ext/node/polyfills/vm.ts b/ext/node/polyfills/vm.ts index c5ced9cbedd827..83db0ddbeec073 100644 --- a/ext/node/polyfills/vm.ts +++ b/ext/node/polyfills/vm.ts @@ -6,11 +6,11 @@ import { core } from "ext:core/mod.js"; import { notImplemented } from "ext:deno_node/_utils.ts"; import { - op_vm_create_script, op_vm_create_context, + op_vm_create_script, + op_vm_is_context, op_vm_script_run_in_context, op_vm_script_run_in_this_context, - op_vm_is_context, } from "ext:core/ops"; export class Script { @@ -29,11 +29,6 @@ export class Script { } runInNewContext(contextObject: any, options: any) { - if (options) { - console.warn( - "Script.runInNewContext options are currently not supported", - ); - } const context = createContext(contextObject); return this.runInContext(context, options); } diff --git a/tests/unit_node/vm_test.ts b/tests/unit_node/vm_test.ts index 26d2e5bffe2a7a..94ddcbce405d65 100644 --- a/tests/unit_node/vm_test.ts +++ b/tests/unit_node/vm_test.ts @@ -1,5 +1,11 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -import { createContext, isContext, runInContext, runInNewContext, Script } from "node:vm"; +import { + createContext, + isContext, + runInContext, + runInNewContext, + Script, +} from "node:vm"; import { assertEquals, assertThrows } from "@std/assert/mod.ts"; Deno.test({ From 2c09b43e78b545e3bbf75633b2a791fa2b262913 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 5 Apr 2024 22:59:23 +0530 Subject: [PATCH 08/12] More tests --- ext/node/ops/vm.rs | 2 +- ext/node/ops/vm_internal.rs | 19 ++++--------------- tests/unit_node/vm_test.ts | 23 +++++++++++++++++------ 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/ext/node/ops/vm.rs b/ext/node/ops/vm.rs index 416dbf59a1d093..d6ed9594f45567 100644 --- a/ext/node/ops/vm.rs +++ b/ext/node/ops/vm.rs @@ -7,7 +7,7 @@ use deno_core::v8; use super::vm_internal as i; -struct Script { +pub struct Script { inner: i::ContextifyScript, } diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 56068fcf9ed8da..ed83256afaba52 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -35,7 +35,7 @@ impl ContextifyScript { pub fn eval_machine<'s>( &self, scope: &mut v8::HandleScope<'s>, - context: v8::Local, + _context: v8::Local, ) -> Option> { let tc_scope = &mut v8::TryCatch::new(scope); @@ -79,7 +79,6 @@ impl ContextifyContext { sandbox_obj: v8::Local, ) { let main_context = scope.get_current_context(); - let new_context_global = v8_context.global(scope); v8_context.set_security_token(main_context.get_security_token(scope)); let context = v8::Global::new(scope, v8_context); @@ -198,7 +197,7 @@ fn init_global_template<'a>( v8::Local::new(scope, object_template_slot.0) } -extern "C" fn c_noop(info: *const v8::FunctionCallbackInfo) {} +extern "C" fn c_noop(_: *const v8::FunctionCallbackInfo) {} thread_local! { pub static GETTER_MAP_FN: v8::GenericNamedPropertyGetterCallback<'static> = property_getter.map_fn_to(); @@ -274,7 +273,6 @@ fn property_getter<'s>( ) { let ctx = ContextifyContext::get(scope, args.this()).unwrap(); - let context = ctx.context(scope); let sandbox = ctx.sandbox(scope); let tc_scope = &mut v8::TryCatch::new(scope); @@ -306,7 +304,6 @@ fn property_setter<'s>( ) { let ctx = ContextifyContext::get(scope, args.this()).unwrap(); - let context = ctx.context(scope); let (attributes, is_declared_on_global_proxy) = match ctx .global_proxy(scope) .get_real_named_property_attributes(scope, key) @@ -429,7 +426,7 @@ fn property_definer<'s>( key: v8::Local<'s, v8::Name>, desc: &v8::PropertyDescriptor, args: v8::PropertyCallbackArguments<'s>, - mut rv: v8::ReturnValue, + _: v8::ReturnValue, ) { let ctx = ContextifyContext::get(scope, args.this()).unwrap(); @@ -454,7 +451,7 @@ fn property_definer<'s>( let sandbox = ctx.sandbox(scope); let scope = &mut v8::ContextScope::new(scope, context); - let mut define_prop_on_sandbox = + let define_prop_on_sandbox = |scope: &mut v8::HandleScope, desc_for_sandbox: &mut v8::PropertyDescriptor| { if desc.has_enumerable() { @@ -535,8 +532,6 @@ fn indexed_property_getter<'s>( args: v8::PropertyCallbackArguments<'s>, rv: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()).unwrap(); - let key = uint32_to_name(scope, index); property_getter(scope, key, args, rv); } @@ -548,8 +543,6 @@ fn indexed_property_setter<'s>( args: v8::PropertyCallbackArguments<'s>, rv: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()).unwrap(); - let key = uint32_to_name(scope, index); property_setter(scope, key, value, args, rv); } @@ -581,8 +574,6 @@ fn indexed_property_definer<'s>( args: v8::PropertyCallbackArguments<'s>, rv: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()).unwrap(); - let key = uint32_to_name(scope, index); property_definer(scope, key, descriptor, args, rv); } @@ -593,8 +584,6 @@ fn indexed_property_descriptor<'s>( args: v8::PropertyCallbackArguments<'s>, rv: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()).unwrap(); - let key = uint32_to_name(scope, index); property_descriptor(scope, key, args, rv); } diff --git a/tests/unit_node/vm_test.ts b/tests/unit_node/vm_test.ts index 94ddcbce405d65..33bc82b46d4b29 100644 --- a/tests/unit_node/vm_test.ts +++ b/tests/unit_node/vm_test.ts @@ -32,20 +32,30 @@ x }, }); +// https://github.com/denoland/deno/issues/23186 Deno.test({ name: "vm runInNewContext sandbox", fn() { - // deno-lint-ignore no-var - var a = 1; - assertThrows(() => runInNewContext("a + 1")); + const sandbox = { fromAnotherRealm: false }; + runInNewContext("fromAnotherRealm = {}", sandbox); - runInNewContext("a = 2"); - assertEquals(a, 1); + assertEquals(typeof sandbox.fromAnotherRealm, "object"); }, }); +// https://github.com/denoland/deno/issues/22395 Deno.test({ - name: "vm createContext", + name: "vm runInewContext with context object", + fn() { + const context = { a: 1, b: 2 }; + const result = runInNewContext("a + b", context); + assertEquals(result, 3); + }, +}); + +// https://github.com/denoland/deno/issues/18299 +Deno.test({ + name: "vm createContext and runInContext", fn() { // @ts-expect-error implicit any globalThis.globalVar = 3; @@ -89,6 +99,7 @@ Deno.test({ }, }); +// https://github.com/denoland/deno/issues/18315 Deno.test({ name: "vm isContext", fn() { From 9b46e8f2006d57af6b30b7a0641f11d60deb5be2 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 5 Apr 2024 23:06:04 +0530 Subject: [PATCH 09/12] lint --- ext/node/ops/vm_internal.rs | 13 +++++++++---- ext/node/polyfills/vm.ts | 3 --- tests/unit_node/vm_test.ts | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index ed83256afaba52..91b4e777c5693c 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -88,6 +88,9 @@ impl ContextifyContext { let ptr = deno_core::cppgc::try_unwrap_cppgc_object::(wrapper.into()) .unwrap(); + // SAFETY: We are storing a pointer to the ContextifyContext + // in the embedder data of the v8::Context. The contextified wrapper + // lives longer than the execution context, so this should be safe. unsafe { v8_context.set_aligned_pointer_in_embedder_data( 0, @@ -117,7 +120,7 @@ impl ContextifyContext { }) } - pub fn is_contextify_context<'a>( + pub fn is_contextify_context( scope: &mut v8::HandleScope, object: v8::Local, ) -> bool { @@ -146,8 +149,8 @@ impl ContextifyContext { v8::Local::new(scope, &self.sandbox) } - fn get<'a, 'b, 'c>( - scope: &'b mut v8::HandleScope<'a>, + fn get<'a, 'c>( + scope: &mut v8::HandleScope<'a>, object: v8::Local<'a, v8::Object>, ) -> Option<&'c ContextifyContext> { let Some(context) = object.get_creation_context(scope) else { @@ -155,6 +158,8 @@ impl ContextifyContext { }; let context_ptr = context.get_aligned_pointer_from_embedder_data(0); + // SAFETY: We are storing a pointer to the ContextifyContext + // in the embedder data of the v8::Context during creation. Some(unsafe { &*(context_ptr as *const ContextifyContext) }) } } @@ -462,7 +467,7 @@ fn property_definer<'s>( desc_for_sandbox.set_configurable(desc.configurable()); } - sandbox.define_property(scope, key.into(), desc_for_sandbox); + sandbox.define_property(scope, key, desc_for_sandbox); }; if desc.has_get() || desc.has_set() { diff --git a/ext/node/polyfills/vm.ts b/ext/node/polyfills/vm.ts index 83db0ddbeec073..6150d39024633a 100644 --- a/ext/node/polyfills/vm.ts +++ b/ext/node/polyfills/vm.ts @@ -1,8 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -// TODO(petamoriken): enable prefer-primordials for node polyfills -// deno-lint-ignore-file no-explicit-any prefer-primordials - import { core } from "ext:core/mod.js"; import { notImplemented } from "ext:deno_node/_utils.ts"; import { diff --git a/tests/unit_node/vm_test.ts b/tests/unit_node/vm_test.ts index 33bc82b46d4b29..3f398bbc669e3d 100644 --- a/tests/unit_node/vm_test.ts +++ b/tests/unit_node/vm_test.ts @@ -1,4 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +import { assertEquals } from "@std/assert/mod.ts"; import { createContext, isContext, @@ -6,7 +7,6 @@ import { runInNewContext, Script, } from "node:vm"; -import { assertEquals, assertThrows } from "@std/assert/mod.ts"; Deno.test({ name: "vm runInNewContext", From c71bbb4ce09d7fd8ef3344f7aa18569fcf2e2b39 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 5 Apr 2024 23:10:56 +0530 Subject: [PATCH 10/12] lint js --- ext/node/polyfills/vm.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/node/polyfills/vm.ts b/ext/node/polyfills/vm.ts index 6150d39024633a..d792c09fe9a89a 100644 --- a/ext/node/polyfills/vm.ts +++ b/ext/node/polyfills/vm.ts @@ -1,6 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -import { core } from "ext:core/mod.js"; +// deno-lint-ignore-file no-explicit-any + import { notImplemented } from "ext:deno_node/_utils.ts"; import { op_vm_create_context, From 1dbafb8f0c31f93130d27bc8baf1d0a625d856b7 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Sat, 6 Apr 2024 09:17:24 +0530 Subject: [PATCH 11/12] Fix rethrow --- ext/node/ops/vm.rs | 4 ++-- ext/node/ops/vm_internal.rs | 1 + tests/unit_node/vm_test.ts | 23 ++++++++++++++++++++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/ext/node/ops/vm.rs b/ext/node/ops/vm.rs index d6ed9594f45567..bbb307553fc722 100644 --- a/ext/node/ops/vm.rs +++ b/ext/node/ops/vm.rs @@ -32,7 +32,7 @@ impl Script { let result = self .inner .eval_machine(&mut scope, context) - .ok_or_else(|| type_error("Failed to run script"))?; + .unwrap_or_else(|| v8::undefined(&mut scope).into()); Ok(scope.escape(result)) } @@ -54,7 +54,7 @@ impl Script { let result = self .inner .eval_machine(&mut scope, context) - .ok_or_else(|| type_error("Failed to run script"))?; + .unwrap_or_else(|| v8::undefined(&mut scope).into()); Ok(scope.escape(result)) } } diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 91b4e777c5693c..9aacf094c67a8f 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -45,6 +45,7 @@ impl ContextifyScript { let result = script.run(tc_scope); if tc_scope.has_caught() { + // If there was an exception thrown during script execution, re-throw it. if !tc_scope.has_terminated() { tc_scope.rethrow(); } diff --git a/tests/unit_node/vm_test.ts b/tests/unit_node/vm_test.ts index 3f398bbc669e3d..b557350ad5e417 100644 --- a/tests/unit_node/vm_test.ts +++ b/tests/unit_node/vm_test.ts @@ -1,10 +1,11 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -import { assertEquals } from "@std/assert/mod.ts"; +import { assertEquals, assertThrows } from "@std/assert/mod.ts"; import { createContext, isContext, runInContext, runInNewContext, + runInThisContext, Script, } from "node:vm"; @@ -69,6 +70,26 @@ Deno.test({ }, }); +Deno.test({ + name: "vm runInThisContext Error rethrow", + fn() { + assertThrows( + () => { + runInThisContext("throw new Error('error')"); + }, + Error, + "error", + ); + assertThrows( + () => { + runInThisContext("throw new TypeError('type error')"); + }, + TypeError, + "type error", + ); + }, +}); + // https://github.com/webpack/webpack/blob/87660921808566ef3b8796f8df61bd79fc026108/lib/javascript/JavascriptParser.js#L4329 Deno.test({ name: "vm runInNewContext webpack magic comments", From b58615ea4ed64941846a21627f19c20f233b507e Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Mon, 8 Apr 2024 08:44:25 +0530 Subject: [PATCH 12/12] Review --- ext/node/ops/vm.rs | 4 ++++ ext/node/ops/vm_internal.rs | 4 ++++ ext/node/polyfills/vm.ts | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/ext/node/ops/vm.rs b/ext/node/ops/vm.rs index bbb307553fc722..f18038f8f05ab2 100644 --- a/ext/node/ops/vm.rs +++ b/ext/node/ops/vm.rs @@ -91,6 +91,10 @@ pub fn op_vm_create_context( sandbox_obj: v8::Local, ) { // Don't allow contextifying a sandbox multiple times. + assert!(!i::ContextifyContext::is_contextify_context( + scope, + sandbox_obj + )); i::ContextifyContext::attach(scope, sandbox_obj); } diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 9aacf094c67a8f..274fac91a4345b 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -32,6 +32,7 @@ impl ContextifyScript { Ok(Self { script }) } + // TODO(littledivy): Support `options` pub fn eval_machine<'s>( &self, scope: &mut v8::HandleScope<'s>, @@ -205,6 +206,9 @@ fn init_global_template<'a>( extern "C" fn c_noop(_: *const v8::FunctionCallbackInfo) {} +// Using thread_local! to get around compiler bug. +// +// See NOTE in ext/node/global.rs#L12 thread_local! { pub static GETTER_MAP_FN: v8::GenericNamedPropertyGetterCallback<'static> = property_getter.map_fn_to(); pub static SETTER_MAP_FN: v8::GenericNamedPropertySetterCallback<'static> = property_setter.map_fn_to(); diff --git a/ext/node/polyfills/vm.ts b/ext/node/polyfills/vm.ts index d792c09fe9a89a..3378e38862663a 100644 --- a/ext/node/polyfills/vm.ts +++ b/ext/node/polyfills/vm.ts @@ -37,6 +37,10 @@ export class Script { } export function createContext(contextObject: any = {}, _options: any) { + if (isContext(contextObject)) { + return contextObject; + } + op_vm_create_context(contextObject); return contextObject; }