Bug 1355198 - Fix "DIRECT" return type to take no arguments and update error handling r?mixedpuppy draft
authorMatthew Wein <mwein@mozilla.com>
Tue, 11 Apr 2017 12:19:53 -0400
changeset 568948 7d7409c136c4dc0a781a31dfc609b65cd1edf591
parent 568807 a477e80f03b61be9961bc61770a2b55cce139b91
child 626075 ba35b71b4fa1e2f6518e82ca83592eddffb60d58
push id56032
push usermwein@mozilla.com
push dateWed, 26 Apr 2017 20:28:03 +0000
reviewersmixedpuppy
bugs1355198
milestone55.0a1
Bug 1355198 - Fix "DIRECT" return type to take no arguments and update error handling r?mixedpuppy MozReview-Commit-ID: AKY6cjDoSmZ
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
@@ -153,58 +153,76 @@ 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: Missing Proxy 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.SOCKS:
         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 = PROXY_TYPES.SOCKS;
-        if (parts[0] == PROXY_TYPES.PROXY) {
+        if (parts[0].toLowerCase() == PROXY_TYPES.PROXY) {
           type = PROXY_TYPES.HTTPS;
         }
 
         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
@@ -58,33 +58,96 @@ add_task(function* test_invalid_FindProx
   yield 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(function* test_invalid_FindProxyForURL_return_type() {
+add_task(function* test_invalid_FindProxyForURL_return_types() {
   yield testProxyScript(
     () => {
       function FindProxyForURL() {
         return 5;
       }
     }, {
       message: "FindProxyForURL: Return type must be a string",
     });
 
   yield testProxyScript(
     () => {
       function FindProxyForURL() {
         return "INVALID";
       }
     }, {
-      message: "FindProxyForURL: Invalid Proxy Rule: INVALID",
+      message: "FindProxyForURL: Unrecognized proxy type: \"INVALID\"",
+    });
+
+  yield testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "SOCKS";
+      }
+    }, {
+      message: "FindProxyForURL: Missing argument for proxy type: \"SOCKS\"",
+    });
+
+  yield 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\"",
+    });
+
+  yield testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "PROXY :";
+      }
+    }, {
+      message: "FindProxyForURL: Unable to parse host and port from proxy rule: \"PROXY :\"",
+    });
+
+  yield testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "PROXY :8080";
+      }
+    }, {
+      message: "FindProxyForURL: Unable to parse host and port from proxy rule: \"PROXY :8080\"",
+    });
+
+  yield testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "PROXY ::";
+      }
+    }, {
+      message: "FindProxyForURL: Unable to parse host and port from proxy rule: \"PROXY ::\"",
+    });
+
+  yield 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:\"",
+    });
+
+  yield testProxyScript(
+    () => {
+      function FindProxyForURL() {
+        return "DIRECT 1.2.3.4:8080";
+      }
+    }, {
+      message: "FindProxyForURL: Invalid argument for proxy type: \"DIRECT\"",
     });
 });
 
 add_task(function* test_runtime_error_in_FindProxyForURL() {
   yield testProxyScript(
     () => {
       function FindProxyForURL() {
         return not_defined; // eslint-disable-line no-undef