Bug 1626167 - Several improvements in address book manager. r=mkmelin
authorGeoff Lankow <geoff@darktrojan.net>
Thu, 02 Apr 2020 20:41:58 +1300
changeset 38689 f30adb6e9f15523a628bc773574a7f35ef5abfcd
parent 38688 fa5c346560d7e9b1ed341e9e2bf644ebde887889
child 38690 2232bbc484164c7b42f432b13eadee4989c8ee34
push id400
push userclokep@gmail.com
push dateMon, 04 May 2020 18:56:09 +0000
reviewersmkmelin
bugs1626167
Bug 1626167 - Several improvements in address book manager. r=mkmelin * Reset mail.collect_addressbook pref when the address book it points to is deleted * Prevent the removal of built-in address books * Throw more useful exceptions in several places
mailnews/addrbook/jsaddrbook/AddrBookManager.jsm
mailnews/addrbook/test/unit/test_delete_book.js
mailnews/addrbook/test/unit/xpcshell.ini
--- a/mailnews/addrbook/jsaddrbook/AddrBookManager.jsm
+++ b/mailnews/addrbook/jsaddrbook/AddrBookManager.jsm
@@ -69,17 +69,17 @@ if (AppConstants.platform == "macosx") {
  * @param {string} uri - URI for the directory.
  * @param {boolean} shouldStore - Whether to keep a reference to this address
  *   book in the store.
  * @returns {nsIAbDirectory}
  */
 function createDirectoryObject(uri, shouldStore = false) {
   let uriParts = URI_REGEXP.exec(uri);
   if (!uriParts) {
-    throw Cr.NS_ERROR_UNEXPECTED;
+    throw Cr.NS_ERROR_MALFORMED_URI;
   }
 
   let [, scheme] = uriParts;
   let dir = Cc[
     `@mozilla.org/addressbook/directory;1?type=${scheme}`
   ].createInstance(Ci.nsIAbDirectory);
   if (shouldStore) {
     store.set(uri, dir);
@@ -173,27 +173,30 @@ AddrBookManager.prototype = {
         return aPosition - bPosition;
       }
       return a.URI < b.URI ? -1 : 1;
     });
     return new SimpleEnumerator(dirs);
   },
   getDirectory(uri) {
     if (uri.startsWith("moz-abdirectory://")) {
-      throw Cr.NS_ERROR_FAILURE;
+      throw new Components.Exception(
+        "The root address book no longer exists",
+        Cr.NS_ERROR_FAILURE
+      );
     }
 
     ensureInitialized();
     if (store.has(uri)) {
       return store.get(uri);
     }
 
     let uriParts = URI_REGEXP.exec(uri);
     if (!uriParts) {
-      throw Cr.NS_ERROR_UNEXPECTED;
+      throw Cr.NS_ERROR_MALFORMED_URI;
     }
     let [, scheme, , tail] = uriParts;
     if (tail && types.includes(scheme)) {
       // `tail` could either point to a mailing list or a query.
       // Both of these will be handled differently in future.
       return createDirectoryObject(uri);
     }
     throw Cr.NS_ERROR_FAILURE;
@@ -218,17 +221,20 @@ AddrBookManager.prototype = {
       let uniqueCount = 0;
       prefName = `ldap_2.servers.${leafName}`;
       while (existingNames.includes(prefName)) {
         prefName = `ldap_2.servers.${leafName}_${++uniqueCount}`;
       }
     }
 
     if (!dirName) {
-      throw Cr.NS_ERROR_UNEXPECTED;
+      throw new Components.Exception(
+        "dirName must be specified",
+        Cr.NS_ERROR_INVALID_ARG
+      );
     }
 
     ensureInitialized();
 
     switch (type) {
       case LDAP_DIRECTORY_TYPE: {
         let file = Services.dirsvc.get("ProfD", Ci.nsIFile);
         file.append("ldap.sqlite");
@@ -308,54 +314,70 @@ AddrBookManager.prototype = {
         throw Cr.NS_ERROR_UNEXPECTED;
     }
 
     return prefName;
   },
   deleteAddressBook(uri) {
     let uriParts = URI_REGEXP.exec(uri);
     if (!uriParts) {
-      throw Cr.NS_ERROR_UNEXPECTED;
+      throw Cr.NS_ERROR_MALFORMED_URI;
     }
 
     let [, scheme, fileName, tail] = uriParts;
     if (tail) {
       if (tail.startsWith("/MailList")) {
         let dir = store.get(`${scheme}://${fileName}`);
         let list = this.getDirectory(uri);
         if (dir && list) {
           dir.deleteDirectory(list);
           return;
         }
       }
     }
 
     let dir = store.get(uri);
     if (!dir) {
-      return;
+      throw new Components.Exception(
+        `Address book not found: ${uri}`,
+        Cr.NS_ERROR_UNEXPECTED
+      );
     }
 
     let prefName = dir.dirPrefId;
     fileName = dir.fileName;
 
+    // Deleting the built-in address books is very bad.
+    if (["ldap_2.servers.pab", "ldap_2.servers.history"].includes(prefName)) {
+      throw new Components.Exception(
+        "Refusing to delete a built-in address book",
+        Cr.NS_ERROR_FAILURE
+      );
+    }
+
     Services.prefs.clearUserPref(`${prefName}.description`);
     if (
       Services.prefs.getIntPref(`${prefName}.dirType`, 0) == MAPI_DIRECTORY_TYPE
     ) {
       // The prefs for this directory type are defaults. Setting the dirType
       // to -1 ensures the directory is ignored.
       Services.prefs.setIntPref(`${prefName}.dirType`, -1);
     } else {
       Services.prefs.clearUserPref(`${prefName}.dirType`);
     }
     Services.prefs.clearUserPref(`${prefName}.filename`);
     Services.prefs.clearUserPref(`${prefName}.uid`);
     Services.prefs.clearUserPref(`${prefName}.uri`);
     store.delete(uri);
 
+    // Clear this reference to the deleted address book.
+    if (Services.prefs.getStringPref("mail.collect_addressbook") == uri) {
+      Services.prefs.clearUserPref("mail.collect_addressbook");
+    }
+
     if (fileName) {
       let file = Services.dirsvc.get("ProfD", Ci.nsIFile);
       file.append(fileName);
       closeConnectionTo(file).then(() => {
         if (file.exists()) {
           file.remove(false);
         }
         this.notifyDirectoryDeleted(null, dir);
new file mode 100644
--- /dev/null
+++ b/mailnews/addrbook/test/unit/test_delete_book.js
@@ -0,0 +1,95 @@
+/* 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 http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+var { MailServices } = ChromeUtils.import(
+  "resource:///modules/MailServices.jsm"
+);
+var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
+
+function getExistingDirectories() {
+  return Array.from(MailServices.ab.directories, d => d.dirPrefId);
+}
+
+function promiseDirectoryRemoved() {
+  return new Promise(resolve => {
+    let observer = {
+      onItemRemoved() {
+        MailServices.ab.removeAddressBookListener(this);
+        resolve();
+      },
+    };
+    MailServices.ab.addAddressBookListener(
+      observer,
+      Ci.nsIAbListener.directoryRemoved
+    );
+  });
+}
+
+add_task(async function clearPref() {
+  Assert.deepEqual(getExistingDirectories(), [
+    "ldap_2.servers.pab",
+    "ldap_2.servers.history",
+  ]);
+  equal(
+    Services.prefs.getStringPref("mail.collect_addressbook"),
+    "jsaddrbook://history.sqlite"
+  );
+
+  let dirPrefId = MailServices.ab.newAddressBook("delete me", "", 101);
+  let book = MailServices.ab.getDirectoryFromId(dirPrefId);
+
+  Assert.deepEqual(getExistingDirectories(), [
+    "ldap_2.servers.deleteme",
+    "ldap_2.servers.pab",
+    "ldap_2.servers.history",
+  ]);
+  Services.prefs.setStringPref("mail.collect_addressbook", book.URI);
+
+  MailServices.ab.deleteAddressBook(book.URI);
+  await promiseDirectoryRemoved();
+
+  Assert.deepEqual(getExistingDirectories(), [
+    "ldap_2.servers.pab",
+    "ldap_2.servers.history",
+  ]);
+  equal(
+    Services.prefs.getStringPref("mail.collect_addressbook"),
+    "jsaddrbook://history.sqlite"
+  );
+});
+
+add_task(async function protectBuiltIns() {
+  Assert.deepEqual(getExistingDirectories(), [
+    "ldap_2.servers.pab",
+    "ldap_2.servers.history",
+  ]);
+  equal(
+    Services.prefs.getStringPref("mail.collect_addressbook"),
+    "jsaddrbook://history.sqlite"
+  );
+
+  Assert.throws(() => {
+    MailServices.ab.deleteAddressBook("this is completely wrong");
+  }, /NS_ERROR_MALFORMED_URI/);
+  Assert.throws(() => {
+    MailServices.ab.deleteAddressBook("jsaddrbook://bad.sqlite");
+  }, /NS_ERROR_UNEXPECTED/);
+  Assert.throws(() => {
+    MailServices.ab.deleteAddressBook("jsaddrbook://history.sqlite");
+  }, /NS_ERROR_FAILURE/);
+  Assert.throws(() => {
+    MailServices.ab.deleteAddressBook("jsaddrbook://abook.sqlite");
+  }, /NS_ERROR_FAILURE/);
+
+  Assert.deepEqual(getExistingDirectories(), [
+    "ldap_2.servers.pab",
+    "ldap_2.servers.history",
+  ]);
+  equal(
+    Services.prefs.getStringPref("mail.collect_addressbook"),
+    "jsaddrbook://history.sqlite"
+  );
+});
--- a/mailnews/addrbook/test/unit/xpcshell.ini
+++ b/mailnews/addrbook/test/unit/xpcshell.ini
@@ -8,16 +8,17 @@ tags = addrbook
 [test_bug387403.js]
 [test_bug448165.js]
 [test_bug534822.js]
 [test_bug1522453.js]
 [test_cardForEmail.js]
 [test_collection.js]
 [test_collection_2.js]
 [test_db_enumerator.js]
+[test_delete_book.js]
 [test_export.js]
 [test_jsaddrbook.js]
 [test_jsaddrbook_inner.js]
 [test_ldap1.js]
 [test_ldap2.js]
 [test_ldapOffline.js]
 [test_ldapReplication.js]
 skip-if = debug # Fails for unknown reasons.