Bug 1389251 - Do not escape spaces in nsSimpleURI r=bz draft
authorValentin Gosu <valentin.gosu@gmail.com>
Mon, 18 Jun 2018 15:06:24 +0200
changeset 808128 2da1b5f305ca2abcce2f9988cd6a5cbc12635c61
parent 808097 f804cc575bba9c6dcb4e3770f7dafe4d8934e73c
child 808129 7ff705a16984bca7bd318864ab2b0b3a89455805
push id113284
push uservalentin.gosu@gmail.com
push dateMon, 18 Jun 2018 13:48:27 +0000
reviewersbz
bugs1389251
milestone62.0a1
Bug 1389251 - Do not escape spaces in nsSimpleURI r=bz This change makes nsEscape::T_EscapeURL not escape spaces when passed esc_OnlyNonASCII. This fixes a web-compat issue for URLs such as "javascript: alert('hello');" and the fact that data: URIs with spaces around MIME type are rejected. MozReview-Commit-ID: 91Qw9foW6Y3
browser/components/sessionstore/test/browser_formdata.js
dom/url/tests/test_url.html
netwerk/test/unit/test_URIs.js
testing/web-platform/meta/fetch/data-urls/processing.any.js.ini
testing/web-platform/meta/url/url-constructor.html.ini
xpcom/io/nsEscape.cpp
--- a/browser/components/sessionstore/test/browser_formdata.js
+++ b/browser/components/sessionstore/test/browser_formdata.js
@@ -57,17 +57,17 @@ add_task(async function test_formdata() 
 });
 
 /**
  * This test ensures that a malicious website can't trick us into restoring
  * form data into a wrong website and that we always check the stored URL
  * before doing so.
  */
 add_task(async function test_url_check() {
-  const URL = "data:text/html;charset=utf-8,<input%20id=input>";
+  const URL = "data:text/html;charset=utf-8,<input id=input>";
   const VALUE = "value-" + Math.random();
 
   // Create a tab with an iframe containing an input field.
   let tab = BrowserTestUtils.addTab(gBrowser, URL);
   let browser = tab.linkedBrowser;
   await promiseBrowserLoaded(browser);
 
   // Restore a tab state with a given form data url.
@@ -99,17 +99,17 @@ add_task(async function test_url_check()
 add_task(async function test_nested() {
   const URL = "data:text/html;charset=utf-8," +
               "<iframe src='data:text/html;charset=utf-8," +
               "<input autofocus=true>'/>";
 
   const FORM_DATA = {
     children: [{
       xpath: {"/xhtml:html/xhtml:body/xhtml:input": "m"},
-      url: "data:text/html;charset=utf-8,<input%20autofocus=true>"
+      url: "data:text/html;charset=utf-8,<input autofocus=true>"
     }]
   };
 
   // Create a tab with an iframe containing an input field.
   let tab = gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser, URL);
   let browser = tab.linkedBrowser;
   await promiseBrowserLoaded(browser);
 
--- a/dom/url/tests/test_url.html
+++ b/dom/url/tests/test_url.html
@@ -474,17 +474,17 @@
     url.hash = "newhash";
     is(url.href, "scheme:path?query#newhash");
     url.search = "";
     is(url.href, "scheme:path#newhash");
 
     // we don't implement a spec-compliant parser yet.
     // make sure we are bug compatible with existing implementations.
     url = new URL("data:text/html,<a href=\"http://example.org/?q\">Link</a>");
-    is(url.href, "data:text/html,<a%20href=\"http://example.org/?q\">Link</a>");
+    is(url.href, "data:text/html,<a href=\"http://example.org/?q\">Link</a>");
   </script>
 
   <script>
     var u = new URL('http://www.example.org');
     ok(u.toJSON(), 'http://www.example.org', "URL.toJSON()");
     is(JSON.stringify(u), "\"http://www.example.org/\"", "JSON.stringify(u) works");
   </script>
 </body>
--- a/netwerk/test/unit/test_URIs.js
+++ b/netwerk/test/unit/test_URIs.js
@@ -48,20 +48,26 @@ var gTests = [
     ref:     "",
     nsIURL:  false, nsINestedURI: false },
   { spec:    "data:text/html;charset=utf-8,<html>\r\n\t</html>",
     scheme:  "data",
     prePath: "data:",
     pathQueryRef: "text/html;charset=utf-8,<html></html>",
     ref:     "",
     nsIURL:  false, nsINestedURI: false },
+  { spec:    "data:text/plain,hello%20world",
+    scheme:  "data",
+    prePath: "data:",
+    pathQueryRef: "text/plain,hello%20world",
+    ref:     "",
+    nsIURL:  false, nsINestedURI: false },
   { spec:    "data:text/plain,hello world",
     scheme:  "data",
     prePath: "data:",
-    pathQueryRef: "text/plain,hello%20world",
+    pathQueryRef: "text/plain,hello world",
     ref:     "",
     nsIURL:  false, nsINestedURI: false },
   { spec:    "file:///dir/afile",
     scheme:  "data",
     prePath: "data:",
     pathQueryRef: "text/plain,2",
     ref:     "",
     relativeURI: "data:te\nxt/plain,2",
@@ -189,17 +195,17 @@ var gTests = [
     scheme:  "mailto",
     prePath: "mailto:",
     pathQueryRef: "webmaster@mozilla.com",
     ref:     "",
     nsIURL:  false, nsINestedURI: false },
   { spec:    "javascript:new Date()",
     scheme:  "javascript",
     prePath: "javascript:",
-    pathQueryRef: "new%20Date()",
+    pathQueryRef: "new Date()",
     ref:     "",
     nsIURL:  false, nsINestedURI: false },
   { spec:    "blob:123456",
     scheme:  "blob",
     prePath: "blob:",
     pathQueryRef: "123456",
     ref:     "",
     nsIURL:  false, nsINestedURI: false, immutable: true },
--- a/testing/web-platform/meta/fetch/data-urls/processing.any.js.ini
+++ b/testing/web-platform/meta/fetch/data-urls/processing.any.js.ini
@@ -1,18 +1,15 @@
 [processing.any.html]
   ["data://test/,X"]
     expected: FAIL
 
   ["data://test:test/,X"]
     expected: FAIL
 
-  ["data:text/plain ,X"]
-    expected: FAIL
-
   ["data:;x=x;charset=x,X"]
     expected: FAIL
 
   ["data:;x=x,X"]
     expected: FAIL
 
   ["data:IMAGE/gif;hi=x,%C2%B1"]
     expected: FAIL
@@ -24,19 +21,16 @@
     expected: FAIL
 
   ["data:\\0,%FF"]
     expected: FAIL
 
   ["data:%00,%FF"]
     expected: FAIL
 
-  ["data:text/html  ,X"]
-    expected: FAIL
-
   ["data:text / html,X"]
     expected: FAIL
 
   ["data:†,X"]
     expected: FAIL
 
   ["data:X,X"]
     expected: FAIL
@@ -110,19 +104,16 @@
 
 [processing.any.worker.html]
   ["data://test/,X"]
     expected: FAIL
 
   ["data://test:test/,X"]
     expected: FAIL
 
-  ["data:text/plain ,X"]
-    expected: FAIL
-
   ["data:;x=x;charset=x,X"]
     expected: FAIL
 
   ["data:;x=x,X"]
     expected: FAIL
 
   ["data:IMAGE/gif;hi=x,%C2%B1"]
     expected: FAIL
@@ -134,19 +125,16 @@
     expected: FAIL
 
   ["data:\\0,%FF"]
     expected: FAIL
 
   ["data:%00,%FF"]
     expected: FAIL
 
-  ["data:text/html  ,X"]
-    expected: FAIL
-
   ["data:text / html,X"]
     expected: FAIL
 
   ["data:†,X"]
     expected: FAIL
 
   ["data:X,X"]
     expected: FAIL
--- a/testing/web-platform/meta/url/url-constructor.html.ini
+++ b/testing/web-platform/meta/url/url-constructor.html.ini
@@ -1,12 +1,9 @@
 [url-constructor.html]
-  [Parsing: <a:\t foo.com> against <http://example.org/foo/bar>]
-    expected: FAIL
-
   [Parsing: <foo://> against <http://example.org/foo/bar>]
     expected: FAIL
 
   [Parsing: <http::@c:29> against <http://example.org/foo/bar>]
     expected: FAIL
 
   [Parsing: <http://::@c@d:2> against <http://example.org/foo/bar>]
     expected: FAIL
@@ -210,19 +207,16 @@
     expected: FAIL
 
   [Parsing: <http://256.256.256.256> against <http://other.com/>]
     expected: FAIL
 
   [Parsing: <..> against <file:///C:/>]
     expected: FAIL
 
-  [Parsing: <lolscheme:x x#x x> against <about:blank>]
-    expected: FAIL
-
   [Parsing: <file://example:1/> against <about:blank>]
     expected: FAIL
 
   [Parsing: <file://example:test/> against <about:blank>]
     expected: FAIL
 
   [Parsing: <file://example%/> against <about:blank>]
     expected: FAIL
--- a/xpcom/io/nsEscape.cpp
+++ b/xpcom/io/nsEscape.cpp
@@ -351,21 +351,20 @@ T_EscapeURL(const typename T::char_type*
     // And, we will not escape non-ascii characters if requested.
     // On special request we will also escape the colon even when
     // not covered by the matrix.
     // ignoreAscii is not honored for control characters (C0 and DEL)
     //
     // And, we should escape the '|' character when it occurs after any
     // non-ASCII character as it may be aPart of a multi-byte character.
     //
-    // 0x20..0x7e are the valid ASCII characters. We also escape spaces
-    // (0x20) since they are not legal in URLs.
+    // 0x20..0x7e are the valid ASCII characters.
     if ((dontNeedEscape(c, aFlags) || (c == HEX_ESCAPE && !forced)
          || (c > 0x7f && ignoreNonAscii)
-         || (c > 0x20 && c < 0x7f && ignoreAscii))
+         || (c >= 0x20 && c < 0x7f && ignoreAscii))
         && !(c == ':' && colon)
         && !(previousIsNonASCII && c == '|' && !ignoreNonAscii)) {
       if (writing) {
         tempBuffer[tempBufferPos++] = c;
       }
     } else { /* do the escape magic */
       if (!writing) {
         if (!aResult.Append(aPart, i, mozilla::fallible)) {