Bug 1240238 - Full DOMStorage schema 1 scope update uniqueness, r=smaug

dom/storage/DOMStorageDBThread.cpp

@@ -65,6 +65,34 @@ Scheme0Scope(DOMStorageCacheBridge* aCache)
     result.Append(':');
   }
 
+  // If there is more than just appid and/or inbrowser stored in origin
+  // attributes, put it to the schema 0 scope as well.  We must do that
+  // to keep the scope column unique (same resolution as schema 1 has
+  // with originAttributes and originKey columns) so that switch between
+  // schema 1 and 0 always works in both ways.
+  nsAutoCString remaining;
+  oa.mAppId = 0;
+  oa.mInBrowser = false;
+  oa.CreateSuffix(remaining);
+  if (!remaining.IsEmpty()) {
+    MOZ_ASSERT(!suffix.IsEmpty());
+
+    if (result.IsEmpty()) {
+      // Must contain the old prefix, otherwise we won't search for the whole
+      // origin attributes suffix.
+      result.Append(NS_LITERAL_CSTRING("0:f:"));
+    }
+    // Append the whole origin attributes suffix despite we have already stored
+    // appid and inbrowser.  We are only looking for it when the scope string
+    // starts with "$appid:$inbrowser:" (with whatever valid values).
+    //
+    // The OriginAttributes suffix is a string in a form like:
+    // "^addonId=101&userContextId=5" and it's ensured it always starts with '^'
+    // and never contains ':'.  See OriginAttributes::CreateSuffix.
+    result.Append(suffix);
+    result.Append(':');
+  }
+
   result.Append(aCache->OriginNoSuffix());
 
   return result;
@@ -442,10 +470,8 @@ DOMStorageDBThread::OpenDatabaseConnection()
 }
 
 nsresult
-DOMStorageDBThread::InitDatabase()
+DOMStorageDBThread::OpenAndUpdateDatabase()
 {
-  Telemetry::AutoTimer<Telemetry::LOCALDOMSTORAGE_INIT_DATABASE_MS> timer;
-
   nsresult rv;
 
   // Here we are on the worker thread. This opens the worker connection.
@@ -457,13 +483,41 @@ DOMStorageDBThread::InitDatabase()
   rv = TryJournalMode();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  return NS_OK;
+}
+
+nsresult
+DOMStorageDBThread::InitDatabase()
+{
+  Telemetry::AutoTimer<Telemetry::LOCALDOMSTORAGE_INIT_DATABASE_MS> timer;
+
+  nsresult rv;
+
+  // Here we are on the worker thread. This opens the worker connection.
+  MOZ_ASSERT(!NS_IsMainThread());
+
+  rv = OpenAndUpdateDatabase();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = DOMStorageDBUpdater::Update(mWorkerConnection);
+  if (NS_FAILED(rv)) {
+    // Update has failed, rather throw the database away and try
+    // opening and setting it up again.
+    rv = mWorkerConnection->Close();
+    mWorkerConnection = nullptr;
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = mDatabaseFile->Remove(false);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = OpenAndUpdateDatabase();
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
   // Create a read-only clone
   (void)mWorkerConnection->Clone(true, getter_AddRefs(mReaderConnection));
   NS_ENSURE_TRUE(mReaderConnection, NS_ERROR_FAILURE);
 
-  rv = DOMStorageDBUpdater::Update(mWorkerConnection);
-  NS_ENSURE_SUCCESS(rv, rv);
-
   // Database open and all initiation operation are done.  Switching this flag
   // to true allow main thread to read directly from the database.
   // If we would allow this sooner, we would have opened a window where main thread

dom/storage/DOMStorageDBThread.h

@@ -360,6 +360,7 @@ private:
 
   // Opens the database, first thing we do after start of the thread.
   nsresult OpenDatabaseConnection();
+  nsresult OpenAndUpdateDatabase();
   nsresult InitDatabase();
   nsresult ShutdownDatabase();
 

dom/storage/DOMStorageDBUpdater.cpp

@@ -104,8 +104,38 @@ public:
     // OK, we have found appId and inBrowser flag, create the suffix from it
     // and take the rest as the origin key.
 
-    PrincipalOriginAttributes attrs(appId, inBrowser);
-    attrs.CreateSuffix(suffix);
+    // If the profile went through schema 1 -> schema 0 -> schema 1 switching
+    // we may have stored the full attributes origin suffix when there were
+    // more than just appid and inbrowser set on storage principal's
+    // OriginAttributes.
+    //
+    // To preserve full uniqueness we store this suffix to the scope key.
+    // Schema 0 code will just ignore it while keeping the scoping unique.
+    //
+    // The whole scope string is in one of the following forms (when we are here):
+    //
+    // "1001:f:^appId=1001&inBrowser=false&addonId=101:gro.allizom.rxd.:https:443"
+    // "1001:f:gro.allizom.rxd.:https:443"
+    //         |
+    //         +- the parser cursor position.
+    //
+    // If there is '^', the full origin attributes suffix follows.  We search
+    // for ':' since it is the delimiter used in the scope string and is never
+    // contained in the origin attributes suffix.  Remaning string after
+    // the comma is the reversed-domain+schema+port tuple.
+    Record();
+    if (CheckChar('^')) {
+      Token t;
+      while (Next(t)) {
+        if (t.Equals(Token::Char(':'))) {
+          Claim(suffix);
+          break;
+        }
+      }
+    } else {
+      PrincipalOriginAttributes attrs(appId, inBrowser);
+      attrs.CreateSuffix(suffix);
+    }
 
     // Consume the rest of the input as "origin".
     origin.Assign(Substring(mCursor, mEnd));
@@ -345,6 +375,11 @@ nsresult Update(mozIStorageConnection *aWorkerConnection)
     MOZ_FALLTHROUGH;
   }
   case CURRENT_SCHEMA_VERSION:
+    // Ensure the tables and indexes are up.  This is mostly a no-op
+    // in common scenarios.
+    rv = CreateSchema1Tables(aWorkerConnection);
+    NS_ENSURE_SUCCESS(rv, rv);
+
     // Nothing more to do here, this is the current schema version
     break;