Bug 1143894 - Part 3: Do not propagate errors in getting the headers to the outside world; r=bkelly
authorEhsan Akhgari <ehsan@mozilla.com>
Tue, 17 Mar 2015 10:57:05 -0400
changeset 234957 edf516516fb35eb9811e7655251eeb90d244cfb1
parent 234956 3ea234420460f0e87a05df0bf4ff2e36b5b03442
child 234958 53bb870dcb5d6e3e9f42b7d5fd71fdc9e6cd1f77
push id28458
push userphilringnalda@gmail.com
push dateSun, 22 Mar 2015 20:58:14 +0000
treeherdermozilla-central@99865532c12f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1143894
milestone39.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 1143894 - Part 3: Do not propagate errors in getting the headers to the outside world; r=bkelly The Vary header may contain invalid header name values. We should just ignore such values as opposed to propagating them to the caller. Before this patch, attempts to add a request with such a Vary header for example would fail, since the internal QueryCache() call when executing the CachePutAllAction would fail.
dom/cache/DBSchema.cpp
dom/cache/FetchPut.cpp
--- a/dom/cache/DBSchema.cpp
+++ b/dom/cache/DBSchema.cpp
@@ -791,26 +791,26 @@ DBSchema::MatchByVaryHeader(mozIStorageC
   ), getter_AddRefs(state));
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
   rv = state->BindInt32Parameter(0, entryId);
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
   nsRefPtr<InternalHeaders> cachedHeaders = new InternalHeaders(HeadersGuardEnum::None);
 
-  ErrorResult errorResult;
-
   while (NS_SUCCEEDED(state->ExecuteStep(&hasMoreData)) && hasMoreData) {
     nsAutoCString name;
     nsAutoCString value;
     rv = state->GetUTF8String(0, name);
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
     rv = state->GetUTF8String(1, value);
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
+    ErrorResult errorResult;
+
     cachedHeaders->Append(name, value, errorResult);
     if (errorResult.Failed()) { return errorResult.ErrorCode(); };
   }
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
   nsRefPtr<InternalHeaders> queryHeaders = new InternalHeaders(aRequest.headers());
 
   // Assume the vary headers match until we find a conflict
@@ -824,28 +824,29 @@ DBSchema::MatchByVaryHeader(mozIStorageC
     bool bailOut = false;
     for (; token;
          token = nsCRT::strtok(rawBuffer, NS_HTTP_HEADER_SEPS, &rawBuffer)) {
       nsDependentCString header(token);
       if (header.EqualsLiteral("*")) {
         continue;
       }
 
+      ErrorResult errorResult;
       nsAutoCString queryValue;
       queryHeaders->Get(header, queryValue, errorResult);
       if (errorResult.Failed()) {
         errorResult.ClearMessage();
-        return errorResult.ErrorCode();
+        MOZ_ASSERT(queryValue.IsEmpty());
       }
 
       nsAutoCString cachedValue;
       cachedHeaders->Get(header, cachedValue, errorResult);
       if (errorResult.Failed()) {
         errorResult.ClearMessage();
-        return errorResult.ErrorCode();
+        MOZ_ASSERT(cachedValue.IsEmpty());
       }
 
       if (queryValue != cachedValue) {
         varyHeadersMatch = false;
         bailOut = true;
         break;
       }
     }
--- a/dom/cache/FetchPut.cpp
+++ b/dom/cache/FetchPut.cpp
@@ -376,35 +376,29 @@ FetchPut::MatchInPutList(const PCacheReq
       bool bailOut = false;
       for (; token;
            token = nsCRT::strtok(rawBuffer, NS_HTTP_HEADER_SEPS, &rawBuffer)) {
         nsDependentCString header(token);
         if (header.EqualsLiteral("*")) {
           continue;
         }
 
-        // The VARY header could in theory contain an illegal header name.  So
-        // we need to detect the error in the Get() calls below.  Treat these
-        // as not matching.
         ErrorResult headerRv;
-
         nsAutoCString value;
-        requestHeaders->Get(header, value, rv);
-        if (NS_WARN_IF(rv.Failed())) {
-          varyHeadersMatch = false;
-          bailOut = true;
-          break;
+        requestHeaders->Get(header, value, headerRv);
+        if (NS_WARN_IF(headerRv.Failed())) {
+          headerRv.ClearMessage();
+          MOZ_ASSERT(value.IsEmpty());
         }
 
         nsAutoCString cachedValue;
-        cachedRequestHeaders->Get(header, value, rv);
-        if (NS_WARN_IF(rv.Failed())) {
-          varyHeadersMatch = false;
-          bailOut = true;
-          break;
+        cachedRequestHeaders->Get(header, value, headerRv);
+        if (NS_WARN_IF(headerRv.Failed())) {
+          headerRv.ClearMessage();
+          MOZ_ASSERT(cachedValue.IsEmpty());
         }
 
         if (value != cachedValue) {
           varyHeadersMatch = false;
           bailOut = true;
           break;
         }
       }