REGRESSION (Safari 11): custom <font-face> tag crashes a page
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Dec 2017 22:51:02 +0000 (22:51 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Dec 2017 22:51:02 +0000 (22:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177848

Reviewed by Darin Adler.

Source/WebCore:

We currently use the CSS property parsers to parse SVG's <font-face> element attributes. Instead,
we should be using the CSS descriptor parsers to parse these attributes. However, this is a
fairly involved task, so until I can finish that, this patch fixes the crash. The crash is simple;
the descriptors shouldn't accept the universal keywords ("initial", "inherit", etc.) and our
font-face machinery assumes this. So the fix is just detect these keywords and explicitly disallow
them.

Test: svg/text/font-style-keyword.html

* svg/SVGFontFaceElement.cpp:
(WebCore::SVGFontFaceElement::parseAttribute):

LayoutTests:

* svg/text/font-style-keyword-expected.txt: Added.
* svg/text/font-style-keyword.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225808 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/svg/text/font-style-keyword-expected.txt [new file with mode: 0644]
LayoutTests/svg/text/font-style-keyword.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSValue.h
Source/WebCore/svg/SVGFontFaceElement.cpp

index 5feba93..d2b9e7c 100644 (file)
@@ -1,3 +1,13 @@
+2017-12-12  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION (Safari 11): custom <font-face> tag crashes a page
+        https://bugs.webkit.org/show_bug.cgi?id=177848
+
+        Reviewed by Darin Adler.
+
+        * svg/text/font-style-keyword-expected.txt: Added.
+        * svg/text/font-style-keyword.html: Added.
+
 2017-12-12  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Implement the playState property on Animation
diff --git a/LayoutTests/svg/text/font-style-keyword-expected.txt b/LayoutTests/svg/text/font-style-keyword-expected.txt
new file mode 100644 (file)
index 0000000..8b13789
--- /dev/null
@@ -0,0 +1 @@
+
diff --git a/LayoutTests/svg/text/font-style-keyword.html b/LayoutTests/svg/text/font-style-keyword.html
new file mode 100644 (file)
index 0000000..0294930
--- /dev/null
@@ -0,0 +1,34 @@
+<html>
+<head>
+</head>
+<body>
+<!-- This test makes sure there is no crash when specifying "font-style=initial" on a <font-face> element. The test passes if there is no crash. -->
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+  <svg>
+    <font-face font-style="initial" font-family="any">
+      <font-face-src>
+        <font-face-name name="any"></font-face-name>
+      </font-face-src>
+    </font-face>
+    <font-face font-style="inherit" font-family="any">
+      <font-face-src>
+        <font-face-name name="any"></font-face-name>
+      </font-face-src>
+    </font-face>
+    <font-face font-style="unset" font-family="any">
+      <font-face-src>
+        <font-face-name name="any"></font-face-name>
+      </font-face-src>
+    </font-face>
+    <font-face font-style="revert" font-family="any">
+      <font-face-src>
+        <font-face-name name="any"></font-face-name>
+      </font-face-src>
+    </font-face>
+  </svg>
+</body>
+
+</html>
index f762671..3dbf259 100644 (file)
@@ -1,3 +1,22 @@
+2017-12-12  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION (Safari 11): custom <font-face> tag crashes a page
+        https://bugs.webkit.org/show_bug.cgi?id=177848
+
+        Reviewed by Darin Adler.
+
+        We currently use the CSS property parsers to parse SVG's <font-face> element attributes. Instead,
+        we should be using the CSS descriptor parsers to parse these attributes. However, this is a
+        fairly involved task, so until I can finish that, this patch fixes the crash. The crash is simple;
+        the descriptors shouldn't accept the universal keywords ("initial", "inherit", etc.) and our
+        font-face machinery assumes this. So the fix is just detect these keywords and explicitly disallow
+        them.
+
+        Test: svg/text/font-style-keyword.html
+
+        * svg/SVGFontFaceElement.cpp:
+        (WebCore::SVGFontFaceElement::parseAttribute):
+
 2017-12-12  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Implement the playState property on Animation
index 42bdd07..c86c33e 100644 (file)
@@ -91,6 +91,7 @@ public:
     bool isInitialValue() const { return m_classType == InitialClass; }
     bool isUnsetValue() const { return m_classType == UnsetClass; }
     bool isRevertValue() const { return m_classType == RevertClass; }
+    bool isGlobalKeyword() const { return isInheritedValue() || isInitialValue() || isUnsetValue() || isRevertValue(); }
     bool treatAsInitialValue(CSSPropertyID) const;
     bool treatAsInheritedValue(CSSPropertyID) const;
     bool isLinearGradientValue() const { return m_classType == LinearGradientClass; }
index 3713f4b..21df8a4 100644 (file)
@@ -63,10 +63,22 @@ Ref<SVGFontFaceElement> SVGFontFaceElement::create(const QualifiedName& tagName,
 
 void SVGFontFaceElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {    
-    CSSPropertyID propId = cssPropertyIdForSVGAttributeName(name);
-    if (propId > 0) {
+    CSSPropertyID propertyId = cssPropertyIdForSVGAttributeName(name);
+    if (propertyId > 0) {
         // FIXME: Parse using the @font-face descriptor grammars, not the property grammars.
-        m_fontFaceRule->mutableProperties().setProperty(propId, value, false);
+        auto& properties = m_fontFaceRule->mutableProperties();
+        bool valueChanged = properties.setProperty(propertyId, value);
+
+        if (valueChanged) {
+            // The above parser is designed for the font-face properties, not descriptors, and the properties accept the global keywords, but descriptors don't.
+            // Rather than invasively modifying the parser for the properties to have a special mode, we can simply detect the error condition after-the-fact and
+            // avoid it explicitly.
+            if (auto parsedValue = properties.getPropertyCSSValue(propertyId)) {
+                if (parsedValue->isGlobalKeyword())
+                    properties.removeProperty(propertyId);
+            }
+        }
+
         rebuildFontFace();
         return;
     }