Restrict floats over setCurrentTime & setCurrentScale to finite values

Restrict floats over setCurrentTime & setCurrentScale to finite values https://bugs.webkit.org/show_bug.cgi?id=249741 Reviewed by Simon Fraser. This patch is to align WebKit with Blink / Chromium, Gecko / Firefox and Web-Specification. Merge - https://chromium.googlesource.com/chromium/blink/+/b4e294d0dec2aeffa4d401e19916e753a7d74a75 This patch modifies "setCurrentTime" and "setCurrentScale" to work with finite floats and also update IDL definition aligned with current web-standard and also aligned with other browsers. * Source/WebCore/svg/SVGSVGElement.cpp: (SVGSVGElement::currentScale): Add ASSERT for finite 'scale' (SVGSVGElement::getCurrentTime): Change "if" to ASSERT for finite seconds * Source/WebCore/svg/SVGSVGElement.idl: Aligned with Web-Spec * LayoutTests/svg/dom/SVGSVGElement-currentTime.html: Add Test Case * LayoutTests/svg/dom/SVGSVGElement-currentTime-expected.txt: Add Test Case Expectation * LayoutTests/svg/dom/SVGSVGElement-currentScale-NaN-no-crash.html: Add Test Case * LayoutTests/svg/dom/SSVGSVGElement-currentScale-NaN-no-crash-expected.txt: Add Test Case Expectation * LayoutTests/svg/animations/animate-setcurrentime.html: Rebaselined * LayoutTests/svg/animations/animate-setcurrentime-expected.txt: Ditto Canonical link: https://commits.webkit.org/258322@main
WebKit · Dec 24, 2022 · e938348a700cfd5e7db4d10a905e8af922328996 · e938348
1 parent ffe153e
commit e938348a700cfd5e7db4d10a905e8af922328996
Show file tree

Hide file tree

Showing 8 changed files with 114 additions and 19 deletions.
diff --git a/LayoutTests/svg/animations/animate-setcurrenttime-expected.txt b/LayoutTests/svg/animations/animate-setcurrenttime-expected.txt
@@ -1,6 +1,11 @@
 SVG 1.1 dynamic animation tests
 
- PASS plain.x.animVal.value is 0
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS svg.setCurrentTime(test.time[0]) threw exception TypeError: The provided value is non-finite.
+PASS nestedsvg.setCurrentTime(test.time[1]) threw exception TypeError: The provided value is non-finite.
+PASS plain.x.animVal.value is 0
 PASS sequential.x.animVal.value is 0
 PASS accumulating.x.animVal.value is 0
 PASS repeating.x.animVal.value is 0
@@ -90,4 +95,7 @@ PASS sequential.x.animVal.value is 128
 PASS accumulating.x.animVal.value is 128
 PASS repeating.x.animVal.value is 128
 PASS nested.x.animVal.value is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
 
diff --git a/LayoutTests/svg/animations/animate-setcurrenttime.html b/LayoutTests/svg/animations/animate-setcurrenttime.html
@@ -1,12 +1,10 @@
 <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
 <html>
 <head>
-  <script src="../../resources/js-test-pre.js"></script>
+  <script src="../../resources/js-test.js"></script>
 </head>
 
 <body>
-  <h1>SVG 1.1 dynamic animation tests</h1>
-
   <svg id='outer-svg' xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' xml:space='preserve'>
     <!-- sequential animations -->
     <rect id='plain' x='0' y='0' width='32' height='32' fill='green'>
@@ -37,6 +35,9 @@ <h1>SVG 1.1 dynamic animation tests</h1>
     </svg>
 
     <script>
+    description("SVG 1.1 dynamic animation tests");
+    self.jsTestIsAsync = true;
+
     var svg = document.getElementById('outer-svg'),
         nestedsvg = document.getElementById('nested-svg');
     var tests, curIdx = 0;
@@ -46,22 +47,28 @@ <h1>SVG 1.1 dynamic animation tests</h1>
     var accumulating = document.getElementById('accumulating');
     var repeating = document.getElementById('repeating');
     var nested = document.getElementById('nested');
+    var test;
 
     function runTest() {
-      var test = tests[curIdx++];
+      test = tests[curIdx++];
+
+      if (test.throws && test.throws[0])
+        shouldThrow("svg.setCurrentTime(test.time[0])");
+      else
+        svg.setCurrentTime(test.time[0]);
 
-      svg.setCurrentTime(test.time[0]);
-      nestedsvg.setCurrentTime(test.time[1]);
+      if (test.throws && test.throws[1])
+        shouldThrow("nestedsvg.setCurrentTime(test.time[1])");
+      else
+        nestedsvg.setCurrentTime(test.time[1]);
 
       setTimeout(function() {
         for (var attr in test.values) {
           shouldBe(attr + '.animVal.value', String(test.values[attr]));
         }
 
-        if (curIdx == tests.length) {
-          if (window.testRunner)
-            testRunner.notifyDone();
-        }
+        if (curIdx == tests.length)
+          finishJSTest();
         else
           runTest();
       }, 0);
@@ -73,7 +80,7 @@ <h1>SVG 1.1 dynamic animation tests</h1>
 
       tests = [
         // Test invalid values.
-        { time: ['tintin', NaN], values: { 'plain.x':   0, 'sequential.x':   0, 'accumulating.x':   0, 'repeating.x':   0, 'nested.x':   0 } },
+        { time: ['tintin', NaN], throws: [true, true], values: { 'plain.x':   0, 'sequential.x':   0, 'accumulating.x':   0, 'repeating.x':   0, 'nested.x':   0 } },
 
         // Test out-of-range values.
         { time: [-1, -1], values: { 'plain.x':   0, 'sequential.x':   0, 'accumulating.x':   0, 'repeating.x':   0, 'nested.x':   0 } },
@@ -117,4 +124,3 @@ <h1>SVG 1.1 dynamic animation tests</h1>
   <div id="console"></div>
 </body>
 </html>
-
diff --git a/LayoutTests/svg/dom/SVGSVGElement-currentScale-NaN-no-crash-expected.txt b/LayoutTests/svg/dom/SVGSVGElement-currentScale-NaN-no-crash-expected.txt
@@ -0,0 +1,11 @@
+Setting .currentScale to a non-finite should throw.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS svgElement.currentScale = NaN threw exception TypeError: The provided value is non-finite.
+PASS svgElement.currentScale = Infinity threw exception TypeError: The provided value is non-finite.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/svg/dom/SVGSVGElement-currentScale-NaN-no-crash.html b/LayoutTests/svg/dom/SVGSVGElement-currentScale-NaN-no-crash.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+</head>
+<body>
+<svg id="svg" xmlns="http://www.w3.org/2000/svg" width="200" height="200"></svg>
+<script>
+description("Setting .currentScale to a non-finite should throw.");
+
+self.jsTestIsAsync = true;
+
+if (window.testRunner) {
+     testRunner.dumpAsText();
+     testRunner.waitUntilDone();
+}
+
+function runTest()
+{
+    svgElement = document.getElementById("svg");
+    shouldThrow("svgElement.currentScale = NaN");
+    shouldThrow("svgElement.currentScale = Infinity");
+    finishJSTest();
+}
+window.onload = runTest;
+</script>
+</body>
+</html>
diff --git a/LayoutTests/svg/dom/SVGSVGElement-currentTime-expected.txt b/LayoutTests/svg/dom/SVGSVGElement-currentTime-expected.txt
@@ -0,0 +1,11 @@
+Calling svgElement.setCurrentTime() with a non-finite value should throw.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS svgElement.setCurrentTime(NaN) threw exception TypeError: The provided value is non-finite.
+PASS svgElement.setCurrentTime(Infinity) threw exception TypeError: The provided value is non-finite.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/svg/dom/SVGSVGElement-currentTime.html b/LayoutTests/svg/dom/SVGSVGElement-currentTime.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+</head>
+<body>
+<svg id="svg" xmlns="http://www.w3.org/2000/svg" width="200" height="200"></svg>
+<script>
+description("Calling svgElement.setCurrentTime() with a non-finite value should throw.");
+
+self.jsTestIsAsync = true;
+
+if (window.testRunner) {
+     testRunner.dumpAsText();
+     testRunner.waitUntilDone();
+}
+
+function runTest()
+{
+    svgElement = document.getElementById("svg");
+    shouldThrow("svgElement.setCurrentTime(NaN)");
+    shouldThrow("svgElement.setCurrentTime(Infinity)");
+    finishJSTest();
+}
+window.onload = runTest;
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/svg/SVGSVGElement.cpp b/Source/WebCore/svg/SVGSVGElement.cpp
@@ -1,7 +1,8 @@
 /*
  * Copyright (C) 2004, 2005, 2006, 2019 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2010 Rob Buis <buis@kde.org>
- * Copyright (C) 2007-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2007-2022 Apple Inc. All rights reserved.
+ * Copyright (C) 2015 Google Inc. All rights reserved.
  * Copyright (C) 2014 Adobe Systems Incorporated. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ -125,6 +126,7 @@ float SVGSVGElement::currentScale() const
 
 void SVGSVGElement::setCurrentScale(float scale)
 {
+    ASSERT(std::isfinite(scale));
     if (auto frame = frameForCurrentScale())
         frame->setPageZoomFactor(scale);
 }
@@ -546,8 +548,7 @@ float SVGSVGElement::getCurrentTime() const
 
 void SVGSVGElement::setCurrentTime(float seconds)
 {
-    if (!std::isfinite(seconds))
-        return;
+    ASSERT(std::isfinite(seconds));
     m_timeContainer->setElapsed(std::max(seconds, 0.0f));
 }
 

diff --git a/Source/WebCore/svg/SVGSVGElement.idl b/Source/WebCore/svg/SVGSVGElement.idl
@@ -2,7 +2,7 @@
  * Copyright (C) 2004, 2005 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2004, 2005, 2010 Rob Buis <buis@kde.org>
  * Copyright (C) 2006 Samuel Weinig <sam.weinig@gmail.com>
- * Copyright (C) 2006-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2022 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -20,6 +20,8 @@
  * Boston, MA 02110-1301, USA.
  */
 
+// https://www.w3.org/TR/SVG2/struct.html#InterfaceSVGSVGElement
+
 [
     JSGenerateToNativeObject,
     ExportMacro=WEBCORE_EXPORT,
@@ -59,8 +61,8 @@
     undefined pauseAnimations();
     undefined unpauseAnimations();
     boolean animationsPaused();
-    unrestricted float getCurrentTime();
-    undefined setCurrentTime(optional unrestricted float seconds = NaN);
+    float getCurrentTime();
+    undefined setCurrentTime(float seconds);
 
     // Deprecated SVG redrawing
     unsigned long suspendRedraw(optional unsigned long maxWaitMilliseconds = 0);