From 69c3a1070f9ddaf8df4d5cf45163bd1424b66b8d Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Tue, 15 Apr 2025 18:01:54 +0100 Subject: [PATCH] LibWeb: Don't construct stylesheet when modifying `CSSStyleSheet` rules Previously, `CSSStyleSheet.replace()` and `CSSStyleSheet.replaceSync()` parsed the given CSS text into a temporary stylesheet object, from which a list of rules was extracted. Doing this had the unintended side-effect that a fetch request would be started if the given CSS text referenced any external resources. This fetch request would cause a crash, since the temporary stylesheet object didn't set the constructed flag, or constructor document. We now parse the given CSS text as a list of rules without constructing a temporary stylesheet. --- Libraries/LibWeb/CSS/CSSStyleSheet.cpp | 10 ++- Libraries/LibWeb/CSS/Parser/Parser.cpp | 16 +++++ Libraries/LibWeb/CSS/Parser/Parser.h | 1 + .../CSSStyleSheet-constructable-baseURL.txt | 8 +++ .../CSSStyleSheet-constructable-baseURL.html | 66 +++++++++++++++++++ 5 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/wpt-import/css/cssom/CSSStyleSheet-constructable-baseURL.txt create mode 100644 Tests/LibWeb/Text/input/wpt-import/css/cssom/CSSStyleSheet-constructable-baseURL.html diff --git a/Libraries/LibWeb/CSS/CSSStyleSheet.cpp b/Libraries/LibWeb/CSS/CSSStyleSheet.cpp index 7cab90e9839..34d6194ca59 100644 --- a/Libraries/LibWeb/CSS/CSSStyleSheet.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleSheet.cpp @@ -1,7 +1,7 @@ /* * Copyright (c) 2019-2022, Andreas Kling * Copyright (c) 2022-2024, Sam Atkins - * Copyright (c) 2024, Tim Ledbetter + * Copyright (c) 2024-2025, Tim Ledbetter * * SPDX-License-Identifier: BSD-2-Clause */ @@ -207,8 +207,7 @@ GC::Ref CSSStyleSheet::replace(String text) HTML::TemporaryExecutionContext execution_context { realm, HTML::TemporaryExecutionContext::CallbacksEnabled::Yes }; // 1. Let rules be the result of running parse a stylesheet’s contents from text. - auto parsed_stylesheet = parse_css_stylesheet(make_parsing_params(), text); - auto& rules = parsed_stylesheet->rules(); + auto rules = CSS::Parser::Parser::create(make_parsing_params(), text).parse_as_stylesheet_contents(); // 2. If rules contains one or more @import rules, remove those rules from rules. GC::RootVector> rules_without_import(realm.heap()); @@ -240,8 +239,7 @@ WebIDL::ExceptionOr CSSStyleSheet::replace_sync(StringView text) return WebIDL::NotAllowedError::create(realm(), "Can't call replaceSync() on non-modifiable stylesheets"_string); // 2. Let rules be the result of running parse a stylesheet’s contents from text. - auto parsed_stylesheet = parse_css_stylesheet(make_parsing_params(), text); - auto& rules = parsed_stylesheet->rules(); + auto rules = CSS::Parser::Parser::create(make_parsing_params(), text).parse_as_stylesheet_contents(); // 3. If rules contains one or more @import rules, remove those rules from rules. GC::RootVector> rules_without_import(realm().heap()); @@ -250,7 +248,7 @@ WebIDL::ExceptionOr CSSStyleSheet::replace_sync(StringView text) rules_without_import.append(rule); } - // 4.Set sheet’s CSS rules to rules. + // 4. Set sheet’s CSS rules to rules. m_rules->set_rules({}, rules_without_import); return {}; diff --git a/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Libraries/LibWeb/CSS/Parser/Parser.cpp index c450f01ecf6..29594100bc1 100644 --- a/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -118,6 +118,22 @@ Vector Parser::parse_a_stylesheets_contents(TokenStream& input) return consume_a_stylesheets_contents(input); } +GC::RootVector> Parser::parse_as_stylesheet_contents() +{ + auto raw_rules = parse_a_stylesheets_contents(m_token_stream); + GC::RootVector> rules(realm().heap()); + for (auto const& raw_rule : raw_rules) { + auto rule = convert_to_rule(raw_rule, Nested::No); + if (!rule) { + log_parse_error(); + continue; + } + rules.append(*rule); + } + + return rules; +} + // https://drafts.csswg.org/css-syntax/#parse-a-css-stylesheet GC::Ref Parser::parse_as_css_stylesheet(Optional<::URL::URL> location, Vector> media_query_list) { diff --git a/Libraries/LibWeb/CSS/Parser/Parser.h b/Libraries/LibWeb/CSS/Parser/Parser.h index 42f0fd74c95..1d98d9e53a5 100644 --- a/Libraries/LibWeb/CSS/Parser/Parser.h +++ b/Libraries/LibWeb/CSS/Parser/Parser.h @@ -100,6 +100,7 @@ public: Vector parse_as_list_of_descriptors(AtRuleID); CSSRule* parse_as_css_rule(); Optional parse_as_supports_condition(); + GC::RootVector> parse_as_stylesheet_contents(); enum class SelectorParsingMode { Standard, diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/cssom/CSSStyleSheet-constructable-baseURL.txt b/Tests/LibWeb/Text/expected/wpt-import/css/cssom/CSSStyleSheet-constructable-baseURL.txt new file mode 100644 index 00000000000..6f09bf85a9c --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/css/cssom/CSSStyleSheet-constructable-baseURL.txt @@ -0,0 +1,8 @@ +Harness status: OK + +Found 3 tests + +3 Pass +Pass Constructing sheet with custom base URL ueses that URL for CSS rules +Pass Constructing sheet with relative URL adds to the constructor document's base URL +Pass Constructing sheet with invalid base URL throws a NotAllowedError \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/wpt-import/css/cssom/CSSStyleSheet-constructable-baseURL.html b/Tests/LibWeb/Text/input/wpt-import/css/cssom/CSSStyleSheet-constructable-baseURL.html new file mode 100644 index 00000000000..d65bd549497 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/css/cssom/CSSStyleSheet-constructable-baseURL.html @@ -0,0 +1,66 @@ + +CSSStyleSheet baseURL + + +
+ + +