Bug 1195820 - Request constructor should throw TypeError if URL has credentials or parse fails. r=bkelly
authorEmilio Cobos Álvarez <ecoal95@gmail.com>
Mon, 24 Aug 2015 22:54:52 +0200
changeset 259392 ba8758c89322f23eb570432cfd72a9ae535e330b
parent 259391 dda56b1c5210e9e983d9a24c92b5a9986fdf60b1
child 259393 cbf83a6ab5e6c89f29f5a29b473fbac4c3f50124
push id29277
push userryanvm@gmail.com
push dateWed, 26 Aug 2015 18:32:23 +0000
treeherdermozilla-central@fea87cbeaa6b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1195820
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1195820 - Request constructor should throw TypeError if URL has credentials or parse fails. r=bkelly
dom/bindings/Errors.msg
dom/fetch/Request.cpp
dom/tests/mochitest/fetch/test_fetch_cors.js
dom/tests/mochitest/fetch/test_request.js
testing/web-platform/meta/service-workers/cache-storage/serviceworker/cache-match.https.html.ini
testing/web-platform/meta/service-workers/cache-storage/serviceworker/cache-put.https.html.ini
testing/web-platform/meta/service-workers/cache-storage/window/cache-match.https.html.ini
testing/web-platform/meta/service-workers/cache-storage/window/cache-put.https.html.ini
testing/web-platform/meta/service-workers/cache-storage/worker/cache-match.https.html.ini
testing/web-platform/meta/service-workers/cache-storage/worker/cache-put.https.html.ini
--- a/dom/bindings/Errors.msg
+++ b/dom/bindings/Errors.msg
@@ -46,16 +46,17 @@ MSG_DEF(MSG_DOM_DECODING_FAILED, 0, JSEX
 MSG_DEF(MSG_NOT_FINITE, 1, JSEXN_TYPEERR, "{0} is not a finite floating-point value.")
 MSG_DEF(MSG_INVALID_VERSION, 0, JSEXN_TYPEERR, "0 (Zero) is not a valid database version.")
 MSG_DEF(MSG_INVALID_BYTESTRING, 2, JSEXN_TYPEERR, "Cannot convert string to ByteString because the character"
         " at index {0} has value {1} which is greater than 255.")
 MSG_DEF(MSG_NOT_DATE, 1, JSEXN_TYPEERR, "{0} is not a date.")
 MSG_DEF(MSG_INVALID_ADVANCE_COUNT, 0, JSEXN_TYPEERR, "0 (Zero) is not a valid advance count.")
 MSG_DEF(MSG_DEFINEPROPERTY_ON_GSP, 0, JSEXN_TYPEERR, "Not allowed to define a property on the named properties object.")
 MSG_DEF(MSG_INVALID_URL, 1, JSEXN_TYPEERR, "{0} is not a valid URL.")
+MSG_DEF(MSG_URL_HAS_CREDENTIALS, 1, JSEXN_TYPEERR, "{0} is an url with embedded credentials.")
 MSG_DEF(MSG_METADATA_NOT_CONFIGURED, 0, JSEXN_TYPEERR, "Either size or lastModified should be true.")
 MSG_DEF(MSG_INVALID_READ_SIZE, 0, JSEXN_TYPEERR, "0 (Zero) is not a valid read size.")
 MSG_DEF(MSG_HEADERS_IMMUTABLE, 0, JSEXN_TYPEERR, "Headers are immutable and cannot be modified.")
 MSG_DEF(MSG_INVALID_HEADER_NAME, 1, JSEXN_TYPEERR, "{0} is an invalid header name.")
 MSG_DEF(MSG_INVALID_HEADER_VALUE, 1, JSEXN_TYPEERR, "{0} is an invalid header value.")
 MSG_DEF(MSG_INVALID_HEADER_SEQUENCE, 0, JSEXN_TYPEERR, "Headers require name/value tuples when being initialized by a sequence.")
 MSG_DEF(MSG_PERMISSION_DENIED_TO_PASS_ARG, 1, JSEXN_TYPEERR, "Permission denied to pass cross-origin object as {0}.")
 MSG_DEF(MSG_MISSING_REQUIRED_DICTIONARY_MEMBER, 1, JSEXN_TYPEERR, "Missing required {0}.")
--- a/dom/fetch/Request.cpp
+++ b/dom/fetch/Request.cpp
@@ -11,16 +11,17 @@
 
 #include "mozilla/ErrorResult.h"
 #include "mozilla/dom/Headers.h"
 #include "mozilla/dom/Fetch.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/URL.h"
 #include "mozilla/dom/WorkerPrivate.h"
 #include "mozilla/dom/workers/bindings/URL.h"
+#include "mozilla/unused.h"
 
 #include "WorkerPrivate.h"
 
 namespace mozilla {
 namespace dom {
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(Request)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(Request)
@@ -76,17 +77,27 @@ GetRequestURLFromDocument(nsIDocument* a
 {
   MOZ_ASSERT(aDocument);
   MOZ_ASSERT(NS_IsMainThread());
 
   nsCOMPtr<nsIURI> baseURI = aDocument->GetBaseURI();
   nsCOMPtr<nsIURI> resolvedURI;
   aRv = NS_NewURI(getter_AddRefs(resolvedURI), aInput, nullptr, baseURI);
   if (NS_WARN_IF(aRv.Failed())) {
-      return;
+    aRv.ThrowTypeError(MSG_INVALID_URL, &aInput);
+    return;
+  }
+
+  // This fails with URIs with weird protocols, even when they are valid,
+  // so we ignore the failure
+  nsAutoCString credentials;
+  unused << resolvedURI->GetUserPass(credentials);
+  if (!credentials.IsEmpty()) {
+    aRv.ThrowTypeError(MSG_URL_HAS_CREDENTIALS, &aInput);
+    return;
   }
 
   nsCOMPtr<nsIURI> resolvedURIClone;
   // We use CloneIgnoringRef to strip away the fragment even if the original URI
   // is immutable.
   aRv = resolvedURI->CloneIgnoringRef(getter_AddRefs(resolvedURIClone));
   if (NS_WARN_IF(aRv.Failed())) {
     return;
@@ -105,16 +116,26 @@ void
 GetRequestURLFromChrome(const nsAString& aInput, nsAString& aRequestURL,
                         ErrorResult& aRv)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   nsCOMPtr<nsIURI> uri;
   aRv = NS_NewURI(getter_AddRefs(uri), aInput, nullptr, nullptr);
   if (NS_WARN_IF(aRv.Failed())) {
+    aRv.ThrowTypeError(MSG_INVALID_URL, &aInput);
+    return;
+  }
+
+  // This fails with URIs with weird protocols, even when they are valid,
+  // so we ignore the failure
+  nsAutoCString credentials;
+  unused << uri->GetUserPass(credentials);
+  if (!credentials.IsEmpty()) {
+    aRv.ThrowTypeError(MSG_URL_HAS_CREDENTIALS, &aInput);
     return;
   }
 
   nsCOMPtr<nsIURI> uriClone;
   // We use CloneIgnoringRef to strip away the fragment even if the original URI
   // is immutable.
   aRv = uri->CloneIgnoringRef(getter_AddRefs(uriClone));
   if (NS_WARN_IF(aRv.Failed())) {
@@ -137,16 +158,34 @@ GetRequestURLFromWorker(const GlobalObje
   workers::WorkerPrivate* worker = workers::GetCurrentThreadWorkerPrivate();
   MOZ_ASSERT(worker);
   worker->AssertIsOnWorkerThread();
 
   NS_ConvertUTF8toUTF16 baseURL(worker->GetLocationInfo().mHref);
   nsRefPtr<workers::URL> url =
     workers::URL::Constructor(aGlobal, aInput, baseURL, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
+    aRv.ThrowTypeError(MSG_INVALID_URL, &aInput);
+    return;
+  }
+
+  nsString username;
+  url->GetUsername(username, aRv);
+  if (NS_WARN_IF(aRv.Failed())) {
+    return;
+  }
+
+  nsString password;
+  url->GetPassword(password, aRv);
+  if (NS_WARN_IF(aRv.Failed())) {
+    return;
+  }
+
+  if (!username.IsEmpty() || !password.IsEmpty()) {
+    aRv.ThrowTypeError(MSG_URL_HAS_CREDENTIALS, &aInput);
     return;
   }
 
   url->SetHash(EmptyString(), aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }
 
--- a/dom/tests/mochitest/fetch/test_fetch_cors.js
+++ b/dom/tests/mochitest/fetch/test_fetch_cors.js
@@ -734,20 +734,23 @@ function testModeCors() {
       req.url += "&preflightStatus=" + test.preflightStatus;
     if (test.responseHeaders)
       req.url += "&responseHeaders=" + escape(test.responseHeaders.toSource());
     if (test.exposeHeaders)
       req.url += "&exposeHeaders=" + escape(test.exposeHeaders);
     if (test.preflightBody)
       req.url += "&preflightBody=" + escape(test.preflightBody);
 
-    var request = new Request(req.url, { method: req.method, mode: "cors",
-                                         headers: req.headers, body: req.body });
-    fetches.push((function(test, request) {
-      return fetch(request).then(function(res) {
+    fetches.push((function(test) {
+      return new Promise(function(resolve) {
+        resolve(new Request(req.url, { method: req.method, mode: "cors",
+                                         headers: req.headers, body: req.body }));
+      }).then(function(request) {
+        return fetch(request);
+      }).then(function(res) {
         ok(test.pass, "Expected test to pass for " + test.toSource());
         if (test.status) {
           is(res.status, test.status, "wrong status in test for " + test.toSource());
           is(res.statusText, test.statusMessage, "wrong status text for " + test.toSource());
         }
         else {
           is(res.status, 200, "wrong status in test for " + test.toSource());
           is(res.statusText, "OK", "wrong status text for " + test.toSource());
@@ -762,31 +765,31 @@ function testModeCors() {
             else {
               is(res.headers.get(header), test.responseHeaders[header],
                  "|Headers.get()|wrong response header (" + header + ") in test for " +
                  test.toSource());
             }
           }
         }
 
-        return res.text().then(function(v) {
-          if (test.method !== "HEAD") {
-            is(v, "<res>hello pass</res>\n",
-               "wrong responseText in test for " + test.toSource());
-          }
-          else {
-            is(v, "",
-               "wrong responseText in HEAD test for " + test.toSource());
-          }
-        });
-      }, function(e) {
-          ok(!test.pass, "Expected test failure for " + test.toSource());
-          ok(e instanceof TypeError, "Exception should be TypeError for " + test.toSource());
+        return res.text();
+      }).then(function(v) {
+        if (test.method !== "HEAD") {
+          is(v, "<res>hello pass</res>\n",
+             "wrong responseText in test for " + test.toSource());
+        }
+        else {
+          is(v, "",
+             "wrong responseText in HEAD test for " + test.toSource());
+        }
+      }).catch(function(e) {
+        ok(!test.pass, "Expected test failure for " + test.toSource());
+        ok(e instanceof TypeError, "Exception should be TypeError for " + test.toSource());
       });
-    })(test, request));
+    })(test));
   }
 
   return Promise.all(fetches);
 }
 
 function testCrossOriginCredentials() {
   var tests = [
            { pass: 1,
--- a/dom/tests/mochitest/fetch/test_request.js
+++ b/dom/tests/mochitest/fetch/test_request.js
@@ -214,16 +214,41 @@ function testMethod() {
   var r = new Request("", { method: "patch", body: "hello" });
 }
 
 function testUrlFragment() {
   var req = new Request("./request#withfragment");
   is(req.url, (new URL("./request", self.location.href)).href, "request.url should be serialized with exclude fragment flag set");
 }
 
+function testUrlMalformed() {
+  try {
+    var req = new Request("http:// example.com");
+    ok(false, "Creating a Request with a malformed URL should throw a TypeError");
+  } catch(e) {
+    is(e.name, "TypeError", "Creating a Request with a malformed URL should throw a TypeError");
+  }
+}
+
+function testUrlCredentials() {
+  try {
+    var req = new Request("http://user@example.com");
+    ok(false, "URLs with credentials should be rejected");
+  } catch(e) {
+    is(e.name, "TypeError", "URLs with credentials should be rejected");
+  }
+
+  try {
+    var req = new Request("http://user:password@example.com");
+    ok(false, "URLs with credentials should be rejected");
+  } catch(e) {
+    is(e.name, "TypeError", "URLs with credentials should be rejected");
+  }
+}
+
 function testBodyUsed() {
   var req = new Request("./bodyused", { method: 'post', body: "Sample body" });
   is(req.bodyUsed, false, "bodyUsed is initially false.");
   return req.text().then((v) => {
     is(v, "Sample body", "Body should match");
     is(req.bodyUsed, true, "After reading body, bodyUsed should be true.");
   }).then((v) => {
     return req.blob().then((v) => {
@@ -466,16 +491,18 @@ function testRequestConsumedByFailedCons
   }
   ok(!r1.bodyUsed, 'Initial request should not be consumed by failed Request constructor');
 }
 
 function runTest() {
   testDefaultCtor();
   testSimpleUrlParse();
   testUrlFragment();
+  testUrlCredentials();
+  testUrlMalformed();
   testMethod();
   testBug1109574();
   testHeaderGuard();
   testModeCorsPreflightEnumValue();
   testBug1154268();
   testRequestConsumedByFailedConstructor();
 
   return Promise.resolve()
--- a/testing/web-platform/meta/service-workers/cache-storage/serviceworker/cache-match.https.html.ini
+++ b/testing/web-platform/meta/service-workers/cache-storage/serviceworker/cache-match.https.html.ini
@@ -1,3 +1,6 @@
 [cache-match.https.html]
   type: testharness
   prefs: [dom.serviceWorkers.enabled: true, dom.serviceWorkers.interception.enabled: true, dom.serviceWorkers.exemptFromPerDomainMax:true, dom.caches.enabled:true]
+  [Cache.match and Cache.matchAll]
+    expected: FAIL
+    bug: https://github.com/w3c/web-platform-tests/issues/2098
--- a/testing/web-platform/meta/service-workers/cache-storage/serviceworker/cache-put.https.html.ini
+++ b/testing/web-platform/meta/service-workers/cache-storage/serviceworker/cache-put.https.html.ini
@@ -1,3 +1,6 @@
 [cache-put.https.html]
   type: testharness
   prefs: [dom.serviceWorkers.enabled: true, dom.serviceWorkers.interception.enabled: true, dom.serviceWorkers.exemptFromPerDomainMax:true, dom.caches.enabled:true]
+  [Cache.put with request URLs containing embedded credentials]
+    expected: FAIL
+    bug: https://github.com/w3c/web-platform-tests/issues/2098
--- a/testing/web-platform/meta/service-workers/cache-storage/window/cache-match.https.html.ini
+++ b/testing/web-platform/meta/service-workers/cache-storage/window/cache-match.https.html.ini
@@ -1,3 +1,5 @@
 [cache-match.https.html]
   type: testharness
   prefs: [dom.caches.enabled:true]
+  expected: ERROR
+  bug: https://github.com/w3c/web-platform-tests/issues/2098
--- a/testing/web-platform/meta/service-workers/cache-storage/window/cache-put.https.html.ini
+++ b/testing/web-platform/meta/service-workers/cache-storage/window/cache-put.https.html.ini
@@ -1,3 +1,6 @@
 [cache-put.https.html]
   type: testharness
   prefs: [dom.caches.enabled:true]
+  [Cache.put with request URLs containing embedded credentials]
+    expected: FAIL
+    bug: https://github.com/w3c/web-platform-tests/issues/2098
--- a/testing/web-platform/meta/service-workers/cache-storage/worker/cache-match.https.html.ini
+++ b/testing/web-platform/meta/service-workers/cache-storage/worker/cache-match.https.html.ini
@@ -1,3 +1,5 @@
 [cache-match.https.html]
   type: testharness
   prefs: [dom.caches.enabled:true]
+  expected: ERROR
+  bug: https://github.com/w3c/web-platform-tests/issues/2098
--- a/testing/web-platform/meta/service-workers/cache-storage/worker/cache-put.https.html.ini
+++ b/testing/web-platform/meta/service-workers/cache-storage/worker/cache-put.https.html.ini
@@ -1,3 +1,6 @@
 [cache-put.https.html]
   type: testharness
   prefs: [dom.caches.enabled:true]
+  [Cache.put with request URLs containing embedded credentials]
+    expected: FAIL
+    bug: https://github.com/w3c/web-platform-tests/issues/2098