From 54be317fa92730a57f762d5fa9263728f05952fd Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Tue, 27 Sep 2022 16:46:22 +0100 Subject: [PATCH] LibWeb: Use TokenStreams when expanding unresolved CSS values This fixes a bug where any whitespace inside the `var()` or `attr()` functions would make the StyleComputer fail to resolve them. --- .../Libraries/LibWeb/CSS/StyleComputer.cpp | 57 ++++++++++++------- Userland/Libraries/LibWeb/CSS/StyleComputer.h | 5 +- 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index e020376dafb..f1ac365eddb 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -556,7 +556,7 @@ static RefPtr get_custom_property(DOM::Element const& element, FlySt return nullptr; } -bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView property_name, HashMap>& dependencies, Vector const& source, Vector& dest, size_t source_start_index) const +bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView property_name, HashMap>& dependencies, Parser::TokenStream& source, Vector& dest) const { // FIXME: Do this better! // We build a copy of the tree of ComponentValues, with all var()s and attr()s replaced with their contents. @@ -565,7 +565,7 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p // Arbitrary large value chosen to avoid the billion-laughs attack. // https://www.w3.org/TR/css-variables-1/#long-variables const size_t MAX_VALUE_COUNT = 16384; - if (source.size() + dest.size() > MAX_VALUE_COUNT) { + if (source.remaining_token_count() + dest.size() > MAX_VALUE_COUNT) { dbgln("Stopped expanding CSS variables: maximum length reached."); return false; } @@ -578,15 +578,16 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p return new_node; }; - for (size_t source_index = source_start_index; source_index < source.size(); source_index++) { - auto const& value = source[source_index]; + while (source.has_next_token()) { + auto const& value = source.next_token(); if (value.is_function()) { if (value.function().name().equals_ignoring_case("var"sv)) { - auto const& var_contents = value.function().values(); - if (var_contents.is_empty()) + Parser::TokenStream var_contents { value.function().values() }; + var_contents.skip_whitespace(); + if (!var_contents.has_next_token()) return false; - auto const& custom_property_name_token = var_contents.first(); + auto const& custom_property_name_token = var_contents.next_token(); if (!custom_property_name_token.is(Parser::Token::Type::Ident)) return false; auto custom_property_name = custom_property_name_token.token().ident(); @@ -606,24 +607,32 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p if (auto custom_property_value = get_custom_property(element, custom_property_name)) { VERIFY(custom_property_value->is_unresolved()); - if (!expand_unresolved_values(element, custom_property_name, dependencies, custom_property_value->as_unresolved().values(), dest, 0)) + Parser::TokenStream custom_property_tokens { custom_property_value->as_unresolved().values() }; + if (!expand_unresolved_values(element, custom_property_name, dependencies, custom_property_tokens, dest)) return false; continue; } // Use the provided fallback value, if any. - if (var_contents.size() > 2 && var_contents[1].is(Parser::Token::Type::Comma)) { - if (!expand_unresolved_values(element, property_name, dependencies, var_contents, dest, 2)) + var_contents.skip_whitespace(); + if (var_contents.has_next_token()) { + auto const& comma_token = var_contents.next_token(); + if (!comma_token.is(Parser::Token::Type::Comma)) + return false; + var_contents.skip_whitespace(); + if (!expand_unresolved_values(element, property_name, dependencies, var_contents, dest)) return false; - continue; } - } else if (value.function().name().equals_ignoring_case("attr"sv)) { + continue; + } + if (value.function().name().equals_ignoring_case("attr"sv)) { // https://drafts.csswg.org/css-values-5/#attr-substitution - auto const& attr_contents = value.function().values(); - if (attr_contents.is_empty()) + Parser::TokenStream attr_contents { value.function().values() }; + attr_contents.skip_whitespace(); + if (!attr_contents.has_next_token()) return false; - auto const& attr_name_token = attr_contents.first(); + auto const& attr_name_token = attr_contents.next_token(); if (!attr_name_token.is(Parser::Token::Type::Ident)) return false; auto attr_name = attr_name_token.token().ident(); @@ -638,8 +647,13 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p // 2. Otherwise, if the attr() function has a fallback value as its last argument, replace the attr() function by the fallback value. // If there are any var() or attr() references in the fallback, substitute them as well. - if (attr_contents.size() > 2 && attr_contents[1].is(Parser::Token::Type::Comma)) { - if (!expand_unresolved_values(element, property_name, dependencies, attr_contents, dest, 2)) + attr_contents.skip_whitespace(); + if (attr_contents.has_next_token()) { + auto const& comma_token = attr_contents.next_token(); + if (!comma_token.is(Parser::Token::Type::Comma)) + return false; + attr_contents.skip_whitespace(); + if (!expand_unresolved_values(element, property_name, dependencies, attr_contents, dest)) return false; continue; } @@ -650,7 +664,8 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p auto const& source_function = value.function(); Vector function_values; - if (!expand_unresolved_values(element, property_name, dependencies, source_function.values(), function_values, 0)) + Parser::TokenStream source_function_contents { source_function.values() }; + if (!expand_unresolved_values(element, property_name, dependencies, source_function_contents, function_values)) return false; NonnullRefPtr function = Parser::Function::create(source_function.name(), move(function_values)); dest.empend(function); @@ -658,8 +673,9 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p } if (value.is_block()) { auto const& source_block = value.block(); + Parser::TokenStream source_block_values { source_block.values() }; Vector block_values; - if (!expand_unresolved_values(element, property_name, dependencies, source_block.values(), block_values, 0)) + if (!expand_unresolved_values(element, property_name, dependencies, source_block_values, block_values)) return false; NonnullRefPtr block = Parser::Block::create(source_block.token(), move(block_values)); dest.empend(move(block)); @@ -679,7 +695,8 @@ RefPtr StyleComputer::resolve_unresolved_style_value(DOM::Element& e Vector expanded_values; HashMap> dependencies; - if (!expand_unresolved_values(element, string_from_property_id(property_id), dependencies, unresolved.values(), expanded_values, 0)) + Parser::TokenStream unresolved_values { unresolved.values() }; + if (!expand_unresolved_values(element, string_from_property_id(property_id), dependencies, unresolved_values, expanded_values)) return {}; if (auto parsed_value = Parser::Parser::parse_css_value({}, Parser::ParsingContext { document() }, property_id, expanded_values)) diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.h b/Userland/Libraries/LibWeb/CSS/StyleComputer.h index 9f2b05feddb..8a22f750d36 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.h +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.h @@ -1,6 +1,6 @@ /* * Copyright (c) 2018-2020, Andreas Kling - * Copyright (c) 2021, Sam Atkins + * Copyright (c) 2021-2022, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -86,7 +87,7 @@ private: void compute_defaulted_property_value(StyleProperties&, DOM::Element const*, CSS::PropertyID, Optional) const; RefPtr resolve_unresolved_style_value(DOM::Element&, PropertyID, UnresolvedStyleValue const&) const; - bool expand_unresolved_values(DOM::Element&, StringView property_name, HashMap>& dependencies, Vector const& source, Vector& dest, size_t source_start_index) const; + bool expand_unresolved_values(DOM::Element&, StringView property_name, HashMap>& dependencies, Parser::TokenStream& source, Vector& dest) const; template void for_each_stylesheet(CascadeOrigin, Callback) const;