Merge r176170 - Assertion hit when setting a very large value to 'bor…

…der-width' / 'font-size' CSS properties https://bugs.webkit.org/show_bug.cgi?id=138770 Reviewed by Darin Adler. Source/WebCore: When setting a very large value to a CSS property, it is represented internally as infinity. r166114 already tried to deal with this by adding an std::isinf() check in CSSValuePool::createValue() and returning an identifier CSSPrimitiveValue with CSSValueInvalid value in such case. The issue is that doing leads to the StyleBuilder getting a CSSValueInvalid CSSPrimitive value as input, which is not handled and leads to assertions. This patch updates the CSSParser to detect cases where the double value is infinity earlier (in CSSParser::validUnit() and parseSimpleLengthValue()), so that we mark the value as invalid and actually drop it. As a result, CSSPrimitiveValues with CSSValueInvalid value no longer make their way to the StyleBuilder. Test: fast/css/style-builder-infinite-value.html fast/css/infinite-floating-value.html * css/CSSParser.cpp: (WebCore::parseSimpleLengthValue): (WebCore::CSSParser::validUnit): * css/CSSValuePool.cpp: (WebCore::CSSValuePool::createValue): LayoutTests: Add a layout test setting very large values to 'border-width' and 'font-size' CSS properties. * fast/css/style-builder-infinite-value-expected.txt: Added. * fast/css/style-builder-infinite-value.html: Added. Canonical link: https://commits.webkit.org/154760.215@webkitgtk/2.6 git-svn-id: https://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.6@176195 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Nov 17, 2014 · 66b22ef · 66b22ef
1 parent fc1436d
commit 66b22ef
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 2 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2014-11-16  Chris Dumez  <cdumez@apple.com>
+
+        Assertion hit when setting a very large value to 'border-width' / 'font-size' CSS properties
+        https://bugs.webkit.org/show_bug.cgi?id=138770
+
+        Reviewed by Darin Adler.
+
+        Add a layout test setting very large values to 'border-width' and
+        'font-size' CSS properties.
+
+        * fast/css/style-builder-infinite-value-expected.txt: Added.
+        * fast/css/style-builder-infinite-value.html: Added.
+
 2014-11-16  Shivakumar JM  <shiva.jm@samsung.com>
 
         Attribute text in HTMLAnchorElement should behave as per specification.

diff --git a/LayoutTests/fast/css/style-builder-infinite-value-expected.txt b/LayoutTests/fast/css/style-builder-infinite-value-expected.txt
@@ -0,0 +1,15 @@
+Tests setting a very large value to several CSS properties.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS testDiv.style['border-width'] is "1px"
+PASS testDiv.style['border-width'] is "1px"
+PASS testDiv.style['font-size'] is "1em"
+PASS testDiv.style['font-size'] is "1em"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+This test passes if it does not crash.
+
+
diff --git a/LayoutTests/fast/css/style-builder-infinite-value.html b/LayoutTests/fast/css/style-builder-infinite-value.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<script src="../../resources/js-test-pre.js"></script>
+<body>
+<p>This test passes if it does not crash.</p>
+
+<div id="testDiv"></div>
+
+<script>
+description("Tests setting a very large value to several CSS properties.");
+
+var testDiv = document.getElementById("testDiv");
+testDiv.style["border-width"] = "1px";
+shouldBeEqualToString("testDiv.style['border-width']", "1px");
+testDiv.style["border-width"] = "900000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000px";
+shouldBeEqualToString("testDiv.style['border-width']", "1px");
+
+testDiv.style["font-size"] = "1em";
+shouldBeEqualToString("testDiv.style['font-size']", "1em");
+testDiv.style["font-size"] = "900000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000%";
+shouldBeEqualToString("testDiv.style['font-size']", "1em");
+</script>
+<script src="../../resources/js-test-post.js"></script>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2014-11-16  Chris Dumez  <cdumez@apple.com>
+
+        Assertion hit when setting a very large value to 'border-width' / 'font-size' CSS properties
+        https://bugs.webkit.org/show_bug.cgi?id=138770
+
+        Reviewed by Darin Adler.
+
+        When setting a very large value to a CSS property, it is represented internally as
+        infinity. r166114 already tried to deal with this by adding an std::isinf() check
+        in CSSValuePool::createValue() and returning an identifier CSSPrimitiveValue with
+        CSSValueInvalid value in such case. The issue is that doing leads to the
+        StyleBuilder getting a CSSValueInvalid CSSPrimitive value as input, which is not
+        handled and leads to assertions.
+
+        This patch updates the CSSParser to detect cases where the double value is
+        infinity earlier (in CSSParser::validUnit() and parseSimpleLengthValue()), so
+        that we mark the value as invalid and actually drop it. As a result,
+        CSSPrimitiveValues with CSSValueInvalid value no longer make their way to the
+        StyleBuilder.
+
+        Test: fast/css/style-builder-infinite-value.html
+              fast/css/infinite-floating-value.html
+
+        * css/CSSParser.cpp:
+        (WebCore::parseSimpleLengthValue):
+        (WebCore::CSSParser::validUnit):
+        * css/CSSValuePool.cpp:
+        (WebCore::CSSValuePool::createValue):
+
 2014-11-16  Shivakumar JM  <shiva.jm@samsung.com>
 
         Attribute text in HTMLAnchorElement should behave as per specification.

diff --git a/Source/WebCore/css/CSSParser.cpp b/Source/WebCore/css/CSSParser.cpp
@@ -631,6 +631,8 @@ static bool parseSimpleLengthValue(MutableStyleProperties* declaration, CSSPrope
     }
     if (number < 0 && !acceptsNegativeNumbers)
         return false;
+    if (std::isinf(number))
+        return false;
 
     RefPtr<CSSValue> value = cssValuePool().createValue(number, unit);
     declaration->addParsedProperty(CSSProperty(propertyId, value.release(), important));
@@ -1701,6 +1703,8 @@ bool CSSParser::validUnit(CSSParserValue* value, Units unitflags, CSSParserMode
     }
     if (b && unitflags & FNonNeg && value->fValue < 0)
         b = false;
+    if (b && std::isinf(value->fValue))
+        b = false;
     return b;
 }
 

diff --git a/Source/WebCore/css/CSSValuePool.cpp b/Source/WebCore/css/CSSValuePool.cpp
@@ -89,8 +89,7 @@ PassRef<CSSPrimitiveValue> CSSValuePool::createColorValue(unsigned rgbValue)
 
 PassRef<CSSPrimitiveValue> CSSValuePool::createValue(double value, CSSPrimitiveValue::UnitTypes type)
 {
-    if (std::isinf(value))
-        return createIdentifierValue(CSSValueID::CSSValueInvalid);
+    ASSERT(std::isfinite(value));
 
     if (value < 0 || value > maximumCacheableIntegerValue)
         return CSSPrimitiveValue::create(value, type);