Bug 1355198 - Fix DIRECT return type to take no arguments and update error handling r?mixedpuppy draft
authorMatthew Wein <mwein@mozilla.com>
Tue, 06 Jun 2017 12:34:12 -0400
changeset 593788 53bb7558bddcca96fe9682c635873c9251caec24
parent 593717 b266a8d8fd595b84a7d6218d7b8c6b7af0b5027c
child 633208 8748592f6dabf1faaf4187e5a7c241ae3d71193b
push id63803
push usermwein@mozilla.com
push dateWed, 14 Jun 2017 04:03:26 +0000
reviewersmixedpuppy
bugs1355198
milestone56.0a1
Bug 1355198 - Fix DIRECT return type to take no arguments and update error handling r?mixedpuppy MozReview-Commit-ID: 67JbDySgPf0
toolkit/components/extensions/ProxyScriptContext.jsm
toolkit/components/extensions/test/mochitest/test_ext_proxy.html
--- a/toolkit/components/extensions/ProxyScriptContext.jsm
+++ b/toolkit/components/extensions/ProxyScriptContext.jsm
@@ -155,62 +155,79 @@ class ProxyScriptContext extends BaseCon
     if (!rules.length) {
       return null;
     }
 
     let rule = rules[0].trim();
 
     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:
       case PROXY_TYPES.SOCKS:
       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;
         }
 
         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;
         }
 
         let failoverProxy = this.createProxyInfo(rules.slice(1));
         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", {
           message: `FindProxyForURL: Unrecognized proxy type: "${parts[0]}"`,
         });
         return null;
     }
   }
--- a/toolkit/components/extensions/test/mochitest/test_ext_proxy.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_proxy.html
@@ -43,52 +43,114 @@ async function testProxyScript(script, e
   is(error.message, expected.message, "Correct error message received");
 
   if (expected.errorInfo) {
     ok(error.fileName.includes("proxy_script.js"), "Error should include file name");
     is(error.lineNumber, 3, "Error should include line number");
     ok(error.stack.includes("proxy_script.js:3:9"), "Error should include stack trace");
   }
 
+  win.close();
   await extension.unload();
-
-  win.close();
 }
 
 add_task(async function test_invalid_FindProxyForURL_type() {
   await testProxyScript(
     () => { }, {
       message: "The proxy script must define FindProxyForURL as a function",
     });
 
   await testProxyScript(
     () => {
       var FindProxyForURL = 5; // eslint-disable-line mozilla/var-only-at-top-level
     }, {
       message: "The proxy script must define FindProxyForURL as a function",
     });
 });
 
-add_task(async function test_invalid_FindProxyForURL_return_type() {
+add_task(async function test_invalid_FindProxyForURL_return_types() {
   await testProxyScript(
     () => {
       function FindProxyForURL() {
         return 5;
       }
     }, {
       message: "FindProxyForURL: Return type must be a string",
     });
 
   await testProxyScript(
     () => {
       function FindProxyForURL() {
         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\"",
     });
 });
 
 add_task(async function test_runtime_error_in_FindProxyForURL() {
   await testProxyScript(
     () => {
       function FindProxyForURL() {
         return not_defined; // eslint-disable-line no-undef