From aeabb8b9e826bd5f13bd75111aaa638ab6d0c6bc Mon Sep 17 00:00:00 2001 From: ldm0 Date: Fri, 9 Oct 2026 20:34:28 +0800 Subject: [PATCH] fix(cssom): convert declaration values before mutation Perform WebIDL string conversion before mutating live, detached, rule, and computed CSS declarations. Preserve thrown coercion values and run conversion before readonly errors; re-resolve native state after reentrant author code. Regressions cover named attributes, cssText, setProperty ordering, null and undefined, and exception identity. Resolve regression bindings from actual CSS object prototypes so legacy popup facades are exercised without requiring the separate popup-global constructor feature. --- moli-renderer-v8/src/css_style.rs | 30 ++++ moli-renderer-v8/src/detached_css_style.rs | 28 +--- .../native_bridge/element/styles/accessors.rs | 23 +-- .../native_bridge/element/styles/methods.rs | 3 - .../src/native_bridge/element/styles/named.rs | 36 +++-- .../script_vm/tests/dom_xhr/computed_style.rs | 2 + .../computed_style/cssom_write_conversion.js | 152 ++++++++++++++++++ .../computed_style/cssom_write_conversion.rs | 18 +++ 8 files changed, 244 insertions(+), 48 deletions(-) create mode 100644 moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.js create mode 100644 moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.rs diff --git a/moli-renderer-v8/src/css_style.rs b/moli-renderer-v8/src/css_style.rs index 8ee2496923..f784c751c3 100644 --- a/moli-renderer-v8/src/css_style.rs +++ b/moli-renderer-v8/src/css_style.rs @@ -38,6 +38,36 @@ pub(crate) struct CssStyleDeclarationItemArgs { pub(crate) index: u32, } +#[derive(webidl::WebIdlArgs)] +#[webidl(prefix = "CSSStyleDeclaration.cssText")] +pub(crate) struct CssStyleDeclarationCssTextArgs { + // An attribute setter converts undefined even when called with no argument. + #[webidl(default = "undefined", treat_null_as_empty_string)] + pub(crate) value: String, +} + +/// Named interceptors receive a value rather than function arguments. Use the +/// same WebIDL string conversion as the declaration's generated arguments. +pub(crate) fn css_style_declaration_property_value<'s>( + scope: &mut v8::PinScope<'s, '_>, + value: v8::Local<'s, v8::Value>, +) -> Option { + match webidl::convert_with_options::( + scope, + value, + webidl::Context::argument("CSSStyleDeclaration", 1), + &webidl::StringOptions { + treat_null_as_empty_string: true, + }, + ) { + Ok(value) => Some(value.0), + Err(error) => { + webidl::throw_error(scope, &error); + None + } + } +} + #[derive(webidl::WebIdlArgs)] #[webidl(prefix = "CSSStyleDeclaration.setProperty")] pub(crate) struct CssStyleDeclarationSetPropertyArgs { diff --git a/moli-renderer-v8/src/detached_css_style.rs b/moli-renderer-v8/src/detached_css_style.rs index 72e1de03c8..c39dfd098a 100644 --- a/moli-renderer-v8/src/detached_css_style.rs +++ b/moli-renderer-v8/src/detached_css_style.rs @@ -2780,19 +2780,15 @@ fn style_css_text_setter_callback<'s>( crate::native_bridge::element::style_css_text_setter_callback(scope, args, _rv); return; } - let value = args.get(0); let Some(style) = lightweight_style_receiver(scope, args.this()) else { return; }; - let css_text = if value.is_null_or_undefined() { - String::new() - } else { - value - .to_string(scope) - .map(|value| value.to_rust_string_lossy(scope)) - .unwrap_or_default() + let Some(parsed) = + webidl::parse_args::(scope, &args) + else { + return; }; - let _ = set_lightweight_css_style_css_text(scope, style, &css_text); + let _ = set_lightweight_css_style_css_text(scope, style, &parsed.value); } fn style_named_property_getter_callback<'s>( @@ -2858,17 +2854,12 @@ fn style_named_property_setter_callback<'s>( ); return; } - let value = if value.is_null_or_undefined() { - String::new() - } else { - value - .to_string(scope) - .map(|value| value.to_rust_string_lossy(scope)) - .unwrap_or_default() - }; let Some(style) = lightweight_style_receiver(scope, args.this()) else { return; }; + let Some(value) = crate::css_style::css_style_declaration_property_value(scope, value) else { + return; + }; set_style_entry(scope, style, &name, &value, false); notify_style_changed(scope, style); } @@ -2886,9 +2877,6 @@ fn style_set_property_callback<'s>( crate::native_bridge::element::style_set_property_callback(scope, args, _rv); return; } - if args.length() > 1 && args.get(1).is_undefined() { - return; - } let Some(parsed) = webidl::parse_args::(scope, &args) else { return; diff --git a/moli-renderer-v8/src/native_bridge/element/styles/accessors.rs b/moli-renderer-v8/src/native_bridge/element/styles/accessors.rs index 67c4e1951b..abe4c2b2d6 100644 --- a/moli-renderer-v8/src/native_bridge/element/styles/accessors.rs +++ b/moli-renderer-v8/src/native_bridge/element/styles/accessors.rs @@ -1,8 +1,5 @@ use crate::util::{throw_type_error, v8_string}; -use crate::css_style::serialize_css_style_entries; - -use super::super::property_string_value; use super::declaration::{ StyleMode, set_inline_style_css_text_with_pdb_storage, style_base_url, style_css_text_for_computed, style_entries, style_entries_css_text_with_pdb, @@ -12,6 +9,10 @@ use super::{ style_object_computation_context, style_object_forces_empty_computed, style_object_property_count_with_context, }; +use crate::{ + css_style::{CssStyleDeclarationCssTextArgs, serialize_css_style_entries}, + webidl, +}; pub(crate) fn style_length_getter_callback<'s>( scope: &mut v8::PinScope<'s, '_>, @@ -91,6 +92,15 @@ pub(crate) fn style_css_text_setter_callback<'s>( args: v8::FunctionCallbackArguments<'s>, mut rv: v8::ReturnValue<'_, v8::Value>, ) { + if style_runtime_and_handle_from_object(scope, args.this()).is_err() { + throw_style_declaration_illegal_invocation(scope, "set", "cssText"); + return; + } + let Some(parsed) = webidl::parse_args::(scope, &args) else { + return; + }; + // Conversion can run author code. Resolve the native declaration again + // before accessing its runtime or applying the readonly flag. let Ok((runtime_ptr, handle, mode)) = style_runtime_and_handle_from_object(scope, args.this()) else { throw_style_declaration_illegal_invocation(scope, "set", "cssText"); @@ -105,14 +115,9 @@ pub(crate) fn style_css_text_setter_callback<'s>( ); return; } - let value = if args.get(0).is_null_or_undefined() { - String::new() - } else { - property_string_value(scope, args.get(0)).unwrap_or_default() - }; let base_url = style_base_url(unsafe { &*runtime_ptr }, handle); unsafe { &mut *runtime_ptr }.set_element_inline_style_base_url(handle, base_url.clone()); - set_inline_style_css_text_with_pdb_storage(scope, runtime_ptr, handle, &value); + set_inline_style_css_text_with_pdb_storage(scope, runtime_ptr, handle, &parsed.value); rv.set_undefined(); } diff --git a/moli-renderer-v8/src/native_bridge/element/styles/methods.rs b/moli-renderer-v8/src/native_bridge/element/styles/methods.rs index 3dbca44e70..196a1679af 100644 --- a/moli-renderer-v8/src/native_bridge/element/styles/methods.rs +++ b/moli-renderer-v8/src/native_bridge/element/styles/methods.rs @@ -36,9 +36,6 @@ pub(crate) fn style_set_property_callback<'s>( throw_style_declaration_method_illegal_invocation(scope, "setProperty"); return; }; - if args.length() > 1 && args.get(1).is_undefined() { - return; - } let Some(parsed) = webidl::parse_args::(scope, &args) else { return; diff --git a/moli-renderer-v8/src/native_bridge/element/styles/named.rs b/moli-renderer-v8/src/native_bridge/element/styles/named.rs index ec732a4323..787235b897 100644 --- a/moli-renderer-v8/src/native_bridge/element/styles/named.rs +++ b/moli-renderer-v8/src/native_bridge/element/styles/named.rs @@ -1,6 +1,3 @@ -use crate::util::v8_string; - -use super::super::property_string_value; use super::declaration::{ StyleMode, cssom_style_property_affected_names_with_pdb, expand_unresolved_box_shorthand_entries_for_mutation, is_style_intrinsic_name, @@ -16,6 +13,10 @@ use super::{ style_object_computation_context, style_object_forces_empty_computed, style_object_pseudo_element, }; +use crate::{ + css_style::css_style_declaration_property_value, + util::{throw_type_error, v8_string}, +}; pub(super) fn style_named_getter<'s>( scope: &mut v8::PinScope<'s, '_>, @@ -174,15 +175,6 @@ pub(crate) fn set_live_style_named_property_value<'s>( let Ok((runtime_ptr, handle, mode)) = style_runtime_and_handle_from_object(scope, style) else { return false; }; - if mode == StyleMode::Computed { - crate::native_bridge::throw_dom_exception( - scope, - "NoModificationAllowedError", - 7, - "Cannot modify a read-only CSSStyleDeclaration.", - ); - return true; - } if is_style_intrinsic_name(raw_key) { return false; } @@ -193,11 +185,23 @@ pub(crate) fn set_live_style_named_property_value<'s>( }; property }; - let value = if value.is_null_or_undefined() { - String::new() - } else { - property_string_value(scope, value).unwrap_or_default() + let Some(value) = css_style_declaration_property_value(scope, value) else { + return true; }; + // Do not retain a native runtime lookup across author string conversion. + let Ok((runtime_ptr, handle, mode)) = style_runtime_and_handle_from_object(scope, style) else { + throw_type_error(scope, "Illegal invocation"); + return true; + }; + if mode == StyleMode::Computed { + crate::native_bridge::throw_dom_exception( + scope, + "NoModificationAllowedError", + 7, + "Cannot modify a read-only CSSStyleDeclaration.", + ); + return true; + } if set_inline_style_property_with_pdb_storage( scope, runtime_ptr, diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs index d23e1c5673..65ca237bde 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs @@ -62,3 +62,5 @@ mod css_computed_map_enumeration; mod nested_document_invalidation; mod properties_and_selectors; mod stylesheet_and_document_lifecycle; + +mod cssom_write_conversion; diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.js b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.js new file mode 100644 index 0000000000..ed63d44dc8 --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.js @@ -0,0 +1,152 @@ +(() => { + const checks = []; + const check = (name, fn) => { + try { checks.push({name, passed: fn() === true}); } + catch (error) { checks.push({name, passed: false, error: String(error)}); } + }; + const caught = fn => { try { fn(); return null; } catch (error) { return error; } }; + const frame = document.createElement('iframe'); + document.body.appendChild(frame); + const popup = window.open('about:blank', '_blank'); + const owners = [window, frame.contentWindow, popup]; + const resources = []; + const initial = 'color: red; background-color: blue; --marker: seed;'; + const operations = [ + ['color', (s, v) => { s.color = v; }, 'green', 'color'], + ['camel', (s, v) => { s.backgroundColor = v; }, 'green', 'background-color'], + ['dashed', (s, v) => { s['background-color'] = v; }, 'green', 'background-color'], + ['cssText', (s, v) => { s.cssText = v; }, 'color: green;', 'color'], + ['setProperty', (s, v) => s.setProperty('color', v), 'green', 'color'], + ['custom', (s, v) => s.setProperty('--marker', v), 'green', '--marker'], + ]; + function descriptor(owner, name) { + const style = owner.document.createElement('div').style; + for (let p = Object.getPrototypeOf(style); p; p = Object.getPrototypeOf(p)) { + const d = Object.getOwnPropertyDescriptor(p, name); + if (d) return d; + } + throw new Error('Missing CSSOM descriptor ' + name); + } + try { + for (const [realm, owner] of owners.entries()) { + const doc = owner.document; + const live = doc.createElement('div'); doc.body.appendChild(live); resources.push(live); + const detached = doc.createElement('div'); + const windowless = doc.implementation.createHTMLDocument('').createElement('div'); + const svg = doc.createElementNS('http://www.w3.org/2000/svg', 'svg'); + const sheetElement = doc.createElement('style'); + sheetElement.textContent = '.write-conversion-target { color: red; }'; + doc.head.appendChild(sheetElement); resources.push(sheetElement); + const Sheet = sheetElement.sheet.constructor; + const constructed = new Sheet(); constructed.insertRule('.constructed { color: red; }'); + const providers = [ + ['inline', live.style, false], ['detached', detached.style, false], + ['windowless', windowless.style, false], ['SVG', svg.style, false], + ['rule', sheetElement.sheet.cssRules[0].style, false], + ['constructed rule', constructed.cssRules[0].style, false], + ['computed', owner.getComputedStyle(live), true], + ]; + for (const [kind, style, readonly] of providers) { + const reset = () => { live.style.cssText = initial; if (!readonly) style.cssText = initial; }; + for (const [operation, write, good, property] of operations) { + const prefix = `${realm}/${kind}/${operation}`; + for (const failure of ['toString', 'toPrimitive', 'getter', 'fallback', 'symbol', 'nonprimitive']) { + check(prefix + '/' + failure + '/exception and unchanged declaration', () => { + reset(); const before = style.cssText; const marker = {prefix, failure}; const calls = []; + let value; + if (failure === 'toString') value = {toString() { calls.push('toString'); throw marker; }}; + if (failure === 'toPrimitive') value = {[Symbol.toPrimitive](hint) { calls.push(hint); throw marker; }}; + if (failure === 'getter') value = {get toString() { calls.push('get'); throw marker; }}; + if (failure === 'fallback') value = {toString() { calls.push('toString'); return {}; }, valueOf() { calls.push('valueOf'); throw marker; }}; + if (failure === 'symbol') value = Symbol('value'); + if (failure === 'nonprimitive') value = {[Symbol.toPrimitive](hint) { calls.push(hint); return {}; }}; + const error = caught(() => write(style, value)); + const expected = failure === 'fallback' ? ['toString', 'valueOf'] : failure === 'getter' ? ['get'] : failure === 'toString' ? ['toString'] : failure === 'symbol' ? [] : ['string']; + return (failure === 'symbol' || failure === 'nonprimitive' ? error?.name === 'TypeError' : error === marker) && + JSON.stringify(calls) === JSON.stringify(expected) && style.cssText === before; + }); + } + check(prefix + '/successful conversion precedes native write or readonly error', () => { + reset(); let calls = 0; + const error = caught(() => write(style, {[Symbol.toPrimitive](hint) { calls++; if (hint !== 'string') throw new Error(hint); return good; }})); + return calls === 1 && (readonly ? error?.name === 'NoModificationAllowedError' : error === null && style.getPropertyValue(property) === 'green'); + }); + for (const [name, value] of [['null', null], ['undefined', undefined]]) { + check(prefix + '/' + name + '/IDL string conversion', () => { + reset(); const before = style.getPropertyValue(property); + const error = caught(() => write(style, value)); + if (readonly) return error?.name === 'NoModificationAllowedError'; + const expected = operation === 'cssText' || value === null ? '' : operation === 'custom' ? 'undefined' : before; + return error === null && style.getPropertyValue(property) === expected; + }); + } + if (!readonly) { + for (const throws of [false, true]) { + check(prefix + '/reentrant mutation/' + (throws ? 'throw' : 'success'), () => { + reset(); const marker = {}; + const error = caught(() => write(style, {toString() { + style.setProperty(property, 'green'); + if (throws) throw marker; + return operation === 'cssText' ? 'color: blue;' : 'blue'; + }})); + return error === (throws ? marker : null) && style.getPropertyValue(property) === (throws ? 'green' : 'blue'); + }); + } + } + } + for (const throwingArgument of ['none', 'property', 'value', 'priority']) { + check(`${realm}/${kind}/setProperty/conversion order/${throwingArgument}`, () => { + reset(); const before = style.cssText; const calls = []; const marker = {}; + const argument = (name, text) => ({toString() { calls.push(name); if (name === throwingArgument) throw marker; return text; }}); + const error = caught(() => style.setProperty(argument('property', 'color'), argument('value', 'green'), argument('priority', 'important'))); + const expected = throwingArgument === 'property' ? ['property'] : throwingArgument === 'value' ? ['property', 'value'] : ['property', 'value', 'priority']; + return JSON.stringify(calls) === JSON.stringify(expected) && (throwingArgument === 'none' ? + readonly ? error?.name === 'NoModificationAllowedError' : error === null && style.color === 'green' && style.getPropertyPriority('color') === 'important' : + error === marker && style.cssText === before); + }); + } + for (const property of ['color', '--marker', 'unknown-css-property']) { + for (const priorityThrows of [false, true]) { + check(`${realm}/${kind}/undefined value/${property}/${priorityThrows}`, () => { + reset(); const before = style.cssText; const calls = []; const marker = {}; + const error = caught(() => style.setProperty({toString() { calls.push('property'); return property; }}, undefined, + {toString() { calls.push('priority'); if (priorityThrows) throw marker; return ''; }})); + if (JSON.stringify(calls) !== '["property","priority"]') return false; + if (priorityThrows) return error === marker && style.cssText === before; + if (readonly) return error?.name === 'NoModificationAllowedError'; + return error === null && (property === '--marker' ? style.getPropertyValue(property) === 'undefined' : style.cssText === before); + }); + } + } + check(`${realm}/${kind}/expando bypasses CSS conversion`, () => { + const value = {toString() { throw new Error('Expando coerced'); }}; + style.__writeConversionExpando = value; + return style.__writeConversionExpando === value; + }); + for (const [calleeIndex, callee] of owners.entries()) { + for (const member of ['cssText', 'setProperty']) { + const binding = descriptor(callee, member); + const fn = member === 'setProperty' ? binding.value : binding.set; + const errorPrototype = fn.constructor('return TypeError.prototype')(); + for (const badKind of ['object', 'inherited', 'author Proxy', 'revoked Proxy']) { + check(`${realm}/${kind}/${calleeIndex}/${member}/${badKind}/receiver before conversion`, () => { + const revocable = Proxy.revocable(style, {}); revocable.revoke(); + const receiver = badKind === 'object' ? {} : badKind === 'inherited' ? Object.create(style) : badKind === 'author Proxy' ? new Proxy(style, {}) : revocable.proxy; + let conversions = 0; const value = {toString() { conversions++; return 'green'; }}; + const error = caught(() => Reflect.apply(fn, receiver, member === 'setProperty' ? [value, value, value] : [value])); + return Object.getPrototypeOf(error) === errorPrototype && conversions === 0; + }); + } + } + } + } + } + } finally { + for (const resource of resources) resource.remove(); + popup.close(); frame.remove(); + } + const facts = {complete: true, total: checks.length, passed: checks.filter(row => row.passed).length, checks}; + globalThis.__cssomWriteConversionResults = facts; + globalThis.__uiEventResults = facts; + return facts.passed === facts.total; +})() diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.rs new file mode 100644 index 0000000000..d2b27acb87 --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.rs @@ -0,0 +1,18 @@ +use super::*; + +#[test] +fn cssom_writes_validate_receivers_then_convert_before_mutating() { + let mut vm = new_storage_page_task_executor_test_vm("https://cssom-write-conversion.test/"); + vm.eval(include_str!("cssom_write_conversion.js")) + .expect("CSSOM write conversion matrix should evaluate"); + assert_eq!( + vm.eval("JSON.stringify(__cssomWriteConversionResults.checks.filter(row => !row.passed))") + .unwrap(), + "[]" + ); + assert_eq!( + vm.eval("__cssomWriteConversionResults.complete && __cssomWriteConversionResults.total === 2085 && __cssomWriteConversionResults.passed === 2085") + .unwrap(), + "true" + ); +}