Changeset 241608 in webkit

Timestamp:

Feb 15, 2019, 1:13:06 PM (8 years ago)

Author:

pvollan@apple.com

Message:

[WebVTT] Inline WebVTT styles should start with '::cue'
​https://bugs.webkit.org/show_bug.cgi?id=194227

Reviewed by Eric Carlson.

Source/WebCore:

The original fix in r241203 is not sufficient, since it only checks if the CSS string starts
with '::cue'. Before accepting a CSS string from a WebVTT file, it should be checked that
all selectors starts with '::cue'.

Test: media/track/track-cue-css.html

• html/track/WebVTTParser.cpp:

(WebCore::WebVTTParser::checkAndStoreStyleSheet):

LayoutTests:

Add invalid 'STYLE' blocks which the WebVTT parser should reject.

• media/track/captions-webvtt/css-styling.vtt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/track/captions-webvtt/css-styling.vtt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/track/WebVTTParser.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-02-15  Per Arne Vollan  <pvollan@apple.com>
+
+        [WebVTT] Inline WebVTT styles should start with '::cue'
+        https://bugs.webkit.org/show_bug.cgi?id=194227
+
+        Reviewed by Eric Carlson.
+
+        Add invalid 'STYLE' blocks which the WebVTT parser should reject.
+
+        * media/track/captions-webvtt/css-styling.vtt:
+
 2019-02-15  Per Arne Vollan  <pvollan@apple.com>
 

trunk/LayoutTests/media/track/captions-webvtt/css-styling.vtt

@@ -32 +32 @@
 }
 
+NOTE the following style block should be discarded since it has a 'video::cue' selector.
+
+STYLE
+::cue {
+color: blue
+font-size: 25px;
+}
+video::cue {
+color: blue;
+font-size: 25px;
+}
+
+NOTE the following style blocks should be discarded since they are invalid in WebVTT files.
+
+STYLE
+::cue,video::cue {
+color: blue;
+font-size: 25px;
+}
+
+STYLE
+color: yellow;
+
+NOTE @import and @namespace CSS rules should not be allowed in WebVTT files.
+NOTE TODO: create a proper testcase for this, see https://bugs.webkit.org/show_bug.cgi?id=194708.
+
+STYLE
+@import url('test.css');
+
+STYLE
+@namespace Foo "test";
+
+
 hello
 00:00:00.000 --> 00:00:10.000

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-02-15  Per Arne Vollan  <pvollan@apple.com>
+
+        [WebVTT] Inline WebVTT styles should start with '::cue'
+        https://bugs.webkit.org/show_bug.cgi?id=194227
+
+        Reviewed by Eric Carlson.
+
+        The original fix in r241203 is not sufficient, since it only checks if the CSS string starts
+        with '::cue'. Before accepting a CSS string from a WebVTT file, it should be checked that
+        all selectors starts with '::cue'.
+
+        Test: media/track/track-cue-css.html
+
+        * html/track/WebVTTParser.cpp:
+        (WebCore::WebVTTParser::checkAndStoreStyleSheet):
+
 2019-02-15  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/html/track/WebVTTParser.cpp

@@ -40 +40 @@
 #include "ISOVTTCue.h"
 #include "ProcessingInstruction.h"
+#include "StyleRule.h"
+#include "StyleRuleImport.h"
 #include "StyleSheetContents.h"
 #include "Text.h"
@@ -370 +372 @@
         return false;
     
-    auto styleSheet = m_currentStyleSheet.stripWhiteSpace();
-    
-    // Inline VTT styles must start with ::cue.
-    if (!styleSheet.startsWith("::cue")) {
-        m_currentStyleSheet = emptyString();
+    auto styleSheet = WTFMove(m_currentStyleSheet);
+    
+    auto contents = StyleSheetContents::create();
+    if (!contents->parseString(styleSheet))
         return true;
-    }
-
-    auto contents = StyleSheetContents::create();
-    if (!contents->parseString(styleSheet)) {
-        m_currentStyleSheet = emptyString();
+
+    auto& namespaceRules = contents->namespaceRules();
+    if (namespaceRules.size())
         return true;
-    }
-    
-    m_styleSheets.append(WTFMove(m_currentStyleSheet));
+
+    auto& importRules = contents->importRules();
+    if (importRules.size())
+        return true;
+
+    auto& childRules = contents->childRules();
+    if (!childRules.size())
+        return true;
+    
+    for (auto rule : childRules) {
+        if (!rule->isStyleRule())
+            return true;
+        const auto& styleRule = downcast<StyleRule>(rule.get());
+
+        const auto& selectorList = styleRule->selectorList();
+        if (selectorList.listSize() != 1)
+            return true;
+        auto selector = selectorList.selectorAt(0);
+        if (selector->selectorText() != "::cue")
+            return true;
+    }
+
+    m_styleSheets.append(styleSheet);
     return true;
 }