Bug 1526890 - enforce default handlers use https, r=mak

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

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

uriloader/exthandler/HandlerService.js

@@ -153,6 +153,28 @@ HandlerService.prototype = {
       } catch (ex) {}
     }
 
+    // Now drop any entries without a uriTemplate, or with a broken one.
+    // The Array.from calls ensure we can safely delete things without
+    // affecting the iterator.
+    for (let [scheme, handlerObject] of Array.from(Object.entries(schemes))) {
+      let handlers = Array.from(Object.entries(handlerObject));
+      let validHandlers = 0;
+      for (let [key, obj] of handlers) {
+        if (
+          !obj.uriTemplate ||
+          !obj.uriTemplate.startsWith("https://") ||
+          !obj.uriTemplate.toLowerCase().includes("%s")
+        ) {
+          delete handlerObject[key];
+        } else {
+          validHandlers++;
+        }
+      }
+      if (!validHandlers) {
+        delete schemes[scheme];
+      }
+    }
+
     // Now, we're going to cheat. Terribly. The idiologically correct way
     // of implementing the following bit of code would be to fetch the
     // handler info objects from the protocol service, manipulate those,
@@ -164,7 +186,7 @@ HandlerService.prototype = {
     // equivalent of appending into the database. So let's just go do that:
     for (let scheme of Object.keys(schemes)) {
       let existingSchemeInfo = this._store.data.schemes[scheme];
-      if (!this._store.data.schemes[scheme]) {
+      if (!existingSchemeInfo) {
         // Haven't seen this scheme before. Default to asking which app the
         // user wants to use:
         existingSchemeInfo = {

uriloader/exthandler/tests/unit/test_defaults_handlerService.js

@@ -104,3 +104,60 @@ add_task(async function test_check_default_modification() {
   Assert.equal(newMail.alwaysAskBeforeHandling, false);
   await deleteHandlerStore();
 });
+
+/**
+ * Check that we don't add bogus handlers.
+ */
+add_task(async function test_check_restrictions() {
+  const kTestData = {
+    testdeleteme: [
+      ["Delete me", ""],
+      ["Delete me insecure", "http://example.com/%s"],
+      ["Delete me no substitution", "https://example.com/"],
+      ["Keep me", "https://example.com/%s"],
+    ],
+    testreallydeleteme: [
+      // used to check we remove the entire entry.
+      ["Delete me", "http://example.com/%s"],
+    ],
+  };
+  for (let [scheme, handlers] of Object.entries(kTestData)) {
+    let count = 1;
+    for (let [name, uriTemplate] of handlers) {
+      let pref = `gecko.handlerService.schemes.${scheme}.${count}.`;
+      let obj = Cc["@mozilla.org/pref-localizedstring;1"].createInstance(
+        Ci.nsIPrefLocalizedString
+      );
+      obj.data = name;
+      Services.prefs.setComplexValue(
+        pref + "name",
+        Ci.nsIPrefLocalizedString,
+        obj
+      );
+      obj.data = uriTemplate;
+      Services.prefs.setComplexValue(
+        pref + "uriTemplate",
+        Ci.nsIPrefLocalizedString,
+        obj
+      );
+      count++;
+    }
+  }
+
+  gHandlerService.wrappedJSObject._injectDefaultProtocolHandlers();
+  let schemeData = gHandlerService.wrappedJSObject._store.data.schemes;
+
+  Assert.ok(schemeData.testdeleteme, "Expect an entry for testdeleteme");
+  Assert.ok(
+    schemeData.testdeleteme.stubEntry,
+    "Expect a stub entry for testdeleteme"
+  );
+
+  Assert.deepEqual(
+    schemeData.testdeleteme.handlers,
+    [null, { name: "Keep me", uriTemplate: "https://example.com/%s" }],
+    "Expect only one handler is kept."
+  );
+
+  Assert.ok(!schemeData.testreallydeleteme, "No entry for reallydeleteme");
+});