Bug 1009648: nsStandardURL crashes when calling SetHost after Clear() r=mcmanus
authorValentin Gosu <valentin.gosu@gmail.com>
Mon, 11 Aug 2014 12:14:03 +0300
changeset 198842 f7ff242d4af8dc532e0bf27f091668313f632c35
parent 198841 bf19c9da15186f9d26e0ed89855ed55a1dd31bec
child 198843 ffe2d5537d2d83f3eb991b24fa21aac1c9529262
push id47503
push uservalentin.gosu@gmail.com
push dateMon, 11 Aug 2014 09:14:50 +0000
treeherdermozilla-inbound@ffe2d5537d2d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1009648
milestone34.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 1009648: nsStandardURL crashes when calling SetHost after Clear() r=mcmanus
netwerk/base/src/nsStandardURL.cpp
netwerk/base/src/nsStandardURL.h
netwerk/test/unit/test_standardurl.js
--- a/netwerk/base/src/nsStandardURL.cpp
+++ b/netwerk/base/src/nsStandardURL.cpp
@@ -1130,21 +1130,24 @@ nsStandardURL::SetSpec(const nsACString 
     ENSURE_MUTABLE();
 
     const nsPromiseFlatCString &flat = PromiseFlatCString(input);
     const char *spec = flat.get();
     int32_t specLength = flat.Length();
 
     LOG(("nsStandardURL::SetSpec [spec=%s]\n", spec));
 
+    if (!spec || !*spec)
+        return NS_ERROR_MALFORMED_URI;
+
+    // Make a backup of the curent URL
+    nsStandardURL prevURL;
+    prevURL.CopyMembers(this, eHonorRef);
     Clear();
 
-    if (!spec || !*spec)
-        return NS_OK;
-
     // filter out unexpected chars "\r\n\t" if necessary
     nsAutoCString buf1;
     if (net_FilterURIString(spec, buf1)) {
         spec = buf1.get();
         specLength = buf1.Length();
     }
 
     // parse the given URL...
@@ -1152,16 +1155,19 @@ nsStandardURL::SetSpec(const nsACString 
     if (NS_SUCCEEDED(rv)) {
         // finally, use the URLSegment member variables to build a normalized
         // copy of |spec|
         rv = BuildNormalizedSpec(spec);
     }
 
     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);
         return rv;
     }
 
 #if defined(PR_LOGGING)
     if (LOG_ENABLED()) {
         LOG((" spec      = %s\n", mSpec.get()));
         LOG((" port      = %d\n", mPort));
         LOG((" scheme    = (%u,%d)\n", mScheme.mPos,    mScheme.mLen));
@@ -1830,46 +1836,64 @@ nsresult
 nsStandardURL::CloneInternal(nsStandardURL::RefHandlingEnum refHandlingMode,
                              nsIURI **result)
 
 {
     nsRefPtr<nsStandardURL> clone = StartClone();
     if (!clone)
         return NS_ERROR_OUT_OF_MEMORY;
 
-    clone->mSpec = mSpec;
-    clone->mDefaultPort = mDefaultPort;
-    clone->mPort = mPort;
-    clone->mScheme = mScheme;
-    clone->mAuthority = mAuthority;
-    clone->mUsername = mUsername;
-    clone->mPassword = mPassword;
-    clone->mHost = mHost;
-    clone->mPath = mPath;
-    clone->mFilepath = mFilepath;
-    clone->mDirectory = mDirectory;
-    clone->mBasename = mBasename;
-    clone->mExtension = mExtension;
-    clone->mQuery = mQuery;
-    clone->mRef = mRef;
-    clone->mOriginCharset = mOriginCharset;
-    clone->mURLType = mURLType;
-    clone->mParser = mParser;
-    clone->mFile = mFile;
-    clone->mHostA = mHostA ? strdup(mHostA) : nullptr;
-    clone->mMutable = true;
-    clone->mSupportsFileURL = mSupportsFileURL;
-    clone->mHostEncoding = mHostEncoding;
-    clone->mSpecEncoding = mSpecEncoding;
+    // Copy local members into clone.
+    // Also copies the cached members mFile, mHostA
+    clone->CopyMembers(this, refHandlingMode, true);
+
+    clone.forget(result);
+    return NS_OK;
+}
+
+nsresult nsStandardURL::CopyMembers(nsStandardURL * source,
+    nsStandardURL::RefHandlingEnum refHandlingMode, bool copyCached)
+{
+    mSpec = source->mSpec;
+    mDefaultPort = source->mDefaultPort;
+    mPort = source->mPort;
+    mScheme = source->mScheme;
+    mAuthority = source->mAuthority;
+    mUsername = source->mUsername;
+    mPassword = source->mPassword;
+    mHost = source->mHost;
+    mPath = source->mPath;
+    mFilepath = source->mFilepath;
+    mDirectory = source->mDirectory;
+    mBasename = source->mBasename;
+    mExtension = source->mExtension;
+    mQuery = source->mQuery;
+    mRef = source->mRef;
+    mOriginCharset = source->mOriginCharset;
+    mURLType = source->mURLType;
+    mParser = source->mParser;
+    mMutable = true;
+    mSupportsFileURL = source->mSupportsFileURL;
+    mHostEncoding = source->mHostEncoding;
+
+    if (copyCached) {
+        mFile = source->mFile;
+        mHostA = source->mHostA ? strdup(source->mHostA) : nullptr;
+        mSpecEncoding = source->mSpecEncoding;
+    } else {
+        // The same state as after calling InvalidateCache()
+        mFile = nullptr;
+        mHostA = nullptr;
+        mSpecEncoding = eEncoding_Unknown;
+    }
 
     if (refHandlingMode == eIgnoreRef) {
-        clone->SetRef(EmptyCString());
+        SetRef(EmptyCString());
     }
 
-    clone.forget(result);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsStandardURL::Resolve(const nsACString &in, nsACString &out)
 {
     const nsPromiseFlatCString &flat = PromiseFlatCString(in);
     const char *relpath = flat.get();
--- a/netwerk/base/src/nsStandardURL.h
+++ b/netwerk/base/src/nsStandardURL.h
@@ -151,16 +151,20 @@ protected:
                             RefHandlingEnum refHandlingMode,
                             bool* result);
 
     virtual nsStandardURL* StartClone();
 
     // Helper to share code between Clone methods.
     nsresult CloneInternal(RefHandlingEnum aRefHandlingMode,
                            nsIURI** aClone);
+    // Helper method that copies member variables from the source StandardURL
+    // if copyCached = true, it will also copy mFile and mHostA
+    nsresult CopyMembers(nsStandardURL * source, RefHandlingEnum mode,
+                         bool copyCached = false);
 
     // Helper for subclass implementation of GetFile().  Subclasses that map
     // URIs to files in a special way should implement this method.  It should
     // ensure that our mFile is initialized, if it's possible.
     // returns NS_ERROR_NO_INTERFACE if the url does not map to a file
     virtual nsresult EnsureFile();
 
 private:
--- a/netwerk/test/unit/test_standardurl.js
+++ b/netwerk/test/unit/test_standardurl.js
@@ -203,16 +203,29 @@ function test_ipv6_fail()
   Assert.throws(() => { url.hostPort = "[2001:1"; }, "bad IPv6 address");
   Assert.throws(() => { url.hostPort = "2001]:1"; }, "bad IPv6 address");
   Assert.throws(() => { url.hostPort = "2001:1]"; }, "bad IPv6 address");
   Assert.throws(() => { url.hostPort = ""; }, "Empty hostPort should fail");
   Assert.throws(() => { url.hostPort = "[2001::1]:"; }, "missing port number");
   Assert.throws(() => { url.hostPort = "[2001::1]:bad"; }, "bad port number");
 }
 
+function test_clearedSpec()
+{
+  var url = stringToURL("http://example.com/path");
+  Assert.throws(() => { url.spec = "http: example"; }, "set bad spec");
+  Assert.throws(() => { url.spec = ""; }, "set empty spec");
+  do_check_eq(url.spec, "http://example.com/path");
+  url.host = "allizom.org";
+
+  var ref = stringToURL("http://allizom.org/path");
+  symmetricEquality(true, url, ref);
+}
+
 function run_test()
 {
   test_setEmptyPath();
   test_setQuery();
   test_setRef();
   test_ipv6();
   test_ipv6_fail();
+  test_clearedSpec();
 }