Bug 1532388 - fix the CLIENTID migration for cases of stmp username not set, or having nttp accounts. r=aleca DONTBUILD
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Fri, 24 Jan 2020 12:38:12 +0200
changeset 37126 9cc39105d698c16ac439cca0f936f967c68af899
parent 37125 ec55ec5db960bc99aaaf3cabf3040c9a3e066473
child 37127 2dff5349a09ff6b7ad3c5febf222bf8e9b7eb414
push id2552
push userclokep@gmail.com
push dateMon, 10 Feb 2020 21:24:16 +0000
treeherdercomm-beta@f95a6f4408a3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaleca
bugs1532388
Bug 1532388 - fix the CLIENTID migration for cases of stmp username not set, or having nttp accounts. r=aleca DONTBUILD
mailnews/base/test/unit/test_accountMigration.js
mailnews/base/util/mailnewsMigrator.js
--- a/mailnews/base/test/unit/test_accountMigration.js
+++ b/mailnews/base/test/unit/test_accountMigration.js
@@ -22,28 +22,29 @@ function testPermission(aURI) {
   return Services.perms.testPermissionFromPrincipal(principal, "image");
 }
 
 function run_test() {
   // Set up some basic accounts with limited prefs - enough to satisfy the
   // migrator.
   Services.prefs.setCharPref("mail.account.account1.server", "server1");
   Services.prefs.setCharPref("mail.account.account2.server", "server2");
+  Services.prefs.setCharPref("mail.account.account3.server", "server3");
 
   // Server1 has nothing set.
 
   // Server2 has useSecAuth set to true, auth_login unset
   Services.prefs.setBoolPref("mail.server.server2.useSecAuth", true);
 
   Services.prefs.setCharPref(
     "mail.accountmanager.accounts",
-    "account1,account2"
+    "account1,account2,account3"
   );
 
-  // Set server1 and server2 username and hostname to test Clientid population.
+  // Set server1 and server2 username and hostname to test clientid population.
   Services.prefs.setCharPref("mail.server.server1.userName", "testuser1");
   Services.prefs.setCharPref("mail.server.server2.userName", "testuser2");
   Services.prefs.setCharPref(
     "mail.server.server1.hostname",
     "mail.sampledomain1.com"
   );
   Services.prefs.setCharPref(
     "mail.server.server2.hostname",
@@ -84,20 +85,24 @@ function run_test() {
     Ci.nsIPrefLocalizedString,
     charset
   );
   Assert.ok(Services.prefs.prefHasUserValue("mailnews.view_default_charset"));
 
   // Now migrate the prefs.
   migrateMailnews();
 
-  // Check that server 1 and server 2 have the same clientid.
+  // Check that server1 and server2 have the same clientid.
   Assert.ok(Services.prefs.prefHasUserValue("mail.server.server1.clientid"));
   Assert.ok(Services.prefs.prefHasUserValue("mail.server.server2.clientid"));
 
+  // Check that server3 didn't get a clientid, since it has no userName.
+  // This is the case for nntp.
+  Assert.ok(!Services.prefs.prefHasUserValue("mail.server.server3.clientid"));
+
   // Check what has been set.
   Assert.ok(!Services.prefs.prefHasUserValue("mail.server.server1.authMethod"));
   Assert.ok(Services.prefs.prefHasUserValue("mail.server.server2.authMethod"));
   Assert.equal(
     Services.prefs.getIntPref("mail.server.server2.authMethod"),
     Ci.nsMsgAuthMethod.secure
   );
 
@@ -111,17 +116,17 @@ function run_test() {
   // This time around, both of these should not be set.
   Assert.ok(!Services.prefs.prefHasUserValue("mail.server.server1.authMethod"));
   Assert.ok(!Services.prefs.prefHasUserValue("mail.server.server2.authMethod"));
 
   //
   // Now check SMTP
   //
 
-  Services.prefs.setCharPref("mail.smtpservers", "smtp1,smtp2");
+  Services.prefs.setCharPref("mail.smtpservers", "smtp1,smtp2,smtp3");
 
   // smtp1 has nothing set.
 
   // smtp2 has useSecAuth set to true, auth_method unset
   Services.prefs.setBoolPref("mail.smtpserver.smtp2.useSecAuth", true);
 
   // Set server1 and server2 username and hostname to test clientid population.
   Services.prefs.setCharPref("mail.smtpserver.smtp1.username", "testuser1");
@@ -129,16 +134,20 @@ function run_test() {
   Services.prefs.setCharPref(
     "mail.smtpserver.smtp1.hostname",
     "mail.sampledomain1.com"
   );
   Services.prefs.setCharPref(
     "mail.smtpserver.smtp2.hostname",
     "mail.sampledomain2.com"
   );
+  Services.prefs.setCharPref(
+    "mail.smtpserver.smtp3.hostname",
+    "mail.nousername.example.com"
+  );
 
   // Migration should now have added permissions for the address that had them
   // and not for the one that didn't have them.
   Assert.ok(Services.prefs.getIntPref("mail.ab_remote_content.migrated") > 0);
   Assert.equal(testPermission(uriAllowed), Services.perms.ALLOW_ACTION);
   Assert.equal(testPermission(uriAllowed2), Services.perms.ALLOW_ACTION);
   Assert.equal(testPermission(uriDisallowed), Services.perms.UNKNOWN_ACTION);
 
@@ -149,16 +158,20 @@ function run_test() {
 
   // Now migrate the prefs
   migrateMailnews();
 
   // Check that smtpserver 1 and smtpserver 2 now have a clientid.
   Assert.ok(Services.prefs.prefHasUserValue("mail.smtpserver.smtp1.clientid"));
   Assert.ok(Services.prefs.prefHasUserValue("mail.smtpserver.smtp2.clientid"));
 
+  // Check that the smtp3server 2 got a clientid, even if it has no
+  // username. All SMTP servers don't require a username.
+  Assert.ok(Services.prefs.prefHasUserValue("mail.smtpserver.smtp3.clientid"));
+
   Assert.ok(
     !Services.prefs.prefHasUserValue("mail.smtpserver.smtp1.authMethod")
   );
   Assert.ok(
     Services.prefs.prefHasUserValue("mail.smtpserver.smtp2.authMethod")
   );
   Assert.equal(
     Services.prefs.getIntPref("mail.smtpserver.smtp2.authMethod"),
--- a/mailnews/base/util/mailnewsMigrator.js
+++ b/mailnews/base/util/mailnewsMigrator.js
@@ -52,82 +52,88 @@ function migrateMailnews() {
  * Creates the server specific 'CLIENTID' prefs and tries to pair up any imap
  * services with smtp services which are using the same username and hostname.
  */
 function MigrateProfileClientid() {
   let uuidGen = Cc["@mozilla.org/uuid-generator;1"].getService(
     Ci.nsIUUIDGenerator
   );
   // Comma-separated list of all account ids.
-  let accounts = Services.prefs.getCharPref("mail.accountmanager.accounts");
+  let accounts = Services.prefs.getCharPref("mail.accountmanager.accounts", "");
   // Comma-separated list of all smtp servers.
-  let smtpServers = Services.prefs.getCharPref("mail.smtpservers");
+  let smtpServers = Services.prefs.getCharPref("mail.smtpservers", "");
   // If both accounts and smtpservers are empty then there is nothing to do.
   if (accounts.length == 0 && smtpServers.length == 0) {
     return;
   }
-  // A cache to allow CLIENTIDS to be stored and shared across services that
+  // A cache to allow CLIENTIDs to be stored and shared across services that
   // share a username and hostname.
   let clientidCache = new Map();
   // There may be accounts but no smtpservers so check the length before
   // trying to split the smtp servers and iterate in the loop below.
   if (smtpServers.length > 0) {
-    // Since the length of the smtpServers string is non-zero then we can split
-    // the string by comma and iterate each entry in the comma-separated list.
-    let smtpServerKeys = smtpServers.split(",");
     // Now walk all smtp servers and generate any missing CLIENTIDS, caching
     // all CLIENTIDS along the way to be reused for matching imap servers
     // if possible.
-    for (let smtpServerKey of smtpServerKeys) {
-      let server = "mail.smtpserver." + smtpServerKey + ".";
+
+    // Since the length of the smtpServers string is non-zero then we can split
+    // the string by comma and iterate each entry in the comma-separated list.
+    for (let key of smtpServers.split(",")) {
+      let server = "mail.smtpserver." + key + ".";
       if (
         !Services.prefs.prefHasUserValue(server + "clientid") ||
-        !Services.prefs.getCharPref(server + "clientid")
+        !Services.prefs.getCharPref(server + "clientid", "")
       ) {
         // Always give outgoing servers a new unique CLIENTID.
         let newClientid = uuidGen
           .generateUUID()
           .toString()
           .replace(/[{}]/g, "");
         Services.prefs.setCharPref(server + "clientid", newClientid);
       }
+      let username = Services.prefs.getCharPref(server + "username", "");
+      if (!username) {
+        // Not all SMTP servers require a username.
+        continue;
+      }
+
       // Cache all CLIENTIDs from all outgoing servers to reuse them for any
       // incoming servers which have a matching username and hostname.
-      let username = Services.prefs.getCharPref(server + "username");
       let hostname = Services.prefs.getCharPref(server + "hostname");
       let combinedKey;
       try {
         combinedKey =
           username + "@" + Services.eTLD.getBaseDomainFromHost(hostname);
       } catch (e) {
         combinedKey = username + "@" + hostname;
       }
       clientidCache.set(
         combinedKey,
         Services.prefs.getCharPref(server + "clientid")
       );
     }
   }
 
-  let accountKeys = accounts.split(",");
-
   // Now walk all imap accounts and generate any missing CLIENTIDS, reusing
   // cached CLIENTIDS if possible.
-  for (let accountKey of accountKeys) {
+  for (let key of accounts.split(",")) {
     let serverKey = Services.prefs.getCharPref(
-      "mail.account." + accountKey + ".server"
+      "mail.account." + key + ".server"
     );
     let server = "mail.server." + serverKey + ".";
     // Check if this server needs the CLIENTID preference to be populated.
     if (
       !Services.prefs.prefHasUserValue(server + "clientid") ||
-      !Services.prefs.getCharPref(server + "clientid")
+      !Services.prefs.getCharPref(server + "clientid", "")
     ) {
       // Grab username + hostname to check if a CLIENTID is cached.
-      let username = Services.prefs.getCharPref(server + "userName");
+      let username = Services.prefs.getCharPref(server + "userName", "");
+      if (!username) {
+        continue;
+      }
       let hostname = Services.prefs.getCharPref(server + "hostname");
       let combinedKey;
       try {
         combinedKey =
           username + "@" + Services.eTLD.getBaseDomainFromHost(hostname);
       } catch (e) {
         combinedKey = username + "@" + hostname;
       }