Merge r164933 - Ensure keySplines is valid in SMIL animations

<http://webkit.org/b/129547> <rdar://problem/15676128> Reviewed by Darin Adler. Merged from Blink (patch by Philip Rogers): https://src.chromium.org/viewvc/blink?revision=156452&view=revision http://crbug.com/276111 This patch fixes a crash in SMIL animations when keySplines are not specified. The SMIL spec is clear on this: http://www.w3.org/TR/2001/REC-smil-animation-20010904/#AnimFuncCalcMode "If there are any errors in the keyTimes specification (bad values, too many or too few values), the animation will have no effect." This patch simply checks that keyTimes is not empty. Previously, splinesCount was set to be m_keySplines.size() + 1 in SVGAnimationElement.cpp; this patch changes splinesCount to be equal to m_keySplines.size() to make the logic easier to follow and to match other checks in SVGAnimationElement::startedActiveInterval. Source/WebCore: Test: svg/animations/animate-keysplines-crash.html * svg/SVGAnimationElement.cpp: (WebCore::SVGAnimationElement::startedActiveInterval): LayoutTests: * svg/animations/animate-keysplines-crash-expected.txt: Added. * svg/animations/animate-keysplines-crash.html: Added.
WebKit · Apr 14, 2014 · e1b99c0 · e1b99c0
1 parent a95aa0c
commit e1b99c0
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 4 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,30 @@
+2014-03-01  David Kilzer  <ddkilzer@apple.com>
+
+        Ensure keySplines is valid in SMIL animations
+        <http://webkit.org/b/129547>
+        <rdar://problem/15676128>
+
+        Reviewed by Darin Adler.
+
+        Merged from Blink (patch by Philip Rogers):
+        https://src.chromium.org/viewvc/blink?revision=156452&view=revision
+        http://crbug.com/276111
+
+            This patch fixes a crash in SMIL animations when keySplines are not
+            specified. The SMIL spec is clear on this:
+            http://www.w3.org/TR/2001/REC-smil-animation-20010904/#AnimFuncCalcMode
+            "If there are any errors in the keyTimes specification (bad values,
+            too many or too few values), the animation will have no effect."
+
+            This patch simply checks that keyTimes is not empty. Previously,
+            splinesCount was set to be m_keySplines.size() + 1 in
+            SVGAnimationElement.cpp; this patch changes splinesCount to be equal
+            to m_keySplines.size() to make the logic easier to follow and to
+            match other checks in SVGAnimationElement::startedActiveInterval.
+
+        * svg/animations/animate-keysplines-crash-expected.txt: Added.
+        * svg/animations/animate-keysplines-crash.html: Added.
+
 2014-02-19  Daniel Bates  <dabates@apple.com>
 
         Do not dispatch change event twice in single step action

diff --git a/LayoutTests/svg/animations/animate-keysplines-crash-expected.txt b/LayoutTests/svg/animations/animate-keysplines-crash-expected.txt
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/svg/animations/animate-keysplines-crash.html b/LayoutTests/svg/animations/animate-keysplines-crash.html
@@ -0,0 +1,28 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+Test for crbug.com/276111: This test passes if it does not crash.
+<svg xmlns="http://www.w3.org/2000/svg">
+  <rect>
+      <animateMotion path="M 1 2Z" id="animateMotionElement" calcMode="spline" values="M 1 2Z; M3 4Z"/>
+  </rect>
+</svg>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+function removePathAndFinishTest() {
+    animateMotionElement.removeAttribute('path');
+    setTimeout(function() {
+        document.write("PASS");
+        if (window.testRunner)
+            testRunner.notifyDone();
+    }, 0);
+}
+
+setTimeout('removePathAndFinishTest()', 0);
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2014-03-01  David Kilzer  <ddkilzer@apple.com>
+
+        Ensure keySplines is valid in SMIL animations
+        <http://webkit.org/b/129547>
+        <rdar://problem/15676128>
+
+        Reviewed by Darin Adler.
+
+        Merged from Blink (patch by Philip Rogers):
+        https://src.chromium.org/viewvc/blink?revision=156452&view=revision
+        http://crbug.com/276111
+
+            This patch fixes a crash in SMIL animations when keySplines are not
+            specified. The SMIL spec is clear on this:
+            http://www.w3.org/TR/2001/REC-smil-animation-20010904/#AnimFuncCalcMode
+            "If there are any errors in the keyTimes specification (bad values,
+            too many or too few values), the animation will have no effect."
+
+            This patch simply checks that keyTimes is not empty. Previously,
+            splinesCount was set to be m_keySplines.size() + 1 in
+            SVGAnimationElement.cpp; this patch changes splinesCount to be equal
+            to m_keySplines.size() to make the logic easier to follow and to
+            match other checks in SVGAnimationElement::startedActiveInterval.
+
+        Test: svg/animations/animate-keysplines-crash.html
+
+        * svg/SVGAnimationElement.cpp:
+        (WebCore::SVGAnimationElement::startedActiveInterval):
+
 2014-02-28  Bem Jones-Bey  <bjonesbe@adobe.com>
 
         Properly clear m_logicallyLastRun to remove use-after-free possibility

diff --git a/Source/WebCore/svg/SVGAnimationElement.cpp b/Source/WebCore/svg/SVGAnimationElement.cpp
@@ -560,10 +560,11 @@ void SVGAnimationElement::startedActiveInterval()
     AnimationMode animationMode = this->animationMode();
     CalcMode calcMode = this->calcMode();
     if (calcMode == CalcModeSpline) {
-        unsigned splinesCount = m_keySplines.size() + 1;
-        if ((fastHasAttribute(SVGNames::keyPointsAttr) && m_keyPoints.size() != splinesCount)
-            || (animationMode == ValuesAnimation && m_values.size() != splinesCount)
-            || (fastHasAttribute(SVGNames::keyTimesAttr) && m_keyTimes.size() != splinesCount))
+        unsigned splinesCount = m_keySplines.size();
+        if (!splinesCount
+            || (fastHasAttribute(SVGNames::keyPointsAttr) && m_keyPoints.size() - 1 != splinesCount)
+            || (animationMode == ValuesAnimation && m_values.size() - 1 != splinesCount)
+            || (fastHasAttribute(SVGNames::keyTimesAttr) && m_keyTimes.size() - 1 != splinesCount))
             return;
     }