CSS canvas color parsing accepts invalid color identifiers
authorjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Jul 2014 21:56:05 +0000 (21:56 +0000)
committerjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Jul 2014 21:56:05 +0000 (21:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=134661

Reviewed by Benjamin Poulain.

Source/WebCore:
Current implementation of the CSSParser::parseSystemColor assumes
that if a valid cssValueKeywordID is got then it has to be a valid
color. Such assumption is wrong and lead to many bugs and layout
test failures.

The parseSystemFunction determines now whether the parsed color is
valid or not.

Addtionally, a new method has been added to share the logic of
determining whether a CSSValueID is a valid primitive values for
colors or not. Generally, we should avoid passing invalid color
identifiers to the theming API.

No new tests, but added additional cases to the
canvas-color-serialization.html, test-setting-canvas-color and
rgb-color-parse test.

* css/CSSParser.cpp:
(WebCore::validPrimitiveValueColor): Added.
(WebCore::parseColorValue):
(WebCore::CSSParser::parseSystemColor):

LayoutTests:
The parseSystemFunction determines now whether the parsed color is
valid or not.

The rgb-color-parser covers css style color parsing cases, which
already provide coverage for invalid color identifiers. I've added
a few more, though.

* fast/canvas/canvas-color-serialization-expected.txt:
* fast/canvas/script-tests/canvas-color-serialization.js:
* fast/css/test-setting-canvas-color-expected.txt:
* fast/css/test-setting-canvas-color.html:
* svg/dom/rgb-color-parser-expected.txt:
* svg/dom/rgb-color-parser.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/canvas/canvas-color-serialization-expected.txt
LayoutTests/fast/canvas/script-tests/canvas-color-serialization.js
LayoutTests/fast/css/test-setting-canvas-color-expected.txt
LayoutTests/fast/css/test-setting-canvas-color.html
LayoutTests/svg/dom/rgb-color-parser-expected.txt
LayoutTests/svg/dom/rgb-color-parser.html
Source/WebCore/ChangeLog
Source/WebCore/css/CSSParser.cpp

index c5bb0b7..f45780b 100644 (file)
@@ -1,3 +1,24 @@
+2014-07-09  Javier Fernandez  <jfernandez@igalia.com>
+
+        CSS canvas color parsing accepts invalid color identifiers
+        https://bugs.webkit.org/show_bug.cgi?id=134661
+
+        Reviewed by Benjamin Poulain.
+
+        The parseSystemFunction determines now whether the parsed color is
+        valid or not.
+
+        The rgb-color-parser covers css style color parsing cases, which
+        already provide coverage for invalid color identifiers. I've added
+        a few more, though.
+
+        * fast/canvas/canvas-color-serialization-expected.txt:
+        * fast/canvas/script-tests/canvas-color-serialization.js:
+        * fast/css/test-setting-canvas-color-expected.txt:
+        * fast/css/test-setting-canvas-color.html:
+        * svg/dom/rgb-color-parser-expected.txt:
+        * svg/dom/rgb-color-parser.html:
+
 2014-06-28  Jer Noble  <jer.noble@apple.com>
 
         [MSE] http/tests/media/media-source/mediasource-remove.html is failing.
index cdc241f..b8f3ebd 100644 (file)
@@ -129,6 +129,30 @@ PASS trySettingShadowColor(Infinity) is '#666666'
 PASS trySettingStrokeStyle(null) is '#666666'
 PASS trySettingFillStyle(null) is '#666666'
 PASS trySettingShadowColor(null) is '#666666'
+PASS trySettingStrokeStyle('left') is '#666666'
+PASS trySettingFillStyle('left') is '#666666'
+PASS trySettingShadowColor('left') is '#666666'
+PASS trySettingStrokeStyle('right') is '#666666'
+PASS trySettingFillStyle('right') is '#666666'
+PASS trySettingShadowColor('right') is '#666666'
+PASS trySettingStrokeStyle('center') is '#666666'
+PASS trySettingFillStyle('center') is '#666666'
+PASS trySettingShadowColor('center') is '#666666'
+PASS trySettingStrokeStyle('border') is '#666666'
+PASS trySettingFillStyle('border') is '#666666'
+PASS trySettingShadowColor('border') is '#666666'
+PASS trySettingStrokeStyle('border-box') is '#666666'
+PASS trySettingFillStyle('border-box') is '#666666'
+PASS trySettingShadowColor('border-box') is '#666666'
+PASS trySettingStrokeStyle('content') is '#666666'
+PASS trySettingFillStyle('content') is '#666666'
+PASS trySettingShadowColor('content') is '#666666'
+PASS trySettingStrokeStyle('logical') is '#666666'
+PASS trySettingFillStyle('logical') is '#666666'
+PASS trySettingShadowColor('logical') is '#666666'
+PASS trySettingStrokeStyle('visual') is '#666666'
+PASS trySettingFillStyle('visual') is '#666666'
+PASS trySettingShadowColor('visual') is '#666666'
 PASS trySettingStrokeColorWithSetter('transparent') is 'rgba(0, 0, 0, 0)'
 PASS trySettingFillColorWithSetter('transparent') is 'rgba(0, 0, 0, 0)'
 PASS trySettingShadowWithSetter('transparent') is 'rgba(0, 0, 0, 0)'
index d308992..75cebb4 100644 (file)
@@ -230,6 +230,14 @@ trySettingColor("-1", "'#666666'");
 trySettingColor("NaN", "'#666666'");
 trySettingColor("Infinity", "'#666666'");
 trySettingColor("null", "'#666666'");
+trySettingColor("'left'", "'#666666'");
+trySettingColor("'right'", "'#666666'");
+trySettingColor("'center'", "'#666666'");
+trySettingColor("'border'", "'#666666'");
+trySettingColor("'border-box'", "'#666666'");
+trySettingColor("'content'", "'#666666'");
+trySettingColor("'logical'", "'#666666'");
+trySettingColor("'visual'", "'#666666'");
 
 trySettingColorWithSetter("'transparent'", "'rgba(0, 0, 0, 0)'");
 trySettingColorWithSetter("'red'", "'#ff0000'");
index c29cf2e..434ec8c 100644 (file)
@@ -214,6 +214,14 @@ PASS Setting color to inherited was not set (as expected).
 PASS Setting color to #100% was not set (as expected).
 PASS Setting color to #100px was not set (as expected).
 PASS Setting color to -webkit-var("test") was not set (as expected).
+PASS Setting color to left was not set (as expected).
+PASS Setting color to right was not set (as expected).
+PASS Setting color to center was not set (as expected).
+PASS Setting color to border was not set (as expected).
+PASS Setting color to border was not set (as expected).
+PASS Setting color to content was not set (as expected).
+PASS Setting color to logical was not set (as expected).
+PASS Setting color to visual was not set (as expected).
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 9288ee2..bfc684c 100644 (file)
@@ -242,6 +242,14 @@ shouldNotSuccessfullyParse("inherited");
 shouldNotSuccessfullyParse("#100%");
 shouldNotSuccessfullyParse("#100px");
 shouldNotSuccessfullyParse('-webkit-var("test")');
+shouldNotSuccessfullyParse('left');
+shouldNotSuccessfullyParse('right');
+shouldNotSuccessfullyParse('center');
+shouldNotSuccessfullyParse('border');
+shouldNotSuccessfullyParse('border');
+shouldNotSuccessfullyParse('content');
+shouldNotSuccessfullyParse('logical');
+shouldNotSuccessfullyParse('visual');
 </script>
 <script src="../../resources/js-test-post.js"></script>
 </html>
index 094e11b..16ea03d 100644 (file)
@@ -5,6 +5,152 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 Parsed as rgb(0,0,255): blue
 Parsed as rgb(0,255,0): rgb(0, 255, 0)
+Parsed as rgb(240,248,255): aliceblue
+Parsed as rgb(250,235,215): antiquewhite
+Parsed as rgb(0,255,255): aqua
+Parsed as rgb(127,255,212): aquamarine
+Parsed as rgb(240,255,255): azure
+Parsed as rgb(245,245,220): beige
+Parsed as rgb(255,228,196): bisque
+Parsed as rgb(0,0,0): black
+Parsed as rgb(255,235,205): blanchedalmond
+Parsed as rgb(0,0,255): blue
+Parsed as rgb(138,43,226): blueviolet
+Parsed as rgb(165,42,42): brown
+Parsed as rgb(222,184,135): burlywood
+Parsed as rgb(95,158,160): cadetblue
+Parsed as rgb(127,255,0): chartreuse
+Parsed as rgb(210,105,30): chocolate
+Parsed as rgb(255,127,80): coral
+Parsed as rgb(100,149,237): cornflowerblue
+Parsed as rgb(255,248,220): cornsilk
+Parsed as rgb(220,20,60): crimson
+Parsed as rgb(0,255,255): cyan
+Parsed as rgb(0,0,139): darkblue
+Parsed as rgb(0,139,139): darkcyan
+Parsed as rgb(184,134,11): darkgoldenrod
+Parsed as rgb(169,169,169): darkgray
+Parsed as rgb(0,100,0): darkgreen
+Parsed as rgb(169,169,169): darkgrey
+Parsed as rgb(189,183,107): darkkhaki
+Parsed as rgb(139,0,139): darkmagenta
+Parsed as rgb(85,107,47): darkolivegreen
+Parsed as rgb(255,140,0): darkorange
+Parsed as rgb(153,50,204): darkorchid
+Parsed as rgb(139,0,0): darkred
+Parsed as rgb(233,150,122): darksalmon
+Parsed as rgb(143,188,143): darkseagreen
+Parsed as rgb(72,61,139): darkslateblue
+Parsed as rgb(47,79,79): darkslategray
+Parsed as rgb(47,79,79): darkslategrey
+Parsed as rgb(0,206,209): darkturquoise
+Parsed as rgb(148,0,211): darkviolet
+Parsed as rgb(255,20,147): deeppink
+Parsed as rgb(0,191,255): deepskyblue
+Parsed as rgb(105,105,105): dimgray
+Parsed as rgb(105,105,105): dimgrey
+Parsed as rgb(30,144,255): dodgerblue
+Parsed as rgb(178,34,34): firebrick
+Parsed as rgb(255,250,240): floralwhite
+Parsed as rgb(34,139,34): forestgreen
+Parsed as rgb(255,0,255): fuchsia
+Parsed as rgb(220,220,220): gainsboro
+Parsed as rgb(248,248,255): ghostwhite
+Parsed as rgb(255,215,0): gold
+Parsed as rgb(218,165,32): goldenrod
+Parsed as rgb(128,128,128): gray
+Parsed as rgb(0,128,0): green
+Parsed as rgb(173,255,47): greenyellow
+Parsed as rgb(128,128,128): grey
+Parsed as rgb(240,255,240): honeydew
+Parsed as rgb(255,105,180): hotpink
+Parsed as rgb(205,92,92): indianred
+Parsed as rgb(75,0,130): indigo
+Parsed as rgb(255,255,240): ivory
+Parsed as rgb(240,230,140): khaki
+Parsed as rgb(230,230,250): lavender
+Parsed as rgb(255,240,245): lavenderblush
+Parsed as rgb(124,252,0): lawngreen
+Parsed as rgb(255,250,205): lemonchiffon
+Parsed as rgb(173,216,230): lightblue
+Parsed as rgb(240,128,128): lightcoral
+Parsed as rgb(224,255,255): lightcyan
+Parsed as rgb(250,250,210): lightgoldenrodyellow
+Parsed as rgb(211,211,211): lightgray
+Parsed as rgb(144,238,144): lightgreen
+Parsed as rgb(211,211,211): lightgrey
+Parsed as rgb(255,182,193): lightpink
+Parsed as rgb(255,160,122): lightsalmon
+Parsed as rgb(32,178,170): lightseagreen
+Parsed as rgb(135,206,250): lightskyblue
+Parsed as rgb(119,136,153): lightslategray
+Parsed as rgb(119,136,153): lightslategrey
+Parsed as rgb(176,196,222): lightsteelblue
+Parsed as rgb(255,255,224): lightyellow
+Parsed as rgb(0,255,0): lime
+Parsed as rgb(50,205,50): limegreen
+Parsed as rgb(250,240,230): linen
+Parsed as rgb(255,0,255): magenta
+Parsed as rgb(128,0,0): maroon
+Parsed as rgb(102,205,170): mediumaquamarine
+Parsed as rgb(0,0,205): mediumblue
+Parsed as rgb(186,85,211): mediumorchid
+Parsed as rgb(147,112,219): mediumpurple
+Parsed as rgb(60,179,113): mediumseagreen
+Parsed as rgb(123,104,238): mediumslateblue
+Parsed as rgb(0,250,154): mediumspringgreen
+Parsed as rgb(72,209,204): mediumturquoise
+Parsed as rgb(199,21,133): mediumvioletred
+Parsed as rgb(25,25,112): midnightblue
+Parsed as rgb(245,255,250): mintcream
+Parsed as rgb(255,228,225): mistyrose
+Parsed as rgb(255,228,181): moccasin
+Parsed as rgb(255,222,173): navajowhite
+Parsed as rgb(0,0,128): navy
+Parsed as rgb(253,245,230): oldlace
+Parsed as rgb(128,128,0): olive
+Parsed as rgb(107,142,35): olivedrab
+Parsed as rgb(255,165,0): orange
+Parsed as rgb(255,69,0): orangered
+Parsed as rgb(218,112,214): orchid
+Parsed as rgb(238,232,170): palegoldenrod
+Parsed as rgb(152,251,152): palegreen
+Parsed as rgb(175,238,238): paleturquoise
+Parsed as rgb(219,112,147): palevioletred
+Parsed as rgb(255,239,213): papayawhip
+Parsed as rgb(255,218,185): peachpuff
+Parsed as rgb(205,133,63): peru
+Parsed as rgb(255,192,203): pink
+Parsed as rgb(221,160,221): plum
+Parsed as rgb(176,224,230): powderblue
+Parsed as rgb(128,0,128): purple
+Parsed as rgb(188,143,143): rosybrown
+Parsed as rgb(65,105,225): royalblue
+Parsed as rgb(139,69,19): saddlebrown
+Parsed as rgb(250,128,114): salmon
+Parsed as rgb(244,164,96): sandybrown
+Parsed as rgb(46,139,87): seagreen
+Parsed as rgb(255,245,238): seashell
+Parsed as rgb(160,82,45): sienna
+Parsed as rgb(192,192,192): silver
+Parsed as rgb(135,206,235): skyblue
+Parsed as rgb(106,90,205): slateblue
+Parsed as rgb(112,128,144): slategray
+Parsed as rgb(112,128,144): slategrey
+Parsed as rgb(255,250,250): snow
+Parsed as rgb(0,255,127): springgreen
+Parsed as rgb(70,130,180): steelblue
+Parsed as rgb(210,180,140): tan
+Parsed as rgb(0,128,128): teal
+Parsed as rgb(216,191,216): thistle
+Parsed as rgb(255,99,71): tomato
+Parsed as rgb(64,224,208): turquoise
+Parsed as rgb(238,130,238): violet
+Parsed as rgb(245,222,179): wheat
+Parsed as rgb(255,255,255): white
+Parsed as rgb(245,245,245): whitesmoke
+Parsed as rgb(255,255,0): yellow
+Parsed as rgb(154,205,50): yellowgreen
 Failed to parse: rgb(100%,100%,0%
 Failed to parse: rgba(100%,100%,0%
 Failed to parse: rgb(100%,100%,r)
@@ -21,6 +167,14 @@ Failed to parse: #ffff
 Failed to parse: #fffff
 Failed to parse: #fffffff
 Failed to parse: green,
+Failed to parse: 'left'
+Failed to parse: 'right'
+Failed to parse: 'center'
+Failed to parse: 'border'
+Failed to parse: 'border-
+Failed to parse: 'content'
+Failed to parse: 'logical'
+Failed to parse: 'visual'
 Parsed as rgb(0,10,20): rgb(0, 10, 20)
 Parsed as rgb(255,255,255): #fff
 Parsed as rgb(255,255,255): #ffffff
index 5ea9687..ea13733 100644 (file)
         }
     }
 
+    // Taken from CSS 3 color.
+    var svgColors = [
+        "aliceblue",
+        "antiquewhite",
+        "aqua",
+        "aquamarine",
+        "azure",
+        "beige",
+        "bisque",
+        "black",
+        "blanchedalmond",
+        "blue",
+        "blueviolet",
+        "brown",
+        "burlywood",
+        "cadetblue",
+        "chartreuse",
+        "chocolate",
+        "coral",
+        "cornflowerblue",
+        "cornsilk",
+        "crimson",
+        "cyan",
+        "darkblue",
+        "darkcyan",
+        "darkgoldenrod",
+        "darkgray",
+        "darkgreen",
+        "darkgrey",
+        "darkkhaki",
+        "darkmagenta",
+        "darkolivegreen",
+        "darkorange",
+        "darkorchid",
+        "darkred",
+        "darksalmon",
+        "darkseagreen",
+        "darkslateblue",
+        "darkslategray",
+        "darkslategrey",
+        "darkturquoise",
+        "darkviolet",
+        "deeppink",
+        "deepskyblue",
+        "dimgray",
+        "dimgrey",
+        "dodgerblue",
+        "firebrick",
+        "floralwhite",
+        "forestgreen",
+        "fuchsia",
+        "gainsboro",
+        "ghostwhite",
+        "gold",
+        "goldenrod",
+        "gray",
+        "green",
+        "greenyellow",
+        "grey",
+        "honeydew",
+        "hotpink",
+        "indianred",
+        "indigo",
+        "ivory",
+        "khaki",
+        "lavender",
+        "lavenderblush",
+        "lawngreen",
+        "lemonchiffon",
+        "lightblue",
+        "lightcoral",
+        "lightcyan",
+        "lightgoldenrodyellow",
+        "lightgray",
+        "lightgreen",
+        "lightgrey",
+        "lightpink",
+        "lightsalmon",
+        "lightseagreen",
+        "lightskyblue",
+        "lightslategray",
+        "lightslategrey",
+        "lightsteelblue",
+        "lightyellow",
+        "lime",
+        "limegreen",
+        "linen",
+        "magenta",
+        "maroon",
+        "mediumaquamarine",
+        "mediumblue",
+        "mediumorchid",
+        "mediumpurple",
+        "mediumseagreen",
+        "mediumslateblue",
+        "mediumspringgreen",
+        "mediumturquoise",
+        "mediumvioletred",
+        "midnightblue",
+        "mintcream",
+        "mistyrose",
+        "moccasin",
+        "navajowhite",
+        "navy",
+        "oldlace",
+        "olive",
+        "olivedrab",
+        "orange",
+        "orangered",
+        "orchid",
+        "palegoldenrod",
+        "palegreen",
+        "paleturquoise",
+        "palevioletred",
+        "papayawhip",
+        "peachpuff",
+        "peru",
+        "pink",
+        "plum",
+        "powderblue",
+        "purple",
+        // We do not test red.
+        "rosybrown",
+        "royalblue",
+        "saddlebrown",
+        "salmon",
+        "sandybrown",
+        "seagreen",
+        "seashell",
+        "sienna",
+        "silver",
+        "skyblue",
+        "slateblue",
+        "slategray",
+        "slategrey",
+        "snow",
+        "springgreen",
+        "steelblue",
+        "tan",
+        "teal",
+        "thistle",
+        "tomato",
+        "turquoise",
+        "violet",
+        "wheat",
+        "white",
+        "whitesmoke",
+        "yellow",
+        "yellowgreen"
+    ];
+
     function fuzz()
     {
         // Some valid values.
         parseRGBColor("blue");
         parseRGBColor("rgb(0, 255, 0)");
-        
+        for (var i = 0; i < svgColors.length; ++i)
+            parseRGBColor(svgColors[i]);
+
         // Some invalid ones.
         parseRGBColor("rgb(100%,100%,0%");
         parseRGBColor("rgba(100%,100%,0%");
         parseRGBColor("#fffff");
         parseRGBColor("#fffffff");
         parseRGBColor("green,");
+        parseRGBColor("'left'");
+        parseRGBColor("'right'");
+        parseRGBColor("'center'");
+        parseRGBColor("'border'");
+        parseRGBColor("'border-");
+        parseRGBColor("'content'");
+        parseRGBColor("'logical'");
+        parseRGBColor("'visual'");
 
         // Some more valid ones.
         parseRGBColor("rgb(0, 10, 20)");
index c647443..291902c 100644 (file)
@@ -1,3 +1,31 @@
+2014-07-09  Javier Fernandez  <jfernandez@igalia.com>
+        CSS canvas color parsing accepts invalid color identifiers
+        https://bugs.webkit.org/show_bug.cgi?id=134661
+
+        Reviewed by Benjamin Poulain.
+
+        Current implementation of the CSSParser::parseSystemColor assumes
+        that if a valid cssValueKeywordID is got then it has to be a valid
+        color. Such assumption is wrong and lead to many bugs and layout
+        test failures.
+
+        The parseSystemFunction determines now whether the parsed color is
+        valid or not.
+
+        Addtionally, a new method has been added to share the logic of
+        determining whether a CSSValueID is a valid primitive values for
+        colors or not. Generally, we should avoid passing invalid color
+        identifiers to the theming API.
+
+        No new tests, but added additional cases to the
+        canvas-color-serialization.html, test-setting-canvas-color and
+        rgb-color-parse test.
+
+        * css/CSSParser.cpp:
+        (WebCore::validPrimitiveValueColor): Added.
+        (WebCore::parseColorValue):
+        (WebCore::CSSParser::parseSystemColor):
+
 2014-06-28  Jer Noble  <jer.noble@apple.com>
 
         [MSE] http/tests/media/media-source/mediasource-remove.html is failing
index 6835d66..3705ede 100644 (file)
@@ -501,6 +501,13 @@ static inline bool isColorPropertyID(CSSPropertyID propertyId)
     }
 }
 
+static bool validPrimitiveValueColor(CSSValueID valueID, bool strict = false)
+{
+    return (valueID == CSSValueWebkitText || valueID == CSSValueCurrentcolor || valueID == CSSValueMenu
+        || (valueID >= CSSValueAlpha && valueID <= CSSValueWindowtext)
+        || (valueID >= CSSValueWebkitFocusRingColor && valueID < CSSValueWebkitText && !strict));
+}
+
 static bool parseColorValue(MutableStyleProperties* declaration, CSSPropertyID propertyId, const String& string, bool important, CSSParserMode cssParserMode)
 {
     ASSERT(!string.isEmpty());
@@ -510,17 +517,7 @@ static bool parseColorValue(MutableStyleProperties* declaration, CSSPropertyID p
     CSSParserString cssString;
     cssString.init(string);
     CSSValueID valueID = cssValueKeywordID(cssString);
-    bool validPrimitive = false;
-    if (valueID == CSSValueWebkitText)
-        validPrimitive = true;
-    else if (valueID == CSSValueCurrentcolor)
-        validPrimitive = true;
-    else if ((valueID >= CSSValueAqua && valueID <= CSSValueWindowtext) || valueID == CSSValueMenu
-             || (valueID >= CSSValueWebkitFocusRingColor && valueID < CSSValueWebkitText && !strict)) {
-        validPrimitive = true;
-    }
-
-    if (validPrimitive) {
+    if (validPrimitiveValueColor(valueID, strict)) {
         RefPtr<CSSValue> value = cssValuePool().createIdentifierValue(valueID);
         declaration->addParsedProperty(CSSProperty(propertyId, value.release(), important));
         return true;
@@ -1355,10 +1352,14 @@ bool CSSParser::parseSystemColor(RGBA32& color, const String& string, Document*
     CSSParserString cssColor;
     cssColor.init(string);
     CSSValueID id = cssValueKeywordID(cssColor);
-    if (id <= 0)
+    if (!validPrimitiveValueColor(id))
+        return false;
+
+    Color parsedColor = document->page()->theme().systemColor(id);
+    if (!parsedColor.isValid())
         return false;
 
-    color = document->page()->theme().systemColor(id).rgb();
+    color = parsedColor.rgb();
     return true;
 }