Bug 1150001 - Cache API should not return Response body when matching Request with HEAD method. r=bkelly
authorTimur Valeev <tvaleev@gmail.com>
Thu, 23 Apr 2015 16:17:37 +0300
changeset 241209 77a78b57afbf9325b2dab6c3c3726c7f938a5246
parent 241208 61f85e0e123578a696217d1840048ac9ac3cb7b9
child 241210 48ea157bc34fdd7f6a9aa579e702604f9f82ffd0
push id28655
push userryanvm@gmail.com
push dateMon, 27 Apr 2015 19:14:23 +0000
treeherdermozilla-central@caf25344f73e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1150001
milestone40.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 1150001 - Cache API should not return Response body when matching Request with HEAD method. r=bkelly
dom/cache/Manager.cpp
dom/cache/test/mochitest/test_cache_matchAll_request.js
dom/cache/test/mochitest/test_cache_match_request.js
--- a/dom/cache/Manager.cpp
+++ b/dom/cache/Manager.cpp
@@ -125,16 +125,34 @@ private:
 };
 
 } // anonymous namespace
 
 namespace mozilla {
 namespace dom {
 namespace cache {
 
+namespace {
+
+bool IsHeadRequest(CacheRequest aRequest, CacheQueryParams aParams)
+{
+  return !aParams.ignoreMethod() && aRequest.method().LowerCaseEqualsLiteral("head");
+}
+
+bool IsHeadRequest(CacheRequestOrVoid aRequest, CacheQueryParams aParams)
+{
+  if (aRequest.type() == CacheRequestOrVoid::TCacheRequest) {
+    return !aParams.ignoreMethod() &&
+           aRequest.get_CacheRequest().method().LowerCaseEqualsLiteral("head");
+  }
+  return false;
+}
+
+} // anonymous namespace
+
 // ----------------------------------------------------------------------------
 
 // Singleton class to track Manager instances and ensure there is only
 // one for each unique ManagerId.
 class Manager::Factory
 {
 public:
   friend class StaticAutoPtr<Manager::Factory>;
@@ -505,17 +523,19 @@ public:
   virtual nsresult
   RunSyncWithDBOnTarget(const QuotaInfo& aQuotaInfo, nsIFile* aDBDir,
                         mozIStorageConnection* aConn) override
   {
     nsresult rv = db::CacheMatch(aConn, mCacheId, mArgs.request(),
                                  mArgs.params(), &mFoundResponse, &mResponse);
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
-    if (!mFoundResponse || !mResponse.mHasBodyId) {
+    if (!mFoundResponse || !mResponse.mHasBodyId
+                        || IsHeadRequest(mArgs.request(), mArgs.params())) {
+      mResponse.mHasBodyId = false;
       return rv;
     }
 
     nsCOMPtr<nsIInputStream> stream;
     rv = BodyOpen(aQuotaInfo, aDBDir, mResponse.mBodyId, getter_AddRefs(stream));
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
     if (NS_WARN_IF(!stream)) { return NS_ERROR_FILE_NOT_FOUND; }
 
@@ -568,17 +588,19 @@ public:
   RunSyncWithDBOnTarget(const QuotaInfo& aQuotaInfo, nsIFile* aDBDir,
                         mozIStorageConnection* aConn) override
   {
     nsresult rv = db::CacheMatchAll(aConn, mCacheId, mArgs.requestOrVoid(),
                                     mArgs.params(), mSavedResponses);
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
     for (uint32_t i = 0; i < mSavedResponses.Length(); ++i) {
-      if (!mSavedResponses[i].mHasBodyId) {
+      if (!mSavedResponses[i].mHasBodyId
+          || IsHeadRequest(mArgs.requestOrVoid(), mArgs.params())) {
+        mSavedResponses[i].mHasBodyId = false;
         continue;
       }
 
       nsCOMPtr<nsIInputStream> stream;
       rv = BodyOpen(aQuotaInfo, aDBDir, mSavedResponses[i].mBodyId,
                     getter_AddRefs(stream));
       if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
       if (NS_WARN_IF(!stream)) { return NS_ERROR_FILE_NOT_FOUND; }
@@ -1057,17 +1079,19 @@ public:
   RunSyncWithDBOnTarget(const QuotaInfo& aQuotaInfo, nsIFile* aDBDir,
                         mozIStorageConnection* aConn) override
   {
     nsresult rv = db::CacheKeys(aConn, mCacheId, mArgs.requestOrVoid(),
                                 mArgs.params(), mSavedRequests);
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
     for (uint32_t i = 0; i < mSavedRequests.Length(); ++i) {
-      if (!mSavedRequests[i].mHasBodyId) {
+      if (!mSavedRequests[i].mHasBodyId
+          || IsHeadRequest(mArgs.requestOrVoid(), mArgs.params())) {
+        mSavedRequests[i].mHasBodyId = false;
         continue;
       }
 
       nsCOMPtr<nsIInputStream> stream;
       rv = BodyOpen(aQuotaInfo, aDBDir, mSavedRequests[i].mBodyId,
                     getter_AddRefs(stream));
       if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
       if (NS_WARN_IF(!stream)) { return NS_ERROR_FILE_NOT_FOUND; }
@@ -1119,17 +1143,19 @@ public:
   RunSyncWithDBOnTarget(const QuotaInfo& aQuotaInfo, nsIFile* aDBDir,
                         mozIStorageConnection* aConn) override
   {
     nsresult rv = db::StorageMatch(aConn, mNamespace, mArgs.request(),
                                    mArgs.params(), &mFoundResponse,
                                    &mSavedResponse);
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
-    if (!mFoundResponse || !mSavedResponse.mHasBodyId) {
+    if (!mFoundResponse || !mSavedResponse.mHasBodyId
+                        || IsHeadRequest(mArgs.request(), mArgs.params())) {
+      mSavedResponse.mHasBodyId = false;
       return rv;
     }
 
     nsCOMPtr<nsIInputStream> stream;
     rv = BodyOpen(aQuotaInfo, aDBDir, mSavedResponse.mBodyId,
                   getter_AddRefs(stream));
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
     if (NS_WARN_IF(!stream)) { return NS_ERROR_FILE_NOT_FOUND; }
--- a/dom/cache/test/mochitest/test_cache_matchAll_request.js
+++ b/dom/cache/test/mochitest/test_cache_matchAll_request.js
@@ -73,16 +73,21 @@ function testRequest(request1, request2,
     return c.matchAll(request1);
   }).then(function(r) {
     is(r.length, 1, "Should only find 1 item");
     return checkResponse(r[0], response1, response1Text);
   }).then(function() {
     return c.matchAll(new Request(request1, {method: "HEAD"}));
   }).then(function(r) {
     is(r.length, 1, "Should only find 1 item");
+    return checkResponse(r[0], response1, "");
+  }).then(function() {
+    return c.matchAll(new Request(request1, {method: "HEAD"}), {ignoreMethod: true});
+  }).then(function(r) {
+    is(r.length, 1, "Should only find 1 item");
     return checkResponse(r[0], response1, response1Text);
   }).then(function() {
     return Promise.all(
       ["POST", "PUT", "DELETE", "OPTIONS"]
         .map(function(method) {
           var req = new Request(request1, {method: method});
           return c.matchAll(req)
             .then(function(r) {
--- a/dom/cache/test/mochitest/test_cache_match_request.js
+++ b/dom/cache/test/mochitest/test_cache_match_request.js
@@ -1,28 +1,31 @@
 var request = new Request("//mochi.test:8888/?" + context + "#fragment");
 var requestWithAltQS = new Request("//mochi.test:8888/?queryString");
 var unknownRequest = new Request("//mochi.test:8888/non/existing/path?" + context);
 var response;
 var c;
 var responseText;
 var name = "match-request" + context;
 
-function checkResponse(r) {
+function checkResponse(r, expectedBody) {
+  if (expectedBody === undefined) {
+    expectedBody = responseText;
+  }
   ok(r !== response, "The objects should not be the same");
   is(r.url, response.url.replace("#fragment", ""),
      "The URLs should be the same");
   is(r.status, response.status, "The status codes should be the same");
   is(r.type, response.type, "The response types should be the same");
   is(r.ok, response.ok, "Both responses should have succeeded");
   is(r.statusText, response.statusText,
      "Both responses should have the same status text");
   return r.text().then(function(text) {
     // Avoid dumping out the large response text to the log if they're equal.
-    if (text !== responseText) {
+    if (text !== expectedBody) {
       is(text, responseText, "The response body should be correct");
     }
   });
 }
 
 fetch(new Request(request)).then(function(r) {
   response = r;
   return response.text();
@@ -58,16 +61,20 @@ function testRequest(request, unknownReq
     );
   }).then(function() {
     return c.match(request);
   }).then(function(r) {
     return checkResponse(r);
   }).then(function() {
     return c.match(new Request(request, {method: "HEAD"}));
   }).then(function(r) {
+    return checkResponse(r, '');
+  }).then(function() {
+    return c.match(new Request(request, {method: "HEAD"}), {ignoreMethod: true});
+  }).then(function(r) {
     return checkResponse(r);
   }).then(function() {
     return Promise.all(
       ["POST", "PUT", "DELETE", "OPTIONS"]
         .map(function(method) {
           var req = new Request(request, {method: method});
           return c.match(req)
             .then(function(r) {