Bug 1275746 - Don't allow empty host name for URLTYPE_AUTHORITY URLs r=mcmanus
☠☠ backed out by 8fa9b9b607ff ☠ ☠
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 07 Jul 2016 15:06:08 +0300
changeset 377221 97214e3bf73dbd6a11857f3fd2c2719148cfbc30
parent 377220 18ea64fbf389b11d693899051eb182963dc283d2
child 377222 0b09769075afeffe18d71e41d15911f9cfeeaa5f
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1275746
milestone53.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 1275746 - Don't allow empty host name for URLTYPE_AUTHORITY URLs r=mcmanus * Return NS_ERROR_MALFORMED_URI if mURLType is URLTYPE_AUTHORITY and the hostname is empty. * Make sure nsStandardURL::SetFile calls init with the correct arguments MozReview-Commit-ID: 7t3mZtAbMF3
netwerk/base/nsStandardURL.cpp
netwerk/test/unit/test_URIs.js
netwerk/test/unit/test_standardurl.js
--- a/netwerk/base/nsStandardURL.cpp
+++ b/netwerk/base/nsStandardURL.cpp
@@ -1545,16 +1545,21 @@ nsStandardURL::SetSpec(const nsACString 
     // parse the given URL...
     nsresult rv = ParseURL(spec, specLength);
     if (NS_SUCCEEDED(rv)) {
         // finally, use the URLSegment member variables to build a normalized
         // copy of |spec|
         rv = BuildNormalizedSpec(spec);
     }
 
+    // Make sure that a URLTYPE_AUTHORITY has a non-empty hostname.
+    if (mURLType == URLTYPE_AUTHORITY && mHost.mLen == -1) {
+        rv = NS_ERROR_MALFORMED_URI;
+    }
+
     if (NS_FAILED(rv)) {
         Clear();
         // If parsing the spec has failed, restore the old URL
         // so we don't end up with an empty URL.
         CopyMembers(&prevURL, eHonorRef, EmptyCString());
         return rv;
     }
 
@@ -3173,30 +3178,36 @@ nsStandardURL::SetFile(nsIFile *file)
     NS_ENSURE_ARG_POINTER(file);
 
     nsresult rv;
     nsAutoCString url;
 
     rv = net_GetURLSpecFromFile(file, url);
     if (NS_FAILED(rv)) return rv;
 
-    SetSpec(url);
-
-    rv = Init(mURLType, mDefaultPort, url, nullptr, nullptr);
+    uint32_t oldURLType = mURLType;
+    uint32_t oldDefaultPort = mDefaultPort;
+    rv = Init(nsIStandardURL::URLTYPE_NO_AUTHORITY, -1, url, nullptr, nullptr);
+
+    if (NS_FAILED(rv)) {
+        // Restore the old url type and default port if the call to Init fails.
+        mURLType = oldURLType;
+        mDefaultPort = oldDefaultPort;
+        return rv;
+    }
 
     // must clone |file| since its value is not guaranteed to remain constant
-    if (NS_SUCCEEDED(rv)) {
-        InvalidateCache();
-        if (NS_FAILED(file->Clone(getter_AddRefs(mFile)))) {
-            NS_WARNING("nsIFile::Clone failed");
-            // failure to clone is not fatal (GetFile will generate mFile)
-            mFile = nullptr;
-        }
+    InvalidateCache();
+    if (NS_FAILED(file->Clone(getter_AddRefs(mFile)))) {
+        NS_WARNING("nsIFile::Clone failed");
+        // failure to clone is not fatal (GetFile will generate mFile)
+        mFile = nullptr;
     }
-    return rv;
+
+    return NS_OK;
 }
 
 //----------------------------------------------------------------------------
 // nsStandardURL::nsIStandardURL
 //----------------------------------------------------------------------------
 
 inline bool
 IsUTFCharset(const char *aCharset)
--- a/netwerk/test/unit/test_URIs.js
+++ b/netwerk/test/unit/test_URIs.js
@@ -87,28 +87,16 @@ var gTests = [
     nsIURL:  true, nsINestedURI: false },
   { spec:    "file:///dir/dir2/",
     scheme:  "file",
     prePath: "file://",
     path:    "/dir/dir2/data/text/plain,2",
     ref:     "",
     relativeURI: "data/text/plain,2",
     nsIURL:  true, nsINestedURI: false },
-  { spec:    "ftp://",
-    scheme:  "ftp",
-    prePath: "ftp://",
-    path:    "/",
-    ref:     "",
-    nsIURL:  true, nsINestedURI: false },
-  { spec:    "ftp:///",
-    scheme:  "ftp",
-    prePath: "ftp://",
-    path:    "/",
-    ref:     "",
-    nsIURL:  true, nsINestedURI: false },
   { spec:    "ftp://ftp.mozilla.org/pub/mozilla.org/README",
     scheme:  "ftp",
     prePath: "ftp://ftp.mozilla.org",
     path:    "/pub/mozilla.org/README",
     ref:     "",
     nsIURL:  true, nsINestedURI: false },
   { spec:    "ftp://foo:bar@ftp.mozilla.org:100/pub/mozilla.org/README",
     scheme:  "ftp",
@@ -130,28 +118,16 @@ var gTests = [
     nsIURL:  true, nsINestedURI: false },
   //Bug 706249
   { spec:    "gopher://mozilla.org/",
     scheme:  "gopher",
     prePath: "gopher:",
     path:    "//mozilla.org/",
     ref:     "",
     nsIURL:  false, nsINestedURI: false },
-  { spec:    "http://",
-    scheme:  "http",
-    prePath: "http://",
-    path:    "/",
-    ref:     "",
-    nsIURL:  true, nsINestedURI: false },
-  { spec:    "http:///",
-    scheme:  "http",
-    prePath: "http://",
-    path:    "/",
-    ref:     "",
-    nsIURL:  true, nsINestedURI: false },
   { spec:    "http://www.example.com/",
     scheme:  "http",
     prePath: "http://www.example.com",
     path:    "/",
     ref:     "",
     nsIURL:  true, nsINestedURI: false },
   { spec:    "http://www.exa\nmple.com/",
     scheme:  "http",
--- a/netwerk/test/unit/test_standardurl.js
+++ b/netwerk/test/unit/test_standardurl.js
@@ -328,16 +328,24 @@ add_test(function test_backslashReplacem
   url = stringToURL("http:\\\\test.com\\example.org/path\\to/file");
   do_check_eq(url.spec, "http://test.com/example.org/path/to/file");
   do_check_eq(url.host, "test.com");
   do_check_eq(url.path, "/example.org/path/to/file");
 
   run_next_test();
 });
 
+add_test(function test_authority_host()
+{
+  Assert.throws(() => { stringToURL("http:"); }, "TYPE_AUTHORITY should have host");
+  Assert.throws(() => { stringToURL("http:///"); }, "TYPE_AUTHORITY should have host");
+
+  run_next_test();
+});
+
 add_test(function test_trim_C0_and_space()
 {
   var url = stringToURL("\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f http://example.com/ \x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f ");
   do_check_eq(url.spec, "http://example.com/");
   url.spec = "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f http://test.com/ \x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f ";
   do_check_eq(url.spec, "http://test.com/");
   Assert.throws(() => { url.spec = "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19 "; }, "set empty spec");
   run_next_test();