Bug 1110476 - Stripped url fragment from Request::GetUrl() by calling either nsIURI::SetRef() or workers::URL::SetHash() in Request's url getter utility methods. Stripped url fragment from Response::GetUrl() by adding the method InternalRequest::StripFragmentAndSetUrl() which calls nsIURI::SetRef(). Added a test in dom/tests/mochitest/fetch/test_request.js for Request::GetUrl(). Removed manual url stripping from dom/cache/TypeUtils.cpp. r=bkelly

dom/cache/TypeUtils.cpp

@@ -414,25 +414,15 @@ TypeUtils::ProcessURL(nsACString& aUrl, bool* aSchemeValidOut,
 
   uint32_t queryPos;
   int32_t queryLen;
-  uint32_t refPos;
-  int32_t refLen;
 
   aRv = urlParser->ParsePath(url + pathPos, flatURL.Length() - pathPos,
                              nullptr, nullptr,               // ignore filepath
                              &queryPos, &queryLen,
-                             &refPos, &refLen);
+                             nullptr, nullptr);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }
 
-  // TODO: Remove this once Request/Response properly strip the fragment (bug 1110476)
-  if (refLen >= 0) {
-    // ParsePath gives us ref position relative to the start of the path
-    refPos += pathPos;
-
-    aUrl = Substring(aUrl, 0, refPos - 1);
-  }
-
   if (!aUrlWithoutQueryOut) {
     return;
   }

dom/fetch/FetchDriver.cpp

@@ -4,6 +4,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include "mozilla/DebugOnly.h"
 #include "mozilla/dom/FetchDriver.h"
 
 #include "nsIDocument.h"
@@ -580,7 +581,8 @@ FetchDriver::BeginAndGetFilteredResponse(InternalResponse* aResponse, nsIURI* aF
   } else {
     mRequest->GetURL(reqURL);
   }
-  aResponse->SetUrl(reqURL);
+  DebugOnly<nsresult> rv = aResponse->StripFragmentAndSetUrl(reqURL);
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
 
   // FIXME(nsm): Handle mixed content check, step 7 of fetch.
 

dom/fetch/InternalRequest.h

@@ -374,6 +374,7 @@ private:
   MapContentPolicyTypeToRequestContext(nsContentPolicyType aContentPolicyType);
 
   nsCString mMethod;
+  // mURL always stores the url with the ref stripped
   nsCString mURL;
   nsRefPtr<InternalHeaders> mHeaders;
   nsCOMPtr<nsIInputStream> mBodyStream;

dom/fetch/InternalResponse.cpp

@@ -6,9 +6,11 @@
 
 #include "InternalResponse.h"
 
+#include "mozilla/Assertions.h"
 #include "mozilla/dom/InternalHeaders.h"
 #include "mozilla/dom/cache/CacheTypes.h"
 #include "mozilla/ipc/PBackgroundSharedTypes.h"
+#include "nsIURI.h"
 #include "nsStreamUtils.h"
 
 namespace mozilla {
@@ -85,6 +87,37 @@ InternalResponse::SetPrincipalInfo(UniquePtr<mozilla::ipc::PrincipalInfo> aPrinc
   mPrincipalInfo = Move(aPrincipalInfo);
 }
 
+nsresult
+InternalResponse::StripFragmentAndSetUrl(const nsACString& aUrl)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  nsCOMPtr<nsIURI> iuri;
+  nsresult rv;
+
+  rv = NS_NewURI(getter_AddRefs(iuri), aUrl);
+  if(NS_WARN_IF(NS_FAILED(rv))){
+    return rv;
+  }
+
+  nsCOMPtr<nsIURI> iuriClone;
+  // We use CloneIgnoringRef to strip away the fragment even if the original URI
+  // is immutable.
+  rv = iuri->CloneIgnoringRef(getter_AddRefs(iuriClone));
+  if(NS_WARN_IF(NS_FAILED(rv))){
+    return rv;
+  }
+
+  nsCString spec;
+  rv = iuriClone->GetSpec(spec);
+  if(NS_WARN_IF(NS_FAILED(rv))){
+    return rv;
+  }
+
+  SetUrl(spec);
+  return NS_OK;
+}
+
 already_AddRefed<InternalResponse>
 InternalResponse::OpaqueResponse()
 {

dom/fetch/InternalResponse.h

@@ -78,6 +78,7 @@ public:
     aURL.Assign(mURL);
   }
 
+  // SetUrl should only be called when the fragment has alredy been stripped
   void
   SetUrl(const nsACString& aURL)
   {
@@ -179,6 +180,9 @@ public:
   void
   SetPrincipalInfo(UniquePtr<mozilla::ipc::PrincipalInfo> aPrincipalInfo);
 
+  nsresult
+  StripFragmentAndSetUrl(const nsACString& aUrl);
+
 private:
   ~InternalResponse();
 

dom/fetch/Request.cpp

@@ -84,8 +84,16 @@ GetRequestURLFromDocument(nsIDocument* aDocument, const nsAString& 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;
+  }
+
   nsAutoCString spec;
-  aRv = resolvedURI->GetSpec(spec);
+  aRv = resolvedURIClone->GetSpec(spec);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }
@@ -99,23 +107,27 @@ GetRequestURLFromChrome(const nsAString& aInput, nsAString& aRequestURL,
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-#ifdef DEBUG
   nsCOMPtr<nsIURI> uri;
   aRv = NS_NewURI(getter_AddRefs(uri), aInput, nullptr, nullptr);
   if (NS_WARN_IF(aRv.Failed())) {
     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())) {
+    return;
+  }
+
   nsAutoCString spec;
-  aRv = uri->GetSpec(spec);
+  aRv = uriClone->GetSpec(spec);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }
 
   CopyUTF8toUTF16(spec, aRequestURL);
-#else
-  aRequestURL = aInput;
-#endif
 }
 
 void
@@ -133,6 +145,11 @@ GetRequestURLFromWorker(const GlobalObject& aGlobal, const nsAString& aInput,
     return;
   }
 
+  url->SetHash(EmptyString(), aRv);
+  if (NS_WARN_IF(aRv.Failed())) {
+    return;
+  }
+
   url->Stringify(aRequestURL, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return;

dom/tests/mochitest/fetch/test_request.js

@@ -216,7 +216,7 @@ function testMethod() {
 
 function testUrlFragment() {
   var req = new Request("./request#withfragment");
-  ok(req.url, (new URL("./request", self.location.href)).href, "request.url should be serialized with exclude fragment flag set");
+  is(req.url, (new URL("./request", self.location.href)).href, "request.url should be serialized with exclude fragment flag set");
 }
 
 function testBodyUsed() {