Bug 1542888 - avoid Rkv::Manager call to std::fs::canonicalize r=lina

Avoid using Rkv::Manager, which calls std::fs::canonicalize, triggering bug 1531887 in Firefox on Android.

Differential Revision: https://phabricator.services.mozilla.com/D26605

--HG--
extra : moz-landing-system : lando

Cargo.lock

@@ -1493,6 +1493,7 @@ dependencies = [
  "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "crossbeam-utils 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "failure 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.51 (registry+https://github.com/rust-lang/crates.io-index)",
  "lmdb-rkv 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",

toolkit/components/kvstore/Cargo.toml

@@ -6,6 +6,7 @@ authors = ["Myk Melez <myk@mykzilla.org>"]
 [dependencies]
 atomic_refcell = "0.1"
 crossbeam-utils = "0.6.3"
+lazy_static = "1"
 libc = "0.2"
 lmdb-rkv = "0.11.2"
 log = "0.4"

toolkit/components/kvstore/src/lib.rs

@@ -6,6 +6,8 @@ extern crate atomic_refcell;
 extern crate crossbeam_utils;
 #[macro_use]
 extern crate failure;
+#[macro_use]
+extern crate lazy_static;
 extern crate libc;
 extern crate lmdb;
 extern crate log;
@@ -18,6 +20,7 @@ extern crate thin_vec;
 extern crate xpcom;
 
 mod error;
+mod manager;
 mod owned_value;
 mod task;
 

toolkit/components/kvstore/src/manager.rs

@@ -0,0 +1,61 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+//! A manager of handles to Rkv environments that ensures that multiple calls
+//! to `nsIKeyValueService.get_or_create()` with the same path reuse the same
+//! environment handle.
+//!
+//! The Rkv crate itself provides this functionality, but it calls
+//! `Path.canonicalize()` on the path, which crashes in Firefox on Android
+//! because of https://bugzilla.mozilla.org/show_bug.cgi?id=1531887.
+//!
+//! Since kvstore consumers use canonical paths in practice, this manager
+//! works around that bug by not doing so itself.
+
+use rkv::{Rkv, StoreError};
+use std::collections::HashMap;
+use std::collections::hash_map::Entry;
+use std::path::{
+    Path,
+    PathBuf,
+};
+use std::sync::{
+    Arc,
+    RwLock,
+};
+
+lazy_static! {
+    static ref MANAGER: RwLock<Manager> = RwLock::new(Manager::new());
+}
+
+pub(crate) struct Manager {
+    environments: HashMap<PathBuf, Arc<RwLock<Rkv>>>,
+}
+
+impl Manager {
+    fn new() -> Manager {
+        Manager {
+            environments: Default::default(),
+        }
+    }
+
+    pub(crate) fn singleton() -> &'static RwLock<Manager> {
+        &*MANAGER
+    }
+
+    /// Return the open env at `path`, or create it by calling `f`.
+    pub fn get_or_create<'p, F, P>(&mut self, path: P, f: F) -> Result<Arc<RwLock<Rkv>>, StoreError>
+    where
+        F: FnOnce(&Path) -> Result<Rkv, StoreError>,
+        P: Into<&'p Path>,
+    {
+        Ok(match self.environments.entry(path.into().to_path_buf()) {
+            Entry::Occupied(e) => e.get().clone(),
+            Entry::Vacant(e) => {
+                let k = Arc::new(RwLock::new(f(e.key().as_path())?));
+                e.insert(k).clone()
+            },
+        })
+    }
+}

toolkit/components/kvstore/src/task.rs

@@ -10,7 +10,7 @@ use moz_task::Task;
 use nserror::{nsresult, NS_ERROR_FAILURE};
 use nsstring::nsCString;
 use owned_value::owned_to_variant;
-use rkv::{Manager, OwnedValue, Rkv, SingleStore, StoreError, StoreOptions, Value};
+use rkv::{OwnedValue, Rkv, SingleStore, StoreError, StoreOptions, Value};
 use std::{
     path::Path,
     str,
@@ -27,6 +27,7 @@ use xpcom::{
 use KeyValueDatabase;
 use KeyValueEnumerator;
 use KeyValuePairResult;
+use manager::Manager;
 
 /// A macro to generate a done() implementation for a Task.
 /// Takes one argument that specifies the type of the Task's callback function:

toolkit/components/kvstore/test/xpcshell/test_kvstore.js

@@ -28,6 +28,15 @@ add_task(async function getOrCreate() {
   const databaseDir = await makeDatabaseDir("getOrCreate");
   const database = await KeyValueService.getOrCreate(databaseDir, "db");
   Assert.ok(database);
+
+  // Test creating a database with a nonexistent path.
+  const nonexistentDir = OS.Path.join(OS.Constants.Path.profileDir, "nonexistent");
+  await Assert.rejects(KeyValueService.getOrCreate(nonexistentDir, "db"), /DirectoryDoesNotExistError/);
+
+  // Test creating a database with a non-normalized but fully-qualified path.
+  let nonNormalizedDir = await makeDatabaseDir("non-normalized");
+  nonNormalizedDir = OS.Path.join(nonNormalizedDir, "..", ".", "non-normalized");
+  Assert.ok(await KeyValueService.getOrCreate(nonNormalizedDir, "db"));
 });
 
 add_task(async function putGetHasDelete() {