diff --git a/moli-protocol/src/domains/runtime/tests/console.rs b/moli-protocol/src/domains/runtime/tests/console.rs index cc87c820f..ee896a560 100644 --- a/moli-protocol/src/domains/runtime/tests/console.rs +++ b/moli-protocol/src/domains/runtime/tests/console.rs @@ -1310,6 +1310,88 @@ async fn runtime_console_api_called_preserves_basic_argument_shapes() { assert_eq!(args[11]["unserializableValue"], json!("1n")); } +#[tokio::test(flavor = "multi_thread")] +async fn runtime_console_capture_does_not_run_page_hooks_and_keeps_remote_objects() { + let mut ctx = TestContext::new(); + with_loaded_document_async(&mut ctx, "").await; + + // Exercise both states: reporting must be safe even before an Inspector + // Runtime session exists; enabling it must retain real remote object ids. + for (index, enabled) in [false, true].into_iter().enumerate() { + if enabled { + enable_runtime_and_take_execution_context_id_async(&mut ctx, 206_850).await; + } + ctx.sent.clear(); + let command_id = 206_851 + index as u64; + ctx.process_async(json!({ + "id": command_id, + "method": "Runtime.evaluate", + "params": {"expression": r#" + (() => { + const hits = []; + const object = { + answer: 42, + get getter() { hits.push('getter'); return 1; }, + toJSON() { hits.push('toJSON'); return {}; }, + [Symbol.toPrimitive]() { hits.push('toPrimitive'); return 'object'; } + }; + const proxy = new Proxy({}, { + get() { hits.push('Proxy.get'); }, + ownKeys() { hits.push('Proxy.ownKeys'); return []; }, + getOwnPropertyDescriptor() { hits.push('Proxy.descriptor'); } + }); + const error = new Error('console'); + Object.defineProperty(error, 'stack', {get() { hits.push('stack'); return 'stack'; }}); + console.log('safe-capture', object, proxy, error); + return JSON.stringify(hits); + })() + "#} + })).await; + let response = take_response_by_id(&mut ctx, command_id); + assert_eq!( + response["result"]["result"]["value"], + json!("[]"), + "{response}" + ); + if !enabled { + continue; + } + + wait_until_message(&mut ctx, None, "safe console object", |message| { + message["method"] == json!("Runtime.consoleAPICalled") + && message["params"]["args"][0]["value"] == json!("safe-capture") + }) + .await; + let object_id = ctx + .sent + .iter() + .find(|message| { + message["method"] == json!("Runtime.consoleAPICalled") + && message["params"]["args"][0]["value"] == json!("safe-capture") + }) + .unwrap()["params"]["args"][1]["objectId"] + .as_str() + .expect("native object id") + .to_owned(); + ctx.process_async(json!({ + "id": 206_853, + "method": "Runtime.getProperties", + "params": {"objectId": object_id, "ownProperties": true} + })) + .await; + let properties = take_response_by_id(&mut ctx, 206_853); + let properties = properties["result"]["result"] + .as_array() + .expect("inspectable object"); + assert!(properties.iter().any(|property| { + property["name"] == json!("answer") && property["value"]["value"] == json!(42) + })); + assert!(properties.iter().any(|property| { + property["name"] == json!("getter") && property["get"]["type"] == json!("function") + })); + } +} + #[tokio::test(flavor = "multi_thread")] async fn runtime_console_error_does_not_invoke_error_prepare_stack_trace() { let mut ctx = TestContext::new(); diff --git a/moli-renderer-v8/src/context_bootstrap/shared/console.rs b/moli-renderer-v8/src/context_bootstrap/shared/console.rs index dbdfbcf02..a85e93f0c 100644 --- a/moli-renderer-v8/src/context_bootstrap/shared/console.rs +++ b/moli-renderer-v8/src/context_bootstrap/shared/console.rs @@ -41,24 +41,16 @@ pub(in crate::context_bootstrap) fn append_console_message<'s>( level: &str, ) { let mut parts = Vec::with_capacity(args.length().max(0) as usize); + let mut arg_snapshot_values = Vec::with_capacity(parts.capacity()); for index in 0..args.length() { - let value = args.get(index); - let text = value - .to_string(scope) - .map(|value| value.to_rust_string_lossy(scope)) - .unwrap_or_else(|| String::from("undefined")); - parts.push(text); + let snapshot = console_arg_remote_object_json(scope, args.get(index)); + parts.push(console_arg_text(&snapshot)); + arg_snapshot_values.push(snapshot); } let text = parts.join(" "); let message = format!("{level}: {text}"); let stack = current_console_stack(scope); - let mut arg_snapshot_values = Vec::with_capacity(args.length().max(0) as usize); - for index in 0..args.length() { - let value = args.get(index); - arg_snapshot_values.push(console_arg_remote_object_json(scope, value)); - } - if let Some(buffers) = current_console_message_buffers(scope) { let mut buffers = buffers.borrow_mut(); buffers.messages.push(message.clone()); @@ -146,10 +138,9 @@ pub(crate) fn console_arg_remote_object_json( return serde_json::json!({ "type": "number" }); } if value.is_string() { - let value = value - .to_string(scope) - .map(|value| value.to_rust_string_lossy(scope)) - .unwrap_or_default(); + let value = v8::Local::::try_from(value) + .expect("string console argument") + .to_rust_string_lossy(scope); return serde_json::json!({ "type": "string", "value": value, @@ -158,17 +149,25 @@ pub(crate) fn console_arg_remote_object_json( if value.is_function() { return serde_json::json!({ "type": "function", - "description": value_description(scope, value), + "description": console_value_description(scope, value), }); } if value.is_symbol() { + let symbol = v8::Local::::try_from(value).expect("symbol console argument"); + let description = v8::Local::::try_from(symbol.description(scope)) + .map(|description| description.to_rust_string_lossy(scope)) + .unwrap_or_default(); return serde_json::json!({ "type": "symbol", - "description": value_description(scope, value), + "description": format!("Symbol({description})"), }); } if value.is_big_int() { - let mut description = value_description(scope, value); + // ToString of a primitive BigInt cannot invoke author conversion hooks. + let mut description = value + .to_string(scope) + .map(|value| value.to_rust_string_lossy(scope)) + .unwrap_or_default(); description.push('n'); return serde_json::json!({ "type": "bigint", @@ -176,50 +175,70 @@ pub(crate) fn console_arg_remote_object_json( }); } + // This is the renderer-owned reporting snapshot, not the Inspector's + // RemoteObject. The original V8 console still supplies inspectable objectIds + // to CDP. Never serialize/coerce objects here: getters, toJSON, conversion + // hooks and Proxy traps belong to the page, not to log bookkeeping. + let subtype = if value.is_proxy() { + Some("proxy") + } else if value.is_array() { + Some("array") + } else if value.is_native_error() { + Some("error") + } else if value.is_reg_exp() { + Some("regexp") + } else if value.is_date() { + Some("date") + } else if value.is_promise() { + Some("promise") + } else if value.is_map() { + Some("map") + } else if value.is_set() { + Some("set") + } else if value.is_typed_array() { + Some("typedarray") + } else if value.is_array_buffer() { + Some("arraybuffer") + } else { + None + }; let mut object = serde_json::json!({ - "type": "object", - "description": value_description(scope, value), + "type": "object", "description": console_value_description(scope, value) }); - if let Some(serialized) = json_serializable_console_value(scope, value) - && let Some(object) = object.as_object_mut() - { - object.insert("value".to_owned(), serialized); - } - if value.is_array() - && let Some(object) = object.as_object_mut() - { - object.insert( - "subtype".to_owned(), - serde_json::Value::String("array".to_owned()), - ); + if let Some(subtype) = subtype { + object["subtype"] = serde_json::json!(subtype); } object } -fn json_serializable_console_value( +fn console_value_description( scope: &mut v8::PinScope<'_, '_>, value: v8::Local<'_, v8::Value>, -) -> Option { - let json = { - let try_catch = std::pin::pin!(v8::TryCatch::new(scope)); - let scope = try_catch.init(); - let body = v8::json::stringify(&scope, value)?; - let body = body.to_rust_string_lossy(&scope); - if body == "undefined" { - return None; - } - body - }; - serde_json::from_str(&json).ok() -} - -fn value_description(scope: &mut v8::PinScope<'_, '_>, value: v8::Local<'_, v8::Value>) -> String { +) -> String { + // V8 implements ToDetailString with NoSideEffectsToString under a + // no-script scope. Unlike ToString, this cannot call author conversion + // hooks, but still retains useful native Error/function descriptions. value - .to_string(scope) + .to_detail_string(scope) .map(|value| value.to_rust_string_lossy(scope)) .unwrap_or_default() } +fn console_arg_text(snapshot: &serde_json::Value) -> String { + if let Some(value) = snapshot.get("value") { + return value + .as_str() + .map(str::to_owned) + .unwrap_or_else(|| value.to_string()); + } + snapshot + .get("description") + .or_else(|| snapshot.get("unserializableValue")) + .and_then(serde_json::Value::as_str) + .unwrap_or("undefined") + .to_owned() +} + fn record_runtime_observable_console_source_event( scope: &mut v8::PinScope<'_, '_>, message: String, diff --git a/moli-renderer-v8/src/context_bootstrap/window_events/console.rs b/moli-renderer-v8/src/context_bootstrap/window_events/console.rs index fc44362d1..1422c63a1 100644 --- a/moli-renderer-v8/src/context_bootstrap/window_events/console.rs +++ b/moli-renderer-v8/src/context_bootstrap/window_events/console.rs @@ -153,7 +153,7 @@ fn call_original_console_method<'s>( forwarded_args.push(args.get(index)); } let suppress_page_stack_hook = forwarded_args.iter().any(|value| value.is_native_error()) - && error_prepare_stack_trace_is_function(scope); + && error_prepare_stack_trace_has_page_hook(scope); if suppress_page_stack_hook { scope.set_prepare_stack_trace_callback(inspector_console_stack_without_page_hook); } @@ -163,13 +163,52 @@ fn call_original_console_method<'s>( } } -fn error_prepare_stack_trace_is_function(scope: &mut v8::PinScope<'_, '_>) -> bool { +fn error_prepare_stack_trace_has_page_hook(scope: &mut v8::PinScope<'_, '_>) -> bool { let global = scope.get_current_context().global(scope); - global - .get(scope, v8str(scope, "Error").into()) + let Some(descriptor) = own_property_descriptor(scope, global, "Error") else { + return false; + }; + // Inspect descriptor data, never the property itself. Looking for a page + // hook must not invoke an Error/prepareStackTrace accessor or Proxy trap. + if own_descriptor_value(scope, descriptor, "get").is_some_and(|v| v.is_function()) { + return true; + } + let Some(error) = own_descriptor_value(scope, descriptor, "value") + .and_then(|value| v8::Local::::try_from(value).ok()) + else { + return false; + }; + if error.is_proxy() { + return true; + } + let Some(descriptor) = own_property_descriptor(scope, error, "prepareStackTrace") else { + return false; + }; + ["value", "get"].into_iter().any(|key| { + own_descriptor_value(scope, descriptor, key).is_some_and(|value| value.is_function()) + }) +} + +fn own_property_descriptor<'s>( + scope: &mut v8::PinScope<'s, '_>, + object: v8::Local<'s, v8::Object>, + name: &'static str, +) -> Option> { + object + .get_own_property_descriptor(scope, v8str(scope, name).into()) .and_then(|value| v8::Local::::try_from(value).ok()) - .and_then(|error| error.get(scope, v8str(scope, "prepareStackTrace").into())) - .is_some_and(|value| value.is_function()) +} + +fn own_descriptor_value<'s>( + scope: &mut v8::PinScope<'s, '_>, + descriptor: v8::Local<'s, v8::Object>, + name: &'static str, +) -> Option> { + let key = v8str(scope, name); + if descriptor.has_own_property(scope, key.into()) != Some(true) { + return None; + } + descriptor.get(scope, key.into()) } fn inspector_console_stack_without_page_hook<'s>( diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/console.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/console.rs new file mode 100644 index 000000000..68aec6a15 --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/console.rs @@ -0,0 +1,82 @@ +use super::*; + +#[test] +fn console_reporting_does_not_add_page_object_conversions() { + let mut vm = new_storage_test_vm("https://console-reporting.test/"); + let result = vm.eval(r#" +(() => { + const hits = []; + let functionConversions = 0; + const object = { + get property() { hits.push('getter'); return 1; }, + toJSON() { hits.push('toJSON'); return {}; }, + toString() { hits.push('toString'); return 'object'; }, + [Symbol.toPrimitive]() { hits.push('toPrimitive'); return 'object'; } + }; + const proxy = new Proxy({}, { + get() { hits.push('Proxy.get'); }, + ownKeys() { hits.push('Proxy.ownKeys'); return []; }, + getOwnPropertyDescriptor() { hits.push('Proxy.getOwnPropertyDescriptor'); } + }); + const fn = function named() {}; + fn.toString = () => { functionConversions++; return 'function'; }; + const error = new Error('console'); + Object.defineProperty(error, 'stack', {enumerable: true, get() { hits.push('stack'); return 'stack'; }}); + for (const method of ['log','info','warn','error','debug','trace']) { + console[method](object, proxy, fn, error, Symbol('console'), 1n); + } + // Chromium's original V8 console converts a function once per call. The + // renderer reporting copy must not add another conversion to that path. + return JSON.stringify([hits, functionConversions]); +})() +"#).unwrap(); + assert_eq!(result, "[[],6]"); +} + +#[test] +fn console_reporting_cannot_turn_a_throwing_conversion_into_a_page_exception() { + let mut vm = new_storage_test_vm("https://console-reporting.test/"); + assert_eq!( + vm.eval( + r#" +const object = { + [Symbol.toPrimitive]() { throw new Error('conversion must not run'); }, + toJSON() { throw new Error('serialization must not run'); } +}; +console.log(object); +'continued' +"# + ) + .unwrap(), + "continued" + ); +} + +#[test] +fn console_reporting_does_not_read_a_page_prepare_stack_trace_getter() { + let mut vm = new_storage_test_vm("https://console-reporting.test/"); + let result = vm + .eval( + r#" +(() => { + let hits = 0; + const original = Object.getOwnPropertyDescriptor(Error, 'prepareStackTrace'); + Object.defineProperty(Error, 'prepareStackTrace', { + configurable: true, + get() { hits++; return () => 'page stack'; } + }); + try { + console.log(new Error('console')); + const afterConsole = hits; + void new Error('explicit stack').stack; + return `${afterConsole}|${hits}`; + } finally { + delete Error.prepareStackTrace; + if (original) Object.defineProperty(Error, 'prepareStackTrace', original); + } +})() +"#, + ) + .unwrap(); + assert_eq!(result, "0|1"); +} diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs index 612cf6dcd..91d9730e4 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs @@ -2,6 +2,7 @@ use super::*; mod broadcast_channel; mod chrome; +mod console; mod crypto_misc; mod crypto_subtle_aes; mod crypto_subtle_digest; diff --git a/moli-renderer-v8/src/worker/thread/tests/postmessage.rs b/moli-renderer-v8/src/worker/thread/tests/postmessage.rs index 23ad063b7..e174391c6 100644 --- a/moli-renderer-v8/src/worker/thread/tests/postmessage.rs +++ b/moli-renderer-v8/src/worker/thread/tests/postmessage.rs @@ -33,6 +33,39 @@ async fn worker_compression_streams_roundtrip_all_formats() { ); } +#[tokio::test] +async fn worker_console_capture_does_not_serialize_or_coerce_page_objects() { + ensure_v8(); + let mut handle = spawn_worker( + r#" + let hits = 0; + const object = { + get property() { hits++; return 1; }, + toJSON() { hits++; return {}; }, + [Symbol.toPrimitive]() { hits++; return 'object'; } + }; + const proxy = new Proxy({}, { + get() { hits++; }, ownKeys() { hits++; return []; } + }); + console.log(object, proxy); + postMessage(hits); + "# + .into(), + "https://console-worker.test/worker.js".into(), + ); + loop { + let message = timeout(TIMEOUT, handle.recv()) + .await + .expect("worker console probe") + .expect("worker result"); + if matches!(message, WorkerToParentMessage::Console(_)) { + continue; + } + assert_eq!(expect_post_json(message), "0"); + break; + } +} + #[tokio::test] async fn worker_postmessage_to_parent() { ensure_v8();