LibWeb: Validate ASF syntax at parse time

This commit is contained in:
Callum Law 2026-03-28 22:41:48 +13:00 committed by Sam Atkins
parent 04f634eeb2
commit 03d479c1da
18 changed files with 96 additions and 31 deletions

View file

@ -184,7 +184,13 @@ WebIDL::ExceptionOr<NonnullRefPtr<StyleValue const>> CSSUnparsedValue::create_an
auto string = TRY(to_string());
auto parser = Parser::Parser::create(Parser::ParsingParams {}, string);
auto component_values = parser.parse_as_list_of_component_values();
return UnresolvedStyleValue::create(move(component_values));
Parser::SubstitutionFunctionsPresence substitution_presence;
if (Parser::Parser::collect_arbitrary_substitution_function_presence(component_values, substitution_presence).is_error())
return WebIDL::SimpleException { WebIDL::SimpleExceptionType::TypeError, "Invalid arbitrary substitution function syntax"_string };
return UnresolvedStyleValue::create(move(component_values), substitution_presence);
}
}

View file

@ -46,7 +46,8 @@ Parser::ParseErrorOr<NonnullRefPtr<StyleValue const>> Parser::parse_descriptor_v
if (token.is(Token::Type::Semicolon))
return ParseError::SyntaxError;
collect_arbitrary_substitution_function_presence(token, substitution_functions_presence);
if (collect_arbitrary_substitution_function_presence(token, substitution_functions_presence).is_error())
return ParseError::SyntaxError;
}
auto metadata = get_descriptor_metadata(at_rule_id, descriptor_name_and_id.id());

View file

@ -2009,17 +2009,24 @@ NonnullRefPtr<StyleValue const> Parser::parse_as_sizes_attribute(DOM::Element co
return LengthStyleValue::create(Length(100, LengthUnit::Vw));
}
void Parser::collect_arbitrary_substitution_function_presence(Vector<ComponentValue> const& component_values, SubstitutionFunctionsPresence& presence)
Parser::ParseErrorOr<void> Parser::collect_arbitrary_substitution_function_presence(Vector<ComponentValue> const& component_values, SubstitutionFunctionsPresence& presence)
{
for (auto const& component_value : component_values)
collect_arbitrary_substitution_function_presence(component_value, presence);
for (auto const& component_value : component_values) {
if (collect_arbitrary_substitution_function_presence(component_value, presence).is_error())
return ParseError::SyntaxError;
}
return {};
}
void Parser::collect_arbitrary_substitution_function_presence(ComponentValue const& component_value, SubstitutionFunctionsPresence& presence)
Parser::ParseErrorOr<void> Parser::collect_arbitrary_substitution_function_presence(ComponentValue const& component_value, SubstitutionFunctionsPresence& presence)
{
if (component_value.is_function()) {
auto const& function = component_value.function();
if (auto arbitrary_substitution_function = to_arbitrary_substitution_function(function.name); arbitrary_substitution_function.has_value()) {
if (!parse_according_to_argument_grammar(arbitrary_substitution_function.value(), function.value).has_value())
return ParseError::SyntaxError;
switch (arbitrary_substitution_function.value()) {
case ArbitrarySubstitutionFunction::Attr:
presence.attr = true;
@ -2039,10 +2046,13 @@ void Parser::collect_arbitrary_substitution_function_presence(ComponentValue con
}
}
collect_arbitrary_substitution_function_presence(function.value, presence);
} else if (component_value.is_block()) {
collect_arbitrary_substitution_function_presence(component_value.block().value, presence);
return collect_arbitrary_substitution_function_presence(function.value, presence);
}
if (component_value.is_block())
return collect_arbitrary_substitution_function_presence(component_value.block().value, presence);
return {};
}
bool Parser::has_ignored_vendor_prefix(StringView string)

View file

@ -185,12 +185,6 @@ public:
GC::Ref<Descriptors> convert_to_descriptors(AtRuleID, Vector<Declaration> const& declarations);
GC::Ref<CSSStyleProperties> convert_to_style_declaration(Vector<Declaration> const&);
static void collect_arbitrary_substitution_function_presence(Vector<ComponentValue> const&, SubstitutionFunctionsPresence&);
static void collect_arbitrary_substitution_function_presence(ComponentValue const&, SubstitutionFunctionsPresence&);
private:
Parser(ParsingParams const&, Vector<Token>);
enum class ParseError : u8 {
IncludesIgnoredVendorPrefix,
SyntaxError,
@ -198,6 +192,12 @@ private:
template<typename T>
using ParseErrorOr = ErrorOr<T, ParseError>;
static ParseErrorOr<void> collect_arbitrary_substitution_function_presence(Vector<ComponentValue> const&, SubstitutionFunctionsPresence&);
static ParseErrorOr<void> collect_arbitrary_substitution_function_presence(ComponentValue const&, SubstitutionFunctionsPresence&);
private:
Parser(ParsingParams const&, Vector<Token>);
// "Parse a stylesheet" is intended to be the normal parser entry point, for parsing stylesheets.
struct ParsedStyleSheet {
Optional<::URL::URL> location;

View file

@ -485,8 +485,12 @@ Parser::ParseErrorOr<NonnullRefPtr<StyleValue const>> Parser::parse_css_value(Pr
return ParseError::SyntaxError;
}
// FIXME: We should validate ASF grammar syntax at parse time
collect_arbitrary_substitution_function_presence(token, substitution_presence);
// https://drafts.csswg.org/css-values-5/#resolve-property
// If a property value contains one or more arbitrary substitution functions, and all of those functions are
// themselves syntactically valid according to their argument grammars, the entire values grammar must be
// assumed to be valid at parse time.
if (collect_arbitrary_substitution_function_presence(token, substitution_presence).is_error())
return ParseError::SyntaxError;
}
tokens.restore_a_mark();

View file

@ -1324,6 +1324,7 @@ Optional<Parser::FunctionPrelude> Parser::parse_function_prelude(TokenStream<Com
// If a default value and a parameter type are both provided, then the default value must parse successfully
// according to that parameter types syntax. Otherwise, the @function rule is invalid.
// FIXME: Chrome allows ASFs regardless of the parameter's type
TokenStream default_value_token_stream { maybe_default_value.value() };
if (!parse_according_to_syntax_node(default_value_token_stream, *type) || !default_value_token_stream.is_empty())
return {};

View file

@ -233,8 +233,12 @@ RefPtr<StyleValue const> Parser::parse_according_to_syntax_node(TokenStream<Comp
switch (syntax_node.type()) {
case SyntaxNode::NodeType::Universal:
if (auto declaration_value = parse_declaration_value(tokens); declaration_value.has_value()) {
SubstitutionFunctionsPresence substitution_functions_presence;
if (collect_arbitrary_substitution_function_presence(declaration_value.value(), substitution_functions_presence).is_error())
return nullptr;
transaction.commit();
return UnresolvedStyleValue::create(declaration_value.release_value());
return UnresolvedStyleValue::create(declaration_value.release_value(), substitution_functions_presence);
}
return nullptr;
case SyntaxNode::NodeType::Ident: {

View file

@ -23,7 +23,9 @@ ValueComparingNonnullRefPtr<UnresolvedStyleValue const> UnresolvedStyleValue::cr
{
if (!substitution_presence.has_value()) {
substitution_presence = Parser::SubstitutionFunctionsPresence {};
Parser::Parser::collect_arbitrary_substitution_function_presence(values, *substitution_presence);
// FIXME: Make substitution_presence non-optional since all callers need to check ASF argument syntax anyway
auto result = Parser::Parser::collect_arbitrary_substitution_function_presence(values, *substitution_presence);
VERIFY(!result.is_error());
}
return adopt_ref(*new (nothrow) UnresolvedStyleValue(move(values), *substitution_presence, move(original_source_text)));

View file

@ -1 +1,2 @@
@property --foo { syntax: "*"; inherits: true; }
@property --bar { syntax: "*"; inherits: true; }

View file

@ -0,0 +1 @@
@function --var-default(--x: var(--fallback)) { }

View file

@ -2,10 +2,9 @@ Harness status: OK
Found 70 tests
68 Pass
2 Fail
Fail e.style['content'] = "attr()" should not set the property value
Fail e.style['content'] = "attr() / \"alt text\"" should not set the property value
70 Pass
Pass e.style['content'] = "attr()" should not set the property value
Pass e.style['content'] = "attr() / \"alt text\"" should not set the property value
Pass e.style['content'] = "counters(counter-name)" should not set the property value
Pass e.style['content'] = "counters(counter-name) / \"alt text\"" should not set the property value
Pass e.style['content'] = "counter()" should not set the property value

View file

@ -2,5 +2,5 @@ Harness status: OK
Found 1 tests
1 Fail
Fail Test style seralization round tripping with CSS env vars
1 Pass
Pass Test style seralization round tripping with CSS env vars

View file

@ -2,5 +2,5 @@ Harness status: OK
Found 1 tests
1 Fail
Fail Test that CSS env vars work with CSS.supports
1 Pass
Pass Test that CSS env vars work with CSS.supports

View file

@ -2,13 +2,12 @@ Harness status: OK
Found 8 tests
6 Pass
2 Fail
8 Pass
Pass e.style['left'] = "inherit(--x)" should set the property value
Pass e.style['left'] = "calc(inherit(--x) + 1px)" should set the property value
Pass e.style['left'] = "inherit(--x,)" should set the property value
Pass e.style['left'] = "inherit(--x, )" should set the property value
Pass e.style['left'] = "inherit(--x , )" should set the property value
Pass e.style['left'] = "inherit(--x, foo)" should set the property value
Fail e.style['left'] = "inherit(!!, foo)" should not set the property value
Fail e.style['left'] = "inherit(, foo)" should not set the property value
Pass e.style['left'] = "inherit(!!, foo)" should not set the property value
Pass e.style['left'] = "inherit(, foo)" should not set the property value

View file

@ -5,11 +5,18 @@
inherits: true;
initial-value: var(foo);
}
@property --bar {
syntax: "*";
inherits: true;
initial-value: var(!this-does-not-match-argument-grammar);
}
</style>
<div id="target"></div>
<script src="../include.js"></script>
<script>
test(() => {
println(document.styleSheets[0].cssRules[0].cssText);
println(document.styleSheets[0].cssRules[1].cssText);
});
</script>

View file

@ -0,0 +1,16 @@
<!doctype html>
<style>
@function --var-default(--x: var(--fallback)) {
}
@function --invalid-var-default(--x: var(, 1px)) {
}
</style>
<script src="../include.js"></script>
<script>
test(() => {
for (const rule of document.styleSheets[0].cssRules) {
println(rule.cssText);
}
});
</script>

View file

@ -0,0 +1,13 @@
<!doctype html>
<div id="target"></div>
<script src="../include.js"></script>
<script>
test(() => {
try {
target.attributeStyleMap.set("width", new CSSUnparsedValue(["var(,)"]));
println("FAIL");
} catch {
println("PASS");
}
});
</script>