Bug 1143959 - Set the journal mode and foreign key pragmas for all DBActions; r=bkelly
authorEhsan Akhgari <ehsan@mozilla.com>
Mon, 16 Mar 2015 20:27:23 -0400
changeset 250207 08ad099d72abaaafc4ca95205a4e624ae5048478
parent 250206 3174053752fd3fbc07266b3bcd3d774d6e41bf4d
child 250208 57e5f142614844ed6f737898e2be7bd28ac7a9a9
push idunknown
push userunknown
push dateunknown
reviewersbkelly
bugs1143959
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 1143959 - Set the journal mode and foreign key pragmas for all DBActions; r=bkelly Before this patch, we would only set these pragmas as part of CreateSchema which runs in SetupAction. This meant that the connection used to perform other DBActions would not have had these pragmas applied. As a result, sqlite would not honor foreign keys on such connections, so the cascade delete rules responsible for deleting rows from request_headers and response_headers would not get executed when DBSchema::CachePut deleted the old entry before adding a new one. The test in the patch demonstrates how this could result in an observable breakage. Before this patch, the response headers stored in the cache for the overwritten entry would reflect both `Mirrored: `foo' and `Mirrored: bar' headers, which means that attempting to get this header on the cached response would return the first entry, `foo'.
dom/cache/DBAction.cpp
dom/cache/DBSchema.cpp
dom/cache/DBSchema.h
dom/cache/test/mochitest/mirror.sjs
dom/cache/test/mochitest/mochitest.ini
dom/cache/test/mochitest/test_cache_overwrite.html
dom/cache/test/mochitest/test_cache_overwrite.js
--- a/dom/cache/DBAction.cpp
+++ b/dom/cache/DBAction.cpp
@@ -149,16 +149,19 @@ DBAction::OpenConnection(const QuotaInfo
   if (schemaVersion > 0 && schemaVersion < DBSchema::kMaxWipeSchemaVersion) {
     conn = nullptr;
     rv = WipeDatabase(dbFile, aDBDir);
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
     rv = ss->OpenDatabaseWithFileURL(dbFileUrl, getter_AddRefs(conn));
   }
 
+  rv = DBSchema::InitializeConnection(conn);
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
   conn.forget(aConnOut);
 
   return rv;
 }
 
 nsresult
 DBAction::WipeDatabase(nsIFile* aDBFile, nsIFile* aDBDir)
 {
--- a/dom/cache/DBSchema.cpp
+++ b/dom/cache/DBSchema.cpp
@@ -29,31 +29,19 @@ using mozilla::void_t;
 // static
 nsresult
 DBSchema::CreateSchema(mozIStorageConnection* aConn)
 {
   MOZ_ASSERT(!NS_IsMainThread());
   MOZ_ASSERT(aConn);
 
   nsAutoCString pragmas(
-#if defined(MOZ_WIDGET_ANDROID) || defined(MOZ_WIDGET_GONK)
-    // Switch the journaling mode to TRUNCATE to avoid changing the directory
-    // structure at the conclusion of every transaction for devices with slower
-    // file systems.
-    "PRAGMA journal_mode = TRUNCATE; "
-#endif
-    "PRAGMA foreign_keys = ON; "
     // Enable auto-vaccum but in incremental mode in order to avoid doing a lot
     // of work at the end of each transaction.
     "PRAGMA auto_vacuum = INCREMENTAL; "
-
-    // Note, the default encoding of UTF-8 is preferred.  mozStorage does all
-    // the work necessary to convert UTF-16 nsString values for us.  We don't
-    // need ordering and the binary equality operations are correct.  So, do
-    // NOT set PRAGMA encoding to UTF-16.
   );
 
   nsresult rv = aConn->ExecuteSimpleSQL(pragmas);
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
   int32_t schemaVersion;
   rv = aConn->GetSchemaVersion(&schemaVersion);
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
@@ -172,16 +160,47 @@ DBSchema::CreateSchema(mozIStorageConnec
     return NS_ERROR_FAILURE;
   }
 
   return rv;
 }
 
 // static
 nsresult
+DBSchema::InitializeConnection(mozIStorageConnection* aConn)
+{
+  MOZ_ASSERT(!NS_IsMainThread());
+  MOZ_ASSERT(aConn);
+
+  // This function needs to perform per-connection initialization tasks that
+  // need to happen regardless of the schema.
+
+  nsAutoCString pragmas(
+#if defined(MOZ_WIDGET_ANDROID) || defined(MOZ_WIDGET_GONK)
+    // Switch the journaling mode to TRUNCATE to avoid changing the directory
+    // structure at the conclusion of every transaction for devices with slower
+    // file systems.
+    "PRAGMA journal_mode = TRUNCATE; "
+#endif
+    "PRAGMA foreign_keys = ON; "
+
+    // Note, the default encoding of UTF-8 is preferred.  mozStorage does all
+    // the work necessary to convert UTF-16 nsString values for us.  We don't
+    // need ordering and the binary equality operations are correct.  So, do
+    // NOT set PRAGMA encoding to UTF-16.
+  );
+
+  nsresult rv = aConn->ExecuteSimpleSQL(pragmas);
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  return NS_OK;
+}
+
+// static
+nsresult
 DBSchema::CreateCache(mozIStorageConnection* aConn, CacheId* aCacheIdOut)
 {
   MOZ_ASSERT(!NS_IsMainThread());
   MOZ_ASSERT(aConn);
   MOZ_ASSERT(aCacheIdOut);
 
   nsresult rv = aConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
     "INSERT INTO caches DEFAULT VALUES;"
--- a/dom/cache/DBSchema.h
+++ b/dom/cache/DBSchema.h
@@ -28,16 +28,17 @@ class PCacheResponse;
 struct SavedRequest;
 struct SavedResponse;
 
 // TODO: remove static class and use functions in cache namespace (bug 1110485)
 class DBSchema MOZ_FINAL
 {
 public:
   static nsresult CreateSchema(mozIStorageConnection* aConn);
+  static nsresult InitializeConnection(mozIStorageConnection* aConn);
 
   static nsresult CreateCache(mozIStorageConnection* aConn,
                               CacheId* aCacheIdOut);
   // TODO: improve naming (confusing with CacheDelete) (bug 1110485)
   static nsresult DeleteCache(mozIStorageConnection* aConn, CacheId aCacheId,
                               nsTArray<nsID>& aDeletedBodyIdListOut);
 
   // TODO: Consider removing unused IsCacheOrphaned after writing cleanup code. (bug 1110446)
new file mode 100644
--- /dev/null
+++ b/dom/cache/test/mochitest/mirror.sjs
@@ -0,0 +1,5 @@
+function handleRequest(request, response) {
+  response.setStatusLine(request.httpVersion, 200, "OK");
+  response.setHeader("Mirrored", request.getHeader("Mirror"));
+  response.write(request.getHeader("Mirror"));
+}
--- a/dom/cache/test/mochitest/mochitest.ini
+++ b/dom/cache/test/mochitest/mochitest.ini
@@ -5,13 +5,16 @@ support-files =
   worker_driver.js
   worker_wrapper.js
   frame.html
   message_receiver.html
   driver.js
   serviceworker_driver.js
   test_cache_match_request.js
   test_cache_matchAll_request.js
+  test_cache_overwrite.js
+  mirror.sjs
 
 [test_cache.html]
 [test_cache_add.html]
 [test_cache_match_request.html]
 [test_cache_matchAll_request.html]
+[test_cache_overwrite.html]
new file mode 100644
--- /dev/null
+++ b/dom/cache/test/mochitest/test_cache_overwrite.html
@@ -0,0 +1,20 @@
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test what happens when you overwrite a cache entry</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <script type="text/javascript" src="driver.js"></script>
+</head>
+<body>
+<iframe id="frame"></iframe>
+<script class="testbody" type="text/javascript">
+  runTests("test_cache_overwrite.js")
+    .then(function() {
+      SimpleTest.finish();
+    });
+</script>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/dom/cache/test/mochitest/test_cache_overwrite.js
@@ -0,0 +1,44 @@
+var requestURL = "//mochi.test:8888/tests/dom/cache/test/mochitest/mirror.sjs?" + context;
+var response;
+var c;
+var responseText;
+var name = "match-mirror" + context;
+
+function checkResponse(r) {
+  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");
+  is(r.headers.get("Mirrored"), response.headers.get("Mirrored"),
+     "Both responses should have the same Mirrored header");
+  return r.text().then(function(text) {
+    is(text, responseText, "The response body should be correct");
+  });
+}
+
+fetch(new Request(requestURL, {headers: {"Mirror": "bar"}})).then(function(r) {
+  is(r.headers.get("Mirrored"), "bar", "The server should give back the correct header");
+  response = r;
+  return response.text();
+}).then(function(text) {
+  responseText = text;
+  return caches.open(name);
+}).then(function(cache) {
+  c = cache;
+  return c.add(new Request(requestURL, {headers: {"Mirror": "foo"}}));
+}).then(function() {
+  // Overwrite the request, to replace the entry stored in response_headers
+  // with a different value.
+  return c.add(new Request(requestURL, {headers: {"Mirror": "bar"}}));
+}).then(function() {
+  return c.matchAll();
+}).then(function(r) {
+  is(r.length, 1, "Only one request should be in the cache");
+  return checkResponse(r[0]);
+}).then(function() {
+  testDone();
+});