Bug 1406763 - Strip brackets around IPv6 addresses for proxy hosts. r=ato
☠☠ backed out by da7917089c4b ☠ ☠
authorHenrik Skupin <mail@hskupin.info>
Mon, 09 Oct 2017 14:10:03 +0200
changeset 385921 0ce17eb985f1612c5091c123b4bebe970701df07
parent 385920 e7f4ffe10569be04293a8195045cbc8f198209f6
child 385922 b85be3749e704fe1037556e9eea72dfd0ab8c7e2
push id32672
push userarchaeopteryx@coole-files.de
push dateFri, 13 Oct 2017 09:00:05 +0000
treeherdermozilla-central@3efcb26e5f37 [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"));