Bug 1406763 - Strip brackets around IPv6 addresses for proxy hosts. r=ato
authorHenrik Skupin <mail@hskupin.info>
Mon, 09 Oct 2017 14:10:03 +0200
changeset 385901 633b06e5fc74afd202a58c722ae732872e512bc2
parent 385900 d8337396947a7c762722cd67289635a9c0489f2b
child 385902 ee681d6964d3f6f626d4ea2b569f241b4f67ab1b
push id53203
push userhskupin@mozilla.com
push dateThu, 12 Oct 2017 19:03:47 +0000
treeherderautoland@ee681d6964d3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1406763
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1406763 - Strip brackets around IPv6 addresses for proxy hosts. r=ato The WebDriver specification requires IPv6 addresses to be always added with brackets for proxy hosts. But Firefox itself handles those without brackets for both the proxy host, and the noProxy settings. MozReview-Commit-ID: 9vpvXjDDuxd
testing/marionette/session.js
testing/marionette/test_session.js
--- a/testing/marionette/session.js
+++ b/testing/marionette/session.js
@@ -191,16 +191,20 @@ session.Proxy = class {
   /**
    * @param {Object.<string, ?>} json
    *     JSON Object to unmarshal.
    *
    * @throws {InvalidArgumentError}
    *     When proxy configuration is invalid.
    */
   static fromJSON(json) {
+    function stripBracketsFromIpv6Hostname(hostname) {
+      return hostname.includes(":") ? hostname.replace(/[\[\]]/g, "") : hostname;
+    }
+
     // Parse hostname and optional port from host
     function fromHost(scheme, host) {
       assert.string(host);
 
       if (host.includes("://")) {
         throw new InvalidArgumentError(`${host} contains a scheme`);
       }
 
@@ -214,16 +218,18 @@ session.Proxy = class {
         url = new URL("http://" + host);
         if (url.port == "") {
           url = new URL("https://" + host);
         }
       } catch (e) {
         throw new InvalidArgumentError(e.message);
       }
 
+      let hostname = stripBracketsFromIpv6Hostname(url.hostname);
+
       // If the port hasn't been set, use the default port of
       // the selected scheme (except for socks which doesn't have one).
       let port = parseInt(url.port);
       if (!Number.isInteger(port)) {
         if (scheme === "socks") {
           port = null;
         } else {
           port = Services.io.getProtocolHandler(scheme).defaultPort;
@@ -234,17 +240,17 @@ session.Proxy = class {
           url.password != "" ||
           url.pathname != "/" ||
           url.search != "" ||
           url.hash != "") {
         throw new InvalidArgumentError(
             `${host} was not of the form host[:port]`);
       }
 
-      return [url.hostname, port];
+      return [hostname, port];
     }
 
     let p = new session.Proxy();
     if (typeof json == "undefined" || json === null) {
       return p;
     }
 
     assert.object(json);
@@ -273,53 +279,65 @@ session.Proxy = class {
           [p.sslProxy, p.sslProxyPort] = fromHost("https", json.sslProxy);
         }
         if (typeof json.socksProxy != "undefined") {
           [p.socksProxy, p.socksProxyPort] = fromHost("socks", json.socksProxy);
           p.socksVersion = assert.positiveInteger(json.socksVersion);
         }
         if (typeof json.noProxy != "undefined") {
           let entries = assert.array(json.noProxy);
-          for (let entry of entries) {
+          p.noProxy = entries.map(entry => {
             assert.string(entry);
-          }
-          p.noProxy = entries;
+            return stripBracketsFromIpv6Hostname(entry);
+          });
         }
         break;
 
       default:
         throw new InvalidArgumentError(
             `Invalid type of proxy: ${p.proxyType}`);
     }
 
     return p;
   }
 
   /**
    * @return {Object.<string, (number|string)>}
    *     JSON serialisation of proxy object.
    */
   toJSON() {
+    function addBracketsToIpv6Hostname(hostname) {
+      return hostname.includes(":") ? `[${hostname}]` : hostname;
+    }
+
     function toHost(hostname, port) {
       if (!hostname) {
         return null;
       }
 
+      // Add brackets around IPv6 addresses
+      hostname = addBracketsToIpv6Hostname(hostname);
+
       if (port != null) {
         return `${hostname}:${port}`;
       }
 
       return hostname;
     }
 
+    let excludes = this.noProxy;
+    if (excludes) {
+      excludes = excludes.map(addBracketsToIpv6Hostname);
+    }
+
     return marshal({
       proxyType: this.proxyType,
       ftpProxy: toHost(this.ftpProxy, this.ftpProxyPort),
       httpProxy: toHost(this.httpProxy, this.httpProxyPort),
-      noProxy: this.noProxy,
+      noProxy: excludes,
       sslProxy: toHost(this.sslProxy, this.sslProxyPort),
       socksProxy: toHost(this.socksProxy, this.socksProxyPort),
       socksVersion: this.socksVersion,
       proxyAutoconfigUrl: this.proxyAutoconfigUrl,
     });
   }
 
   toString() { return "[object session.Proxy]"; }
--- a/testing/marionette/test_session.js
+++ b/testing/marionette/test_session.js
@@ -218,18 +218,32 @@ add_test(function test_Proxy_toJSON() {
     p[proxy] = "foo";
     p[`${proxy}Port`] = 0;
     expected[proxy] = "foo:0";
     deepEqual(p.toJSON(), expected);
 
     p[`${proxy}Port`] = 42;
     expected[proxy] = "foo:42"
     deepEqual(p.toJSON(), expected);
+
+    // add brackets for IPv6 address as proxy hostname
+    p[proxy] = "2001:db8::1";
+    p[`${proxy}Port`] = 42;
+    expected[proxy] = "foo:42"
+    expected[proxy] = "[2001:db8::1]:42";
+    deepEqual(p.toJSON(), expected);
   }
 
+  // noProxy: add brackets for IPv6 address
+  p = new session.Proxy();
+  p.proxyType = "manual";
+  p.noProxy = ["2001:db8::1"];
+  let expected = {proxyType: "manual", noProxy: "[2001:db8::1]"};
+  deepEqual(p.toJSON(), expected);
+
   run_next_test();
 });
 
 add_test(function test_Proxy_fromJSON() {
   let p = new session.Proxy();
   deepEqual(p, session.Proxy.fromJSON(undefined));
   deepEqual(p, session.Proxy.fromJSON(null));
 
@@ -280,17 +294,17 @@ add_test(function test_Proxy_fromJSON() 
 
     let host_map = {
       "foo:1": {hostname: "foo", port: 1},
       "foo:21": {hostname: "foo", port: 21},
       "foo:80": {hostname: "foo", port: 80},
       "foo:443": {hostname: "foo", port: 443},
       "foo:65535": {hostname: "foo", port: 65535},
       "127.0.0.1:42": {hostname: "127.0.0.1", port: 42},
-      "[2001:db8::1]:42": {hostname: "[2001:db8::1]", port: "42"},
+      "[2001:db8::1]:42": {hostname: "2001:db8::1", port: "42"},
     };
 
     // valid proxy hosts with port
     for (let host in host_map) {
       manual[proxy] = host;
 
       p[`${proxy}`] = host_map[host]["hostname"];
       p[`${proxy}Port`] = host_map[host]["port"];
@@ -317,34 +331,40 @@ add_test(function test_Proxy_fromJSON() 
     }
   }
 
   // missing required socks version
   Assert.throws(() => session.Proxy.fromJSON(
       {proxyType: "manual", socksProxy: "foo:1234"}),
       InvalidArgumentError);
 
-  // invalid noProxy
+  // noProxy: invalid settings
   for (let noProxy of [true, 42, {}, null, "foo",
       [true], [42], [{}], [null]]) {
     Assert.throws(() => session.Proxy.fromJSON(
         {proxyType: "manual", noProxy: noProxy}),
         InvalidArgumentError);
   }
 
-  // valid noProxy
+  // noProxy: valid settings
   p = new session.Proxy();
   p.proxyType = "manual";
-  for (let noProxy of [[], ["foo"], ["foo", "bar"],
-      ["127.0.0.1"], ["[2001:db8::1"]]) {
+  for (let noProxy of [[], ["foo"], ["foo", "bar"], ["127.0.0.1"]]) {
     let manual = {proxyType: "manual", "noProxy": noProxy}
     p.noProxy = noProxy;
     deepEqual(p, session.Proxy.fromJSON(manual));
   }
 
+  // noProxy: IPv6 needs brackets removed
+  p = new session.Proxy();
+  p.proxyType = "manual";
+  p.noProxy = ["2001:db8::1"];
+  let manual = {proxyType: "manual", "noProxy": ["[2001:db8::1]"]}
+  deepEqual(p, session.Proxy.fromJSON(manual));
+
   run_next_test();
 });
 
 add_test(function test_Capabilities_ctor() {
   let caps = new session.Capabilities();
   ok(caps.has("browserName"));
   ok(caps.has("browserVersion"));
   ok(caps.has("platformName"));