Changeset 292816 in webkit

Timestamp:

Apr 13, 2022 11:48:19 AM (3 months ago)

Author:

Antti Koivisto

Message:

[CSS Container Queries] Limit query range syntax
​https://bugs.webkit.org/show_bug.cgi?id=239118

Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

• web-platform-tests/css/css-contain/container-queries/at-container-parsing-expected.txt:

Source/WebCore:

The spec disallows things like (100px = width < 200px) and (100px < width > 200px).

​https://www.w3.org/TR/mediaqueries-4/#mq-range-context

• css/ContainerQueryParser.cpp:

(WebCore::ContainerQueryParser::consumeContainerQuery):

Try as a condition first.
Return UnknownQuery on parse failure.

(WebCore::ContainerQueryParser::consumeRangeSizeFeature):

Validate the ranges so what ends up being allowed matches the spec productions.

Location:

trunk

Files:

• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/at-container-parsing-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/ContainerQueryParser.cpp (modified) (7 diffs)

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2022-04-13  Antti Koivisto  <antti@apple.com>
+
+        [CSS Container Queries] Limit query range syntax
+        https://bugs.webkit.org/show_bug.cgi?id=239118
+
+        Reviewed by Simon Fraser.
+
+        * web-platform-tests/css/css-contain/container-queries/at-container-parsing-expected.txt:
+
 2022-04-13  Youenn Fablet  <youenn@apple.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/at-container-parsing-expected.txt

@@ -52 +52 @@
 PASS (grid)
 PASS (width == 100px)
-FAIL (100px == width) assert_equals: expected 2 but got 0
-FAIL (100px = width = 200px) assert_equals: expected "" but got "true"
-FAIL (100px < width > 200px) assert_equals: expected "" but got "true"
-FAIL (100px <= width >= 200px) assert_equals: expected "" but got "true"
-FAIL (100px <= width > 200px) assert_equals: expected "" but got "true"
-FAIL (100px < width >= 200px) assert_equals: expected "" but got "true"
-FAIL (100px : width : 200px) assert_equals: expected 2 but got 0
+PASS (100px == width)
+PASS (100px = width = 200px)
+PASS (100px < width > 200px)
+PASS (100px <= width >= 200px)
+PASS (100px <= width > 200px)
+PASS (100px < width >= 200px)
+PASS (100px : width : 200px)
 PASS screen
 PASS print

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-04-13  Antti Koivisto  <antti@apple.com>
+
+        [CSS Container Queries] Limit query range syntax
+        https://bugs.webkit.org/show_bug.cgi?id=239118
+
+        Reviewed by Simon Fraser.
+
+        The spec disallows things like (100px = width < 200px) and (100px < width > 200px).
+
+        https://www.w3.org/TR/mediaqueries-4/#mq-range-context
+
+        * css/ContainerQueryParser.cpp:
+        (WebCore::ContainerQueryParser::consumeContainerQuery):
+
+        Try as a condition first.
+        Return UnknownQuery on parse failure.
+
+        (WebCore::ContainerQueryParser::consumeRangeSizeFeature):
+
+        Validate the ranges so what ends up being allowed matches the spec productions.
+
 2022-04-13  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/css/ContainerQueryParser.cpp

@@ -74 +74 @@
         blockRange.consumeWhitespace();
 
-        // Try to parse as a size query first.
-        auto blockForSizeFeature = blockRange;
-        if (auto sizeFeature = consumeSizeFeature(blockForSizeFeature))
+        // Try to parse as a condition first.
+        auto conditionRange = blockRange;
+        if (auto condition = consumeCondition<CQ::ContainerCondition>(conditionRange))
+            return { condition };
+
+        if (auto sizeFeature = consumeSizeFeature(blockRange))
             return { *sizeFeature };
 
-        if (auto condition = consumeCondition<CQ::ContainerCondition>(blockRange))
-            return { condition };
+        return CQ::UnknownQuery { { }, blockRange.serialize() };
     }
 
@@ -156 +158 @@
 
     auto sizeFeature = consume();
+
+    if (!range.atEnd())
+        return { };
+
     if (sizeFeature)
         m_requiredAxes.add(CQ::requiredAxesForFeature(sizeFeature->name));
@@ -204 +210 @@
 
     auto value = consumeValue(range);
+    if (!value)
+        return { };
 
     return CQ::SizeFeature { featureName, CQ::Syntax::Colon, { }, CQ::Comparison { op, WTFMove(value) } };
@@ -240 +248 @@
     };
 
+    bool didFailParsing = false;
+
     auto consumeLeftComparison = [&]() -> std::optional<CQ::Comparison> {
         if (range.peek().type() == IdentToken)
             return { };
         auto value = consumeValue(range);
+        if (!value)
+            return { };
         auto op = consumeRangeOperator();
-        if (!op)
-            return { };
+        if (!op) {
+            didFailParsing = true;
+            return { };
+        }
 
         return CQ::Comparison { *op, WTFMove(value) };
@@ -256 +270 @@
             return { };
         auto value = consumeValue(range);
+        if (!value) {
+            didFailParsing = true;
+            return { };
+        }
 
         return CQ::Comparison { *op, WTFMove(value) };
@@ -268 +286 @@
     auto rightComparison = consumeRightComparison();
 
-    if (!leftComparison && !rightComparison)
+    auto validateComparisons = [&] {
+        if (didFailParsing)
+            return false;
+        if (!leftComparison && !rightComparison)
+            return false;
+        if (!leftComparison || !rightComparison)
+            return true;
+        // Disallow comparisons like (a=b=c), (a=b<c).
+        if (leftComparison->op == CQ::ComparisonOperator::Equal || rightComparison->op == CQ::ComparisonOperator::Equal)
+            return false;
+        // Disallow comparisons like (a<b>c).
+        bool leftIsLess = leftComparison->op == CQ::ComparisonOperator::LessThan || leftComparison->op == CQ::ComparisonOperator::LessThanOrEqual;
+        bool rightIsLess = rightComparison->op == CQ::ComparisonOperator::LessThan || rightComparison->op == CQ::ComparisonOperator::LessThanOrEqual;
+        return leftIsLess == rightIsLess;
+    };
+
+    if (!validateComparisons())
         return { };
 
@@ -284 +318 @@
     if (auto value = CSSPropertyParserHelpers::consumeAspectRatioValue(range))
         return value;
-    range.consumeIncludingWhitespace();
     return nullptr;
 }