Bug 1355198 - Fix DIRECT return type to take no arguments and update error handling r=mixedpuppy

MozReview-Commit-ID: 67JbDySgPf0

--HG--
extra : rebase_source : d4dc53ea2241b0ff92fc2b71f44c3a139e2d1fde

toolkit/components/extensions/ProxyScriptContext.jsm

@@ -160,22 +160,27 @@ class ProxyScriptContext extends BaseContext {
 
     if (!rule) {
       this.extension.emit("proxy-error", {
-        message: "FindProxyForURL: Expected Proxy Rule",
+        message: "FindProxyForURL: Missing Proxy Rule",
       });
       return null;
     }
 
     let parts = rule.split(/\s+/);
-    if (!parts[0] || parts.length !== 2) {
+    if (!parts[0]) {
       this.extension.emit("proxy-error", {
-        message: `FindProxyForURL: Invalid Proxy Rule: ${rule}`,
+        message: `FindProxyForURL: Too many arguments passed for proxy rule: "${rule}"`,
       });
       return null;
     }
 
-    parts[0] = parts[0].toLowerCase();
+    if (parts.length > 2) {
+      this.extension.emit("proxy-error", {
+        message: `FindProxyForURL: Too many arguments passed for proxy rule: "${rule}"`,
+      });
+      return null;
+    }
 
-    switch (parts[0]) {
+    switch (parts[0].toLowerCase()) {
       case PROXY_TYPES.PROXY:
       case PROXY_TYPES.HTTP:
       case PROXY_TYPES.HTTPS:
@@ -183,7 +188,14 @@ class ProxyScriptContext extends BaseContext {
       case PROXY_TYPES.SOCKS4:
         if (!parts[1]) {
           this.extension.emit("proxy-error", {
-            message: `FindProxyForURL: Missing argument for "${parts[0]}"`,
+            message: `FindProxyForURL: Missing argument for proxy type: "${parts[0]}"`,
+          });
+          return null;
+        }
+
+        if (parts.length != 2) {
+          this.extension.emit("proxy-error", {
+            message: `FindProxyForURL: Too many arguments for proxy rule: "${rule}"`,
           });
           return null;
         }
@@ -191,13 +203,13 @@ class ProxyScriptContext extends BaseContext {
         let [host, port] = parts[1].split(":");
         if (!host || !port) {
           this.extension.emit("proxy-error", {
-            message: `FindProxyForURL: Unable to parse argument for ${rule}`,
+            message: `FindProxyForURL: Unable to parse host and port from proxy rule: "${rule}"`,
           });
           return null;
         }
 
         let type = parts[0];
-        if (parts[0] == PROXY_TYPES.PROXY) {
+        if (parts[0].toLowerCase() == PROXY_TYPES.PROXY) {
           // PROXY_TYPES.HTTP and PROXY_TYPES.PROXY are synonyms
           type = PROXY_TYPES.HTTP;
         }
@@ -206,6 +218,11 @@ class ProxyScriptContext extends BaseContext {
         return ProxyService.newProxyInfo(type, host, port, 0,
           PROXY_TIMEOUT_SEC, failoverProxy);
       case PROXY_TYPES.DIRECT:
+        if (parts.length != 1) {
+          this.extension.emit("proxy-error", {
+            message: `FindProxyForURL: Invalid argument for proxy type: "${parts[0]}"`,
+          });
+        }
         return null;
       default:
         this.extension.emit("proxy-error", {

toolkit/components/extensions/test/mochitest/test_ext_proxy.html

@@ -48,9 +48,8 @@ async function testProxyScript(script, expected) {
     ok(error.stack.includes("proxy_script.js:3:9"), "Error should include stack trace");
   }
 
-  await extension.unload();
-
   win.close();
+  await extension.unload();
 }
 
 add_task(async function test_invalid_FindProxyForURL_type() {
@@ -67,7 +66,7 @@ add_task(async function test_invalid_FindProxyForURL_type() {
     });
 });
 
-add_task(async function test_invalid_FindProxyForURL_return_type() {
+add_task(async function test_invalid_FindProxyForURL_return_types() {
   await testProxyScript(
     () => {
       function FindProxyForURL() {
@@ -83,7 +82,70 @@ add_task(async function test_invalid_FindProxyForURL_return_type() {
         return "INVALID";
       }
     }, {
-      message: "FindProxyForURL: Invalid Proxy Rule: INVALID",
+      message: "FindProxyForURL: Unrecognized proxy type: \"INVALID\"",
+    });
+
+  await testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "SOCKS";
+      }
+    }, {
+      message: "FindProxyForURL: Missing argument for proxy type: \"SOCKS\"",
+    });
+
+  await testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "PROXY 1.2.3.4:8080 EXTRA";
+      }
+    }, {
+      message: "FindProxyForURL: Too many arguments passed for proxy rule: \"PROXY 1.2.3.4:8080 EXTRA\"",
+    });
+
+  await testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "PROXY :";
+      }
+    }, {
+      message: "FindProxyForURL: Unable to parse host and port from proxy rule: \"PROXY :\"",
+    });
+
+  await testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "PROXY :8080";
+      }
+    }, {
+      message: "FindProxyForURL: Unable to parse host and port from proxy rule: \"PROXY :8080\"",
+    });
+
+  await testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "PROXY ::";
+      }
+    }, {
+      message: "FindProxyForURL: Unable to parse host and port from proxy rule: \"PROXY ::\"",
+    });
+
+  await testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "PROXY 1.2.3.4:";
+      }
+    }, {
+      message: "FindProxyForURL: Unable to parse host and port from proxy rule: \"PROXY 1.2.3.4:\"",
+    });
+
+  await testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "DIRECT 1.2.3.4:8080";
+      }
+    }, {
+      message: "FindProxyForURL: Invalid argument for proxy type: \"DIRECT\"",
     });
 });