LibWeb/CSS: Treat block at-rules with no block as invalid

Author: https://github.com/tcl3
Commit: b00e57139f
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/5204
Reviewed-by: https://github.com/gmta ✅

Libraries/LibWeb/CSS/Parser/MediaParsing.cpp

@@ -561,6 +561,15 @@ Optional<MediaFeatureValue> Parser::parse_media_feature_value(MediaFeatureID med
 
 GC::Ptr<CSSMediaRule> Parser::convert_to_media_rule(AtRule const& rule, Nested nested)
 {
+    // https://drafts.csswg.org/css-conditional-3/#at-media
+    // @media <media-query-list> {
+    // <rule-list>
+    // }
+    if (!rule.is_block_rule) {
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @media rule: Expected a block.");
+        return nullptr;
+    }
+
     auto media_query_tokens = TokenStream { rule.prelude };
     auto media_query_list = parse_a_media_query_list(media_query_tokens);
     auto media_list = MediaList::create(realm(), move(media_query_list));

Libraries/LibWeb/CSS/Parser/RuleParsing.cpp

@@ -190,14 +190,13 @@ GC::Ptr<CSSImportRule> Parser::convert_to_import_rule(AtRule const& rule)
     //
     // <import-conditions> = [ supports( [ <supports-condition> | <declaration> ] ) ]?
     //                      <media-query-list>?
-
+    if (rule.is_block_rule) {
-    if (rule.prelude.is_empty()) {
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @import rule: Block is not allowed.");
-        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @import rule: Empty prelude.");
         return {};
     }
 
-    if (rule.is_block_rule) {
+    if (rule.prelude.is_empty()) {
-        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @import rule: Block is not allowed.");
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @import rule: Empty prelude.");
         return {};
     }
 
@@ -365,14 +364,16 @@ GC::Ptr<CSSKeyframesRule> Parser::convert_to_keyframes_rule(AtRule const& rule)
     // <keyframes-name> = <custom-ident> | <string>
     // <keyframe-block> = <keyframe-selector># { <declaration-list> }
     // <keyframe-selector> = from | to | <percentage [0,100]>
+    if (!rule.is_block_rule) {
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @keyframes rule: Expected a block.");
+        return nullptr;
+    }
 
     if (rule.prelude.is_empty()) {
         dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @keyframes rule: Empty prelude.");
         return {};
     }
 
-    // FIXME: Is there some way of detecting if there is a block or not?
-
     auto prelude_stream = TokenStream { rule.prelude };
     prelude_stream.discard_whitespace();
     auto& token = prelude_stream.consume_a_token();
@@ -464,14 +465,13 @@ GC::Ptr<CSSNamespaceRule> Parser::convert_to_namespace_rule(AtRule const& rule)
     // https://drafts.csswg.org/css-namespaces/#syntax
     // @namespace <namespace-prefix>? [ <string> | <url> ] ;
     // <namespace-prefix> = <ident>
-
+    if (rule.is_block_rule) {
-    if (rule.prelude.is_empty()) {
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @namespace rule: Block is not allowed.");
-        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @namespace rule: Empty prelude.");
         return {};
     }
 
-    if (rule.is_block_rule) {
+    if (rule.prelude.is_empty()) {
-        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @namespace rule: Block is not allowed.");
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @namespace rule: Empty prelude.");
         return {};
     }
 
@@ -515,6 +515,10 @@ GC::Ptr<CSSSupportsRule> Parser::convert_to_supports_rule(AtRule const& rule, Ne
     // @supports <supports-condition> {
     //   <rule-list>
     // }
+    if (!rule.is_block_rule) {
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @supports rule: Expected a block.");
+        return {};
+    }
 
     if (rule.prelude.is_empty()) {
         dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @supports rule: Empty prelude.");
@@ -553,6 +557,10 @@ GC::Ptr<CSSPropertyRule> Parser::convert_to_property_rule(AtRule const& rule)
     // @property <custom-property-name> {
     // <declaration-list>
     // }
+    if (!rule.is_block_rule) {
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @property rule: Expected a block.");
+        return {};
+    }
 
     if (rule.prelude.is_empty()) {
         dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @property rule: Empty prelude.");
@@ -629,6 +637,11 @@ GC::Ptr<CSSPropertyRule> Parser::convert_to_property_rule(AtRule const& rule)
 GC::Ptr<CSSFontFaceRule> Parser::convert_to_font_face_rule(AtRule const& rule)
 {
     // https://drafts.csswg.org/css-fonts/#font-face-rule
+    if (!rule.is_block_rule) {
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @font-face rule: Expected a block.");
+        return nullptr;
+    }
+
     DescriptorList descriptors { AtRuleID::FontFace };
     rule.for_each_as_declaration_list([&](auto& declaration) {
         if (auto descriptor = convert_to_descriptor(AtRuleID::FontFace, declaration); descriptor.has_value()) {
@@ -643,6 +656,11 @@ GC::Ptr<CSSPageRule> Parser::convert_to_page_rule(AtRule const& page_rule)
 {
     // https://drafts.csswg.org/css-page-3/#syntax-page-selector
     // @page = @page <page-selector-list>? { <declaration-rule-list> }
+    if (!page_rule.is_block_rule) {
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @property rule: Expected a block.");
+        return nullptr;
+    }
+
     TokenStream tokens { page_rule.prelude };
     auto page_selectors = parse_a_page_selector_list(tokens);
     if (page_selectors.is_error())
@@ -672,6 +690,11 @@ GC::Ptr<CSSPageRule> Parser::convert_to_page_rule(AtRule const& page_rule)
 
 GC::Ptr<CSSMarginRule> Parser::convert_to_margin_rule(AtRule const& rule)
 {
+    if (!rule.is_block_rule) {
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse margin rule @{}: Expected a block.", rule.name);
+        return nullptr;
+    }
+
     // https://drafts.csswg.org/css-page-3/#syntax-page-selector
     // There are lots of these, but they're all in the format:
     // @foo = @foo { <declaration-list> };

Tests/LibWeb/Ref/expected/wpt-import/css/CSS2/reference/ref-this-text-should-be-green.xht

@@ -0,0 +1,22 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+
+<html xmlns="http://www.w3.org/1999/xhtml" lang="en">
+
+ <head>
+
+  <title>CSS Reftest Reference</title>
+
+  <link rel="author" title="Gérard Talbot" href="http://www.gtalbot.org/BrowserBugsSection/css21testsuite/" />
+
+  <style type="text/css"><![CDATA[
+  p {color: green;}
+  ]]></style>
+
+ </head>
+
+ <body>
+
+  <p>This text should be green.</p>
+
+ </body>
+</html>

Tests/LibWeb/Ref/input/wpt-import/css/CSS2/cascade/at-import-009.xht

@@ -0,0 +1,20 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml">
+    <head>
+        <title>CSS Test: @import following a known but invalid @rule</title>
+        <link rel="author" title="Elika J. Etemad" href="http://fantasai.inkedblade.net/contact"/>
+        <link rel="help" href="http://www.w3.org/TR/CSS21/syndata.html#at-rules" />
+        <link rel="match" href="../../../../../expected/wpt-import/css/CSS2/cascade/../reference/ref-this-text-should-be-green.xht"/>
+        <meta name="flags" content="invalid" />
+        <meta name="assert" content="@import is respected after known but ignored @rule." />
+        <style type="text/css">
+           @media;
+           @page;
+           @charset;
+           @import "support/import-green.css";
+        </style>
+    </head>
+    <body>
+        <p class="import">This text should be green.</p>
+    </body>
+</html>

Tests/LibWeb/Ref/input/wpt-import/css/CSS2/cascade/support/import-green.css

@@ -0,0 +1 @@
+.import { color: green; }

Tests/LibWeb/Text/expected/wpt-import/css/css-conditional/js/conditional-CSSGroupingRule.txt

@@ -2,8 +2,7 @@ Harness status: OK
 
 Found 34 tests
 
-32 Pass
+34 Pass
-2 Fail
 Pass	@media is CSSGroupingRule
 Pass	@media rule type
 Pass	Empty @media rule length
@@ -12,7 +11,7 @@ Pass	insertRule into empty @media at bad index
 Pass	insertRule into @media updates length
 Pass	insertRule of valid @media into @media
 Pass	insertRule of valid style rule into @media
-Fail	insertRule of invalid @media into @media
+Pass	insertRule of invalid @media into @media
 Pass	insertRule of empty string into @media
 Pass	insertRule of valid @media rule followed by garbage into @media
 Pass	insertRule of valid style rule followed by garbage into @media
@@ -29,7 +28,7 @@ Pass	insertRule into empty @supports at bad index
 Pass	insertRule into @supports updates length
 Pass	insertRule of valid @media into @supports
 Pass	insertRule of valid style rule into @supports
-Fail	insertRule of invalid @media into @supports
+Pass	insertRule of invalid @media into @supports
 Pass	insertRule of empty string into @supports
 Pass	insertRule of valid @media rule followed by garbage into @supports
 Pass	insertRule of valid style rule followed by garbage into @supports