Bug 1462959 - Catch exceptions accessing .hostPort for non-http/ftp schemes in LoginHelper.doLoginsMatch. r=johannh

nsIURI.hostPort would throw NS_ERROR_FAILURE in this case

MozReview-Commit-ID: K4mK3X91ic0

--HG--
extra : rebase_source : a3a977e392b771bd15bbba4df476adcb4ee97c95

toolkit/components/passwordmgr/LoginHelper.jsm

@@ -169,6 +169,21 @@ var LoginHelper = {
     return Services.logins.searchLogins({}, this.newPropertyBag(aSearchOptions));
   },
 
+  /**
+   * @param {string} aURL
+   * @returns {string} which is the hostPort of aURL if supported by the scheme
+   *                   otherwise, returns the original aURL.
+   */
+  maybeGetHostPortForURL(aURL) {
+    try {
+      let uri = Services.io.newURI(aURL);
+      return uri.hostPort;
+    } catch (ex) {
+      // No need to warn for javascript:/data:/about:/chrome:/etc.
+    }
+    return aURL;
+  },
+
   /**
    * @param {String} aLoginOrigin - An origin value from a stored login's
    *                                hostname or formSubmitURL properties.
@@ -220,15 +235,16 @@ var LoginHelper = {
       return false;
 
     if (ignoreSchemes) {
-      let hostname1URI = Services.io.newURI(aLogin1.hostname);
-      let hostname2URI = Services.io.newURI(aLogin2.hostname);
-      if (hostname1URI.hostPort != hostname2URI.hostPort)
+      let login1HostPort = this.maybeGetHostPortForURL(aLogin1.hostname);
+      let login2HostPort = this.maybeGetHostPortForURL(aLogin2.hostname);
+      if (login1HostPort != login2HostPort)
         return false;
 
       if (aLogin1.formSubmitURL != "" && aLogin2.formSubmitURL != "" &&
-          Services.io.newURI(aLogin1.formSubmitURL).hostPort !=
-          Services.io.newURI(aLogin2.formSubmitURL).hostPort)
+          this.maybeGetHostPortForURL(aLogin1.formSubmitURL) !=
+          this.maybeGetHostPortForURL(aLogin2.formSubmitURL)) {
         return false;
+      }
     } else {
       if (aLogin1.hostname != aLogin2.hostname)
         return false;

toolkit/components/passwordmgr/test/unit/test_doLoginsMatch.js

@@ -0,0 +1,50 @@
+/**
+ * Test LoginHelper.doLoginsMatch
+ */
+
+add_task(function test_formSubmitURL_ignoreSchemes() {
+  let httpActionLogin = TestData.formLogin();
+  let httpsActionLogin = TestData.formLogin({
+    formSubmitURL: "https://www.example.com",
+  });
+  let jsActionLogin = TestData.formLogin({
+    formSubmitURL: "javascript:",
+  });
+  let emptyActionLogin = TestData.formLogin({
+    formSubmitURL: "",
+  });
+
+  Assert.notEqual(httpActionLogin.formSubmitURL, httpsActionLogin.formSubmitURL,
+                  "Ensure actions differ");
+
+  const TEST_CASES = [
+    [httpActionLogin, httpActionLogin, true],
+    [httpsActionLogin, httpsActionLogin, true],
+    [jsActionLogin, jsActionLogin, true],
+    [emptyActionLogin, emptyActionLogin, true],
+    // only differing by scheme:
+    [httpsActionLogin, httpActionLogin, true],
+    [httpActionLogin, httpsActionLogin, true],
+
+    // empty matches everything
+    [httpsActionLogin, emptyActionLogin, true],
+    [emptyActionLogin, httpsActionLogin, true],
+    [jsActionLogin, emptyActionLogin, true],
+    [emptyActionLogin, jsActionLogin, true],
+
+    // Begin false cases:
+    [httpsActionLogin, jsActionLogin, false],
+    [jsActionLogin, httpsActionLogin, false],
+    [httpActionLogin, jsActionLogin, false],
+    [jsActionLogin, httpActionLogin, false],
+  ];
+
+  for (let [login1, login2, expected] of TEST_CASES) {
+    Assert.strictEqual(LoginHelper.doLoginsMatch(login1, login2, {
+      ignorePassword: false,
+      ignoreSchemes: true,
+    }), expected, `LoginHelper.doLoginsMatch:
+\t${JSON.stringify(login1)}
+\t${JSON.stringify(login2)}`);
+  }
+});

toolkit/components/passwordmgr/test/unit/xpcshell.ini

@@ -21,6 +21,7 @@ skip-if = os == "android" # The context menu isn't used on Android.
 run-if = buildapp == "browser"
 [test_dedupeLogins.js]
 [test_disabled_hosts.js]
+[test_doLoginsMatch.js]
 [test_getFormFields.js]
 [test_getPasswordFields.js]
 [test_getPasswordOrigin.js]