From e1c037815c418dad1a95517c32c09c993ac4b572 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Sat, 28 Jun 2025 19:31:03 +0200 Subject: [PATCH] Fix skipping CSP checks for styles when cloning nodes (#37465) Cloned nodes were re-parsing already-parsed style attributes. As such, they were also checking CSP, which shouldn't happen as the original node already was checked for it. Testing: The existing WPT test now mostly passes. It had two cases which were passing in no browsers. Fixes: Part of #4577 Signed-off-by: Tim van der Lippe --- components/script/dom/node.rs | 26 ++++++++++++++- tests/wpt/meta/MANIFEST.json | 2 +- ...allowed-while-cloning-objects.sub.html.ini | 33 ++----------------- ...yle-allowed-while-cloning-objects.sub.html | 3 ++ 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 95c1bd5a662..a176cbd86e5 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -44,6 +44,7 @@ use servo_arc::Arc; use servo_config::pref; use servo_url::ServoUrl; use smallvec::SmallVec; +use style::attr::AttrValue; use style::context::QuirksMode; use style::dom::OpaqueNode; use style::properties::ComputedValues; @@ -2713,6 +2714,27 @@ impl Node { parent.owner_doc().remove_script_and_layout_blocker(); } + /// Ensure that for styles, we clone the already-parsed property declaration block. + /// This does two things: + /// 1. it uses the same fast-path as CSSStyleDeclaration + /// 2. it also avoids the CSP checks when cloning (it shouldn't run any when cloning + /// existing valid attributes) + fn compute_attribute_value_with_style_fast_path(attr: &Dom, elem: &Element) -> AttrValue { + if *attr.local_name() == local_name!("style") { + if let Some(ref pdb) = *elem.style_attribute().borrow() { + let document = elem.owner_document(); + let shared_lock = document.style_shared_lock(); + let new_pdb = pdb.read_with(&shared_lock.read()).clone(); + return AttrValue::Declaration( + (**attr.value()).to_owned(), + Arc::new(shared_lock.wrap(new_pdb)), + ); + } + } + + attr.value().clone() + } + /// pub(crate) fn clone( node: &Node, @@ -2835,9 +2857,11 @@ impl Node { let copy_elem = copy.downcast::().unwrap(); for attr in node_elem.attrs().iter() { + let new_value = + Node::compute_attribute_value_with_style_fast_path(attr, node_elem); copy_elem.push_new_attribute( attr.local_name().clone(), - attr.value().clone(), + new_value, attr.name().clone(), attr.namespace().clone(), attr.prefix().cloned(), diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index a8c8608e18c..0ace5e4ccc3 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -577403,7 +577403,7 @@ ] ], "inline-style-allowed-while-cloning-objects.sub.html": [ - "f702e6eb9d65fadfcb3ea9199146d835df036e04", + "32f711a9f15aa19a621c370e3abb26a425982ae9", [ null, {} diff --git a/tests/wpt/meta/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html.ini b/tests/wpt/meta/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html.ini index c99d7bd7844..d62b5c67fca 100644 --- a/tests/wpt/meta/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html.ini +++ b/tests/wpt/meta/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html.ini @@ -2,35 +2,8 @@ [non-HTML namespace] expected: FAIL - [inline-style-allowed-while-cloning-objects 1] - expected: FAIL - - [inline-style-allowed-while-cloning-objects 3] - expected: FAIL - - [inline-style-allowed-while-cloning-objects 5] - expected: FAIL - - [inline-style-allowed-while-cloning-objects 7] - expected: FAIL - - [inline-style-allowed-while-cloning-objects 8] - expected: FAIL - - [inline-style-allowed-while-cloning-objects 9] - expected: FAIL - - [inline-style-allowed-while-cloning-objects 10] - expected: FAIL - - [inline-style-allowed-while-cloning-objects 11] - expected: FAIL - - [inline-style-allowed-while-cloning-objects 17] - expected: FAIL - - [inline-style-allowed-while-cloning-objects 18] - expected: FAIL - [inline-style-allowed-while-cloning-objects 19] expected: FAIL + + [inline-style-allowed-while-cloning-objects 20] + expected: FAIL diff --git a/tests/wpt/tests/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html b/tests/wpt/tests/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html index f702e6eb9d6..32f711a9f15 100644 --- a/tests/wpt/tests/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html +++ b/tests/wpt/tests/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html @@ -80,6 +80,9 @@ test(function() { assert_equals(clonedOps.style.background, "") }); + test(function() { + assert_equals(clonedOps.style.color, "red") + }); test(function() { assert_equals(violetOps.style.background.match(/rgb\(238, 130, 238\)/)[0], "rgb(238, 130, 238)") });