Bug 460335 - disallow redirects when updating an application cache, r=dcamp, sr=cbiesinger
authorHonza Bambas <honzab.moz@firemni.cz>
Fri, 27 Feb 2009 15:09:06 +0100
changeset 25596 413fc69d3b3d17b4130c9d73aa39243590371732
parent 25595 d9068a1a0648327734f6f8340b75e79aed4a3213
child 25597 5dde4d86be49af09865236161c33cd064a046bb0
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdcamp, cbiesinger
bugs460335
milestone1.9.2a1pre
Bug 460335 - disallow redirects when updating an application cache, r=dcamp, sr=cbiesinger
dom/tests/mochitest/ajax/offline/Makefile.in
dom/tests/mochitest/ajax/offline/dynamicRedirect.sjs
dom/tests/mochitest/ajax/offline/explicitRedirect.sjs
dom/tests/mochitest/ajax/offline/manifestRedirect.sjs
dom/tests/mochitest/ajax/offline/obsolete.html
dom/tests/mochitest/ajax/offline/redirects.sjs
dom/tests/mochitest/ajax/offline/test_redirectManifest.html
dom/tests/mochitest/ajax/offline/test_redirectUpdateItem.html
uriloader/prefetch/nsOfflineCacheUpdate.cpp
uriloader/prefetch/nsOfflineCacheUpdate.h
--- a/dom/tests/mochitest/ajax/offline/Makefile.in
+++ b/dom/tests/mochitest/ajax/offline/Makefile.in
@@ -61,16 +61,18 @@ include $(topsrcdir)/config/rules.mk
 	test_offlineIFrame.html \
 	test_offlineMode.html \
 	test_bug445544.html \
 	test_bug460353.html \
 	test_bug474696.html \
 	test_foreign.html \
 	test_fallback.html \
 	test_overlap.html \
+	test_redirectManifest.html \
+	test_redirectUpdateItem.html \
 	overlap.cacheManifest \
 	overlap.cacheManifest^headers^ \
 	test_updatingManifest.html \
 	445544_part1.html \
 	445544_part2.html \
 	445544.cacheManifest \
 	445544.cacheManifest^headers^ \
 	460353_iframe_nomanifest.html \
@@ -79,33 +81,37 @@ include $(topsrcdir)/config/rules.mk
 	test_obsolete.html \
 	obsolete.html \
 	obsoletingManifest.sjs \
 	badManifestMagic.cacheManifest \
 	badManifestMagic.cacheManifest^headers^ \
 	bypass.cacheManifest \
 	bypass.cacheManifest^headers^ \
 	bypass.html \
+	dynamicRedirect.sjs \
+	explicitRedirect.sjs \
 	fallback.html \
 	fallback2.html \
 	fallbackTop.html \
 	fallback.cacheManifest \
 	fallback.cacheManifest^headers^ \
 	foreign1.cacheManifest \
 	foreign1.cacheManifest^headers^ \
 	foreign2.cacheManifest \
 	foreign2.cacheManifest^headers^ \
 	foreign2.html \
 	notonwhitelist.html \
 	onwhitelist.html \
 	onwhitelist.html^headers^ \
 	updatingIframe.sjs \
 	updatingImplicit.html \
+	manifestRedirect.sjs \
 	missingFile.cacheManifest \
 	missingFile.cacheManifest^headers^ \
+	redirects.sjs \
 	simpleManifest.cacheManifest \
 	simpleManifest.cacheManifest^headers^ \
 	updatingManifest.sjs \
 	simpleManifest.notmanifest \
 	changing1Sec.sjs \
 	changing1Hour.sjs \
 	changingManifest.sjs \
 	offlineChild.html \
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/ajax/offline/dynamicRedirect.sjs
@@ -0,0 +1,27 @@
+function handleRequest(request, response)
+{
+  var match = request.queryString.match(/^state=(.*)$/);
+  if (match)
+  {
+    response.setStatusLine(request.httpVersion, 200, "No content");
+    setState("state", match[1]);
+    response.write("state='" + match[1] + "'");
+  }
+
+  if (request.queryString == "")
+  {
+    switch (getState("state"))
+    {
+      case "": // The default value
+        response.setStatusLine(request.httpVersion, 307, "Moved temporarly");
+        response.setHeader("Location", "http://example.com/non-existing-dynamic.html");
+        response.setHeader("Content-Type", "text/html");
+        break;
+      case "on":
+        response.setStatusLine(request.httpVersion, 200, "OK");
+        response.setHeader("Content-Type", "text/html");
+        response.write("<html><body>Dynamic page</body></html>");
+        break;
+    }
+  }
+}
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/ajax/offline/explicitRedirect.sjs
@@ -0,0 +1,27 @@
+function handleRequest(request, response)
+{
+  var match = request.queryString.match(/^state=(.*)$/);
+  if (match)
+  {
+    response.setStatusLine(request.httpVersion, 200, "No content");
+    setState("state", match[1]);
+    response.write("state='" + match[1] + "'");
+  }
+
+  if (request.queryString == "")
+  {
+    switch (getState("state"))
+    {
+      case "": // The default value
+        response.setStatusLine(request.httpVersion, 307, "Moved temporarly");
+        response.setHeader("Location", "http://example.com/non-existing-explicit.html");
+        response.setHeader("Content-Type", "text/html");
+        break;
+      case "on":
+        response.setStatusLine(request.httpVersion, 200, "OK");
+        response.setHeader("Content-Type", "text/html");
+        response.write("<html><body>Explicit page</body></html>");
+        break;
+    }
+  }
+}
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/ajax/offline/manifestRedirect.sjs
@@ -0,0 +1,6 @@
+function handleRequest(request, response)
+{
+  response.setStatusLine(request.httpVersion, 307, "Moved temporarly");
+  response.setHeader("Location", "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updating.cacheManifest");
+  response.setHeader("Content-Type", "text/cache-manifest");
+}
--- a/dom/tests/mochitest/ajax/offline/obsolete.html
+++ b/dom/tests/mochitest/ajax/offline/obsolete.html
@@ -39,16 +39,18 @@ applicationCache.oncached = function() {
     applicationCache.swapCache();
     window.opener.todo(false, "We shouldn't have to swapCache in the oncached handler (bug 443023)");
   } catch(e) {
   }
 
   // Now delete the manifest and refresh, we should get an "obsolete" message.
   applicationCache.oncached = fail;
   applicationCache.onupdateready = fail;
+  applicationCache.onnoupdate = fail;
+  applicationCache.onerror = fail;
   applicationCache.onobsolete = obsolete;
 
   // Make the obsoleting.sjs return 404 NOT FOUND code
   var req = new XMLHttpRequest();
   req.open("GET", "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/obsoletingManifest.sjs?state=");
   var channel = req.channel
     .QueryInterface(Components.interfaces.nsIApplicationCacheChannel);
   channel.chooseApplicationCache = false;
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/ajax/offline/redirects.sjs
@@ -0,0 +1,55 @@
+ver1manifest =
+  "CACHE MANIFEST\n" +
+  "# v1\n" +
+  "\n" +
+  "http://localhost:8888/tests/SimpleTest/SimpleTest.js\n" +
+  "http://localhost:8888/MochiKit/packed.js\n" +
+  "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/offlineTests.js\n" +
+  "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/explicitRedirect.sjs";
+
+ver2manifest =
+  "CACHE MANIFEST\n" +
+  "# v2\n" +
+  "\n" +
+  "http://localhost:8888/tests/SimpleTest/SimpleTest.js\n" +
+  "http://localhost:8888/MochiKit/packed.js\n" +
+  "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/offlineTests.js\n" +
+  "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/explicitRedirect.sjs";
+
+ver3manifest =
+  "CACHE MANIFEST\n" +
+  "# v3\n" +
+  "\n" +
+  "http://localhost:8888/tests/SimpleTest/SimpleTest.js\n" +
+  "http://localhost:8888/MochiKit/packed.js\n" +
+  "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/offlineTests.js\n" +
+  "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/explicitRedirect.sjs";
+
+function handleRequest(request, response)
+{
+  var match = request.queryString.match(/^state=(.*)$/);
+  if (match)
+  {
+    response.setStatusLine(request.httpVersion, 204, "No content");
+    setState("state", match[1]);
+  }
+
+  if (request.queryString == "")
+  {
+    response.setStatusLine(request.httpVersion, 200, "Ok");
+    response.setHeader("Content-Type", "text/cache-manifest");
+    response.setHeader("Cache-Control", "no-cache");
+    switch (getState("state"))
+    {
+      case "": // The default value
+        response.write(ver1manifest + "\n#" + getState("state"));
+        break;
+      case "second":
+        response.write(ver2manifest + "\n#" + getState("state"));
+        break;
+      case "third":
+        response.write(ver3manifest + "\n#" + getState("state"));
+        break;
+    }
+  }
+}
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/ajax/offline/test_redirectManifest.html
@@ -0,0 +1,46 @@
+<html xmlns="http://www.w3.org/1999/xhtml" manifest="http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/manifestRedirect.sjs">
+<head>
+<title>Fail update on manifest redirection test</title>
+
+<script type="text/javascript" src="/MochiKit/packed.js"></script>
+<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+<script type="text/javascript" src="/tests/dom/tests/mochitest/ajax/offline/offlineTests.js"></script>
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+
+<script class="testbody" type="text/javascript">
+
+/**
+ */
+
+function manifestCached()
+{
+  OfflineTests.ok(false, "Manifest must not be cached");
+  finish();
+}
+
+function manifestError()
+{
+  OfflineTest.ok(true, "Error expected");
+  finish();
+}
+
+function finish()
+{
+  OfflineTest.teardown();
+  OfflineTest.finish();
+}
+
+SimpleTest.waitForExplicitFinish();
+
+if (OfflineTest.setup()) {
+  applicationCache.onerror = OfflineTest.priv(manifestError);
+  applicationCache.oncached = OfflineTest.priv(manifestCached);
+}
+
+</script>
+
+</head>
+
+<body>
+</body>
+</html>
new file mode 100755
--- /dev/null
+++ b/dom/tests/mochitest/ajax/offline/test_redirectUpdateItem.html
@@ -0,0 +1,128 @@
+<html xmlns="http://www.w3.org/1999/xhtml" manifest="http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/redirects.sjs">
+<head>
+<title>Entries redirection handling during update test</title>
+
+<script type="text/javascript" src="/MochiKit/packed.js"></script>
+<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+<script type="text/javascript" src="/tests/dom/tests/mochitest/ajax/offline/offlineTests.js"></script>
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+
+<script class="testbody" type="text/javascript">
+
+var gCurrentManifestVersion = 1;
+
+function manifestCached()
+{
+  OfflineTest.checkCache(
+    "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/dynamicRedirect.sjs", false);
+  OfflineTest.checkCache(
+    "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/explicitRedirect.sjs", true);
+  OfflineTest.is(gCurrentManifestVersion, 1, "Cached event for manifest version one");
+
+  // Now add one dynamic entry (now with content overriden redirect sjs)
+  applicationCache.mozAdd(
+    "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/dynamicRedirect.sjs");
+
+  // Wait for the dynamic entry be added to the cache...
+  OfflineTest.waitForAdd(
+    "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/dynamicRedirect.sjs",
+    function() {
+      // ...check it is there...
+      OfflineTest.checkCache(
+        "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/dynamicRedirect.sjs", true);
+
+      // ...revert state of the dynamic entry on the server, now we get the redirect...
+      OfflineTest.setSJSState(
+        "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/dynamicRedirect.sjs",
+        "");
+
+      // ...update manifest to the new version on the server...
+      OfflineTest.setSJSState(
+        "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/redirects.sjs",
+        "second");
+      gCurrentManifestVersion = 2;
+
+      // ...and finally invoke the cache update.
+      applicationCache.update();
+    });
+}
+
+function manifestUpdated()
+{
+  switch (gCurrentManifestVersion)
+  {
+    case 2:
+      // Check the dynamic entry was removed from the cache (because of the redirect)...
+      OfflineTest.checkCache(
+        "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/dynamicRedirect.sjs", false);
+
+      // ...return back redirect for the explicit entry...
+      OfflineTest.setSJSState(
+        "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/explicitRedirect.sjs",
+        "");
+
+      // ...update the manifest to the third version...
+      OfflineTest.setSJSState(
+        "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/redirects.sjs",
+        "third");
+      gCurrentManifestVersion = 3;
+
+      // ...and invoke the cache update, now we must get error.
+      applicationCache.update();
+      break;
+
+    case 3:
+      OfflineTest.ok(false, "Update didn't fail for third version of the manifest");
+      finish();
+      break;
+  }
+}
+
+function manifestError()
+{
+  switch (gCurrentManifestVersion)
+  {
+    case 1:
+      OfflineTest.ok(false, "Error not expected when caching the first version of the manifest");
+      finish();
+      break;
+    case 2:
+      OfflineTest.ok(false, "Error not expected when updating to second version of the manifest");
+      finish();
+      break;
+    case 3:
+      OfflineTest.ok(true, "Error expected when updating to third version of the manifest");
+      finish();
+      break;
+  }
+}
+
+function finish()
+{
+  OfflineTest.teardown();
+  OfflineTest.finish();
+}
+
+SimpleTest.waitForExplicitFinish();
+
+if (OfflineTest.setup()) {
+  applicationCache.onerror = OfflineTest.priv(manifestError);
+  applicationCache.onupdateready = OfflineTest.priv(manifestUpdated);
+  applicationCache.oncached = OfflineTest.priv(manifestCached);
+
+  // Override sjs redirects on the server, it will now return 200 OK and the content
+  OfflineTest.setSJSState(
+      "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/explicitRedirect.sjs",
+      "on");
+  OfflineTest.setSJSState(
+      "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/dynamicRedirect.sjs",
+      "on");
+}
+
+</script>
+
+</head>
+
+<body>
+</body>
+</html>
--- a/uriloader/prefetch/nsOfflineCacheUpdate.cpp
+++ b/uriloader/prefetch/nsOfflineCacheUpdate.cpp
@@ -489,25 +489,32 @@ nsOfflineCacheUpdateItem::GetInterface(c
 // nsOfflineCacheUpdateItem::nsIChannelEventSink
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 nsOfflineCacheUpdateItem::OnChannelRedirect(nsIChannel *aOldChannel,
                                             nsIChannel *aNewChannel,
                                             PRUint32 aFlags)
 {
+    if (!(aFlags & nsIChannelEventSink::REDIRECT_INTERNAL)) {
+        // Don't allow redirect in case of non-internal redirect and cancel
+        // the channel to clean the cache entry.
+        aOldChannel->Cancel(NS_ERROR_ABORT);
+        return NS_ERROR_ABORT;
+    }
+
     nsCOMPtr<nsIURI> newURI;
     nsresult rv = aNewChannel->GetURI(getter_AddRefs(newURI));
     if (NS_FAILED(rv))
         return rv;
 
     nsCOMPtr<nsICachingChannel> oldCachingChannel =
         do_QueryInterface(aOldChannel);
     nsCOMPtr<nsICachingChannel> newCachingChannel =
-      do_QueryInterface(aOldChannel);
+        do_QueryInterface(aNewChannel);
     if (newCachingChannel) {
         rv = newCachingChannel->SetCacheForOfflineUse(PR_TRUE);
         NS_ENSURE_SUCCESS(rv, rv);
         if (!mClientID.IsEmpty()) {
             rv = newCachingChannel->SetOfflineCacheClientID(mClientID);
             NS_ENSURE_SUCCESS(rv, rv);
         }
     }
@@ -576,40 +583,67 @@ nsOfflineCacheUpdateItem::GetLoadedSize(
 
 NS_IMETHODIMP
 nsOfflineCacheUpdateItem::GetReadyState(PRUint16 *aReadyState)
 {
     *aReadyState = mState;
     return NS_OK;
 }
 
+nsresult
+nsOfflineCacheUpdateItem::GetRequestSucceeded(PRBool * succeeded)
+{
+    *succeeded = PR_FALSE;
+
+    if (!mChannel)
+        return NS_OK;
+
+    nsresult rv;
+    nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(mChannel, &rv);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    PRBool reqSucceeded;
+    rv = httpChannel->GetRequestSucceeded(&reqSucceeded);
+    if (NS_ERROR_NOT_AVAILABLE == rv)
+        return NS_OK;
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    if (!reqSucceeded) {
+        LOG(("Request failed"));
+        return NS_OK;
+    }
+
+    nsresult channelStatus;
+    rv = httpChannel->GetStatus(&channelStatus);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    if (NS_FAILED(channelStatus)) {
+        LOG(("Channel status=0x%08x", channelStatus));
+        return NS_OK;
+    }
+
+    *succeeded = PR_TRUE;
+    return NS_OK;
+}
+
 NS_IMETHODIMP
 nsOfflineCacheUpdateItem::GetStatus(PRUint16 *aStatus)
 {
     if (!mChannel) {
         *aStatus = 0;
         return NS_OK;
     }
 
     nsresult rv;
     nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(mChannel, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
 
     PRUint32 httpStatus;
     rv = httpChannel->GetResponseStatus(&httpStatus);
     if (rv == NS_ERROR_NOT_AVAILABLE) {
-        // Someone's calling this before we got a response... Check our
-        // ReadyState.  If we're at RECEIVING or LOADED, then this means the
-        // connection errored before we got any data; return a somewhat
-        // sensible error code in that case.
-        if (mState >= nsIDOMLoadStatus::RECEIVING) {
-            *aStatus = NS_ERROR_NOT_AVAILABLE;
-            return NS_OK;
-        }
-
         *aStatus = 0;
         return NS_OK;
     }
 
     NS_ENSURE_SUCCESS(rv, rv);
     *aStatus = PRUint16(httpStatus);
     return NS_OK;
 }
@@ -1236,21 +1270,21 @@ nsOfflineCacheUpdate::InitPartial(nsIURI
 }
 
 nsresult
 nsOfflineCacheUpdate::HandleManifest(PRBool *aDoUpdate)
 {
     // Be pessimistic
     *aDoUpdate = PR_FALSE;
 
-    PRUint16 status;
-    nsresult rv = mManifestItem->GetStatus(&status);
+    PRBool succeeded;
+    nsresult rv = mManifestItem->GetRequestSucceeded(&succeeded);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    if (status == 0 || status >= 400 || !mManifestItem->ParseSucceeded()) {
+    if (!succeeded || !mManifestItem->ParseSucceeded()) {
         return NS_ERROR_FAILURE;
     }
 
     if (!mManifestItem->NeedsUpdate()) {
         return NS_OK;
     }
 
     // Add items requested by the manifest.
@@ -1370,22 +1404,22 @@ nsOfflineCacheUpdate::LoadCompleted()
         return;
     }
 
     // Normal load finished.
 
     nsRefPtr<nsOfflineCacheUpdateItem> item = mItems[mCurrentItem];
     mCurrentItem++;
 
-    PRUint16 status;
-    rv = item->GetStatus(&status);
-
-    // Check for failures.  4XX and 5XX errors on items explicitly
+    PRBool succeeded;
+    rv = item->GetRequestSucceeded(&succeeded);
+
+    // Check for failures.  3XX, 4XX and 5XX errors on items explicitly
     // listed in the manifest will cause the update to fail.
-    if (NS_FAILED(rv) || status == 0 || status >= 400) {
+    if (NS_FAILED(rv) || !succeeded) {
         if (item->mItemType &
             (nsIApplicationCache::ITEM_EXPLICIT |
              nsIApplicationCache::ITEM_FALLBACK)) {
             mSucceeded = PR_FALSE;
         }
     } else {
         rv = mApplicationCache->MarkEntry(item->mCacheKey, item->mItemType);
         if (NS_FAILED(rv)) {
--- a/uriloader/prefetch/nsOfflineCacheUpdate.h
+++ b/uriloader/prefetch/nsOfflineCacheUpdate.h
@@ -97,16 +97,17 @@ public:
     nsCOMPtr<nsIURI>           mReferrerURI;
     nsCOMPtr<nsIApplicationCache> mPreviousApplicationCache;
     nsCString                  mClientID;
     nsCString                  mCacheKey;
     PRUint32                   mItemType;
 
     nsresult OpenChannel();
     nsresult Cancel();
+    nsresult GetRequestSucceeded(PRBool * succeeded);
 
 private:
     nsOfflineCacheUpdate*          mUpdate;
     nsCOMPtr<nsIChannel>           mChannel;
     PRUint16                       mState;
 
 protected:
     PRInt32                        mBytesRead;