Bug 1355352 - Reject unicode-range descriptor when its value contains invalid part. r?jfkthame draft
authorXidorn Quan <me@upsuper.org>
Tue, 16 May 2017 10:15:18 +1000
changeset 578625 dbe5274bfc9a412cd3bbc2a19a19b6eb77c26b1b
parent 578624 7ce6813b457925386648d62ed0598b873daba54f
child 628775 c6b9bdfff1b6b8a1f59e222c0f708426ab2fbd3e
push id58989
push userxquan@mozilla.com
push dateTue, 16 May 2017 08:18:33 +0000
reviewersjfkthame
bugs1355352
milestone55.0a1
Bug 1355352 - Reject unicode-range descriptor when its value contains invalid part. r?jfkthame MozReview-Commit-ID: 1ZcFCPsIMtx
layout/style/nsCSSParser.cpp
layout/style/test/descriptor_database.js
layout/style/test/stylo-failures.md
layout/style/test/test_font_face_parser.html
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -15070,27 +15070,24 @@ CSSParserImpl::ParseFontRanges(nsCSSValu
     // An invalid range token is a parsing error, causing the entire
     // descriptor to be ignored.
     if (!mToken.mIntegerValid)
       return false;
 
     uint32_t low = mToken.mInteger;
     uint32_t high = mToken.mInteger2;
 
-    // A range that descends, or a range that is entirely outside the
-    // current range of Unicode (U+0-10FFFF) is ignored, but does not
-    // invalidate the descriptor.  A range that straddles the high end
-    // is clipped.
-    if (low <= 0x10FFFF && low <= high) {
-      if (high > 0x10FFFF)
-        high = 0x10FFFF;
-
-      ranges.AppendElement(low);
-      ranges.AppendElement(high);
-    }
+    // A range that descends, or high end exceeds the current range of
+    // Unicode (U+0-10FFFF) invalidates the descriptor.
+    if (low > high || high > 0x10FFFF) {
+      return false;
+    }
+    ranges.AppendElement(low);
+    ranges.AppendElement(high);
+
     if (!ExpectSymbol(',', true))
       break;
   }
 
   if (ranges.Length() == 0)
     return false;
 
   RefPtr<nsCSSValue::Array> srcVals
--- a/layout/style/test/descriptor_database.js
+++ b/layout/style/test/descriptor_database.js
@@ -56,17 +56,17 @@ var gCSSFontFaceDescriptors = {
 			"url(404.ttf) )",
 			"url(404.ttf) ) foo",
 			"url(404.ttf) ! important",
 			"url(404.ttf) ! hello",
 		]
 	},
 	"unicode-range": {
 		domProp: null,
-		values: [ "U+0-10FFFF", "U+3-7B3", "U+3??", "U+6A", "U+3????", "U+???", "U+302-302", "U+0-7,U+A-C", "U+100-17F,U+200-17F", "U+3??, U+500-513 ,U+612 , U+4????", "U+1FFF,U+200-27F" ],
-		invalid_values: [ "U+1????-2????", "U+0-7,A-C", "U+100-17F,200-17F", "U+6A!important", "U+6A)" ]
+		values: [ "U+0-10FFFF", "U+3-7B3", "U+3??", "U+6A", "U+3????", "U+???", "U+302-302", "U+0-7,U+A-C", "U+3??, U+500-513 ,U+612 , U+4????", "U+1FFF,U+200-27F" ],
+		invalid_values: [ "U+1????-2????", "U+0-7,A-C", "U+100-17F,U+200-17F", "U+100-17F,200-27F", "U+6A!important", "U+6A)" ]
 	},
 	"font-display": {
 		domProp: null,
 		values: [ "auto", "block", "swap", "fallback", "optional" ],
 		invalid_values: [ "normal", "initial" ]
 	}
 }
--- a/layout/style/test/stylo-failures.md
+++ b/layout/style/test/stylo-failures.md
@@ -209,22 +209,16 @@ to mochitest command.
 
 ## Assertions
 
 ## Need Gecko change
 
 * Servo is correct but Gecko is wrong
   * flex-basis should be 0px when omitted in flex shorthand bug 1331530
     * test_flexbox_flex_shorthand.html `flex-basis` [10]
-  * should reject whole value bug 1355352
-    * test_descriptor_storage.html `unicode-range` [1]
-    * test_font_face_parser.html `U+A5` [4]
-  * Gecko clamps rather than rejects invalid unicode range bug 1355356
-    * test_font_face_parser.html `U+??????` [2]
-    * ... `12FFFF` [2]
   * Gecko rejects calc() in -webkit-gradient bug 1363349
     * test_property_syntax_errors.html `-webkit-gradient` [20]
 * test_property_syntax_errors.html `linear-gradient(0,`: unitless zero as degree [10]
 
 ## Spec Unclear
 
 * test_property_syntax_errors.html `'background'`: whether background shorthand should accept "text" [200]
 
--- a/layout/style/test/test_font_face_parser.html
+++ b/layout/style/test/test_font_face_parser.html
@@ -177,22 +177,18 @@ function runTest() {
     { rule: _("unicode-range: u+00a5;"),
       d: { "unicode-range" : "U+A5" }, noncanonical: true },
     { rule: _("unicode-range: U+0-FF;"),
       d: { "unicode-range" : "U+0-FF" } },
     { rule: _("unicode-range: U+00??;"),
       d: { "unicode-range" : "U+0-FF" }, noncanonical: true },
     { rule: _("unicode-range: U+?"),
       d: { "unicode-range" : "U+0-F" }, noncanonical: true },
-    { rule: _("unicode-range: U+??????"),
-      d: { "unicode-range" : "U+0-10FFFF" }, noncanonical: true },
     { rule: _("unicode-range: U+590-5ff;"),
       d: { "unicode-range" : "U+590-5FF" }, noncanonical: true },
-    { rule: _("unicode-range: U+A0000-12FFFF"),
-      d: { "unicode-range" : "U+A0000-10FFFF" }, noncanonical: true },
 
     { rule: _("unicode-range: U+A5, U+4E00-9FFF, U+30??, U+FF00-FF9F;"),
       d: { "unicode-range" : "U+A5, U+4E00-9FFF, U+3000-30FF, U+FF00-FF9F" },
       noncanonical: true },
 
     { rule: _("unicode-range: U+104??;"),
       d: { "unicode-range" : "U+10400-104FF" }, noncanonical: true },
     { rule: _("unicode-range: U+320??, U+321??, U+322??, U+323??, U+324??, U+325??;"),
@@ -224,16 +220,18 @@ function runTest() {
     { rule: _("unicode-range: U+0121 U+1023"), d: {} },
     { rule: _("unicode-range: U+ 0121"), d: {} },
     { rule: _("unicode-range: U +0121"), d: {} },
     { rule: _("unicode-range: U+0121-"), d: {} },
     { rule: _("unicode-range: U+0121- 1023"), d: {} },
     { rule: _("unicode-range: U+0121 -1023"), d: {} },
     { rule: _("unicode-range: U+012 ?"), d: {} },
     { rule: _("unicode-range: U+01 2?"), d: {} },
+    { rule: _("unicode-range: U+A0000-12FFFF"), d: {} },
+    { rule: _("unicode-range: U+??????"), d: {} },
 
     // Thorough test of seven-digit rejection: all these are syntax errors
     { rule: _("unicode-range: U+1034560, U+A5"), d: {} },
     { rule: _("unicode-range: U+1034569, U+A5"), d: {} },
     { rule: _("unicode-range: U+103456a, U+A5"), d: {} },
     { rule: _("unicode-range: U+103456f, U+A5"), d: {} },
     { rule: _("unicode-range: U+103456?, U+A5"), d: {} },
     { rule: _("unicode-range: U+103456-1034560, U+A5"), d: {} },
@@ -242,22 +240,19 @@ function runTest() {
     { rule: _("unicode-range: U+103456-103456f, U+A5"), d: {} },
 
     // Syntactically invalid unicode-range tokens invalidate the
     // entire descriptor
     { rule: _("unicode-range: U+1, U+2, U+X"), d: {} },
     { rule: _("unicode-range: U+A5, U+0?F"), d: {} },
     { rule: _("unicode-range: U+A5, U+0F?-E00"), d: {} },
 
-    // Descending ranges and ranges outside 0-10FFFF are ignored
-    // but do not invalidate the descriptor
-    { rule: _("unicode-range: U+A5, U+90-30"),
-      d: { "unicode-range" : "U+A5" }, noncanonical: true },
-    { rule: _("unicode-range: U+A5, U+220043"),
-      d: { "unicode-range" : "U+A5" }, noncanonical: true },
+    // Descending ranges and ranges outside 0-10FFFF are invalid
+    { rule: _("unicode-range: U+A5, U+90-30"), d: {} },
+    { rule: _("unicode-range: U+A5, U+220043"), d: {} },
 
     // font-feature-settings
     { rule: _("font-feature-settings: normal;"),
       d: { "font-feature-settings" : "normal" } },
     { rule: _("font-feature-settings: \"dlig\";"),
       d: { "font-feature-settings" : "\"dlig\"" } },
     { rule: _("font-feature-settings: \"dlig\" 1;"),
       d: { "font-feature-settings" : "\"dlig\"" }, noncanonical: true },