Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

url: use SafeSet to filter known special protocols #24703

Closed
wants to merge 1 commit into from

Conversation

mikesamuel
Copy link
Contributor

@mikesamuel mikesamuel commented Nov 28, 2018

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Avoids a maintenance hazard when reviewers assume that
hostlessProtocol and slashedProtocol are disjoint.

The following may be counter-intuitive:

// These objects seem to have no keys in common
const hostlessProtocol = { 'javascript': true };
const slashedProtocol = { 'http': true };
// A reasonable reviewer may assumes bothTrue is never truthy
function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}
// But
console.log(Boolean(bothTrue('constructor')));  // true

This change uses SafeSet instead of plain-old objects.


Rejected alternative:

We could have used object with a null prototype as lookup tables
so that lowerProto is never treated as a key into Object.prototype.

const hostlessProtocol = { __proto__: null, 'javascript': true };
const slashedProtocol = { __proto__: null, 'http': true };

function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}

console.log(Boolean(bothTrue('constructor')));  // false

@nodejs-github-bot nodejs-github-bot added the url Issues and PRs related to the legacy built-in url module. label Nov 28, 2018
@devsnek
Copy link
Member

devsnek commented Nov 28, 2018

can they just be changed to maps?

@mikesamuel
Copy link
Contributor Author

@devsnek, They could be changed to Sets. There's no need to lookup a value.

Would you prefer that?

@devsnek
Copy link
Member

devsnek commented Nov 28, 2018

@mikesamuel i would definitely prefer using sets

@joyeecheung
Copy link
Member

joyeecheung commented Nov 28, 2018

This seems to make the code harder to understand. We usually just go with Object.create(null) if the object is part of a public API, or go with Map if the object is entirely internal. As @mikesamuel pointed out it should be safe to just use a Set so I'd prefer we go with a Set. If one is really feeling paranoid, they can even use the require('internal/safe_gloabls').SafeSet

@lpinca
Copy link
Member

lpinca commented Nov 28, 2018

I think the reason to no use null prototype objects was lookup performance. I don't know if it's still faster.

@mikesamuel
Copy link
Contributor Author

Reworking to use sets.

@lpinca, Re performance, https://github.com/anvaka/set-vs-object#conclusion is one microbenchmark.

@mikesamuel
Copy link
Contributor Author

I changed it to use SafeSet (thanks @joyeecheung), squashed the commits, and changed the PR description to reflect that.

@mikesamuel
Copy link
Contributor Author

I think my squash broke Travis's first commit message check :(

@joyeecheung
Copy link
Member

joyeecheung commented Nov 28, 2018

@mikesamuel Can you update the commit message to something that describes the current approach? (e.g. something like url: use SafeSet to filter known special protocols)

Avoids a maintenance hazard when reviewers assume that
`hostlessProtocol` and `slashedProtocol` are disjoint.

The following may be counter-intuitive:

```js
// These objects seem to have no keys in common
const hostlessProtocol = { 'javascript': true };
const slashedProtocol = { 'http': true };
// A reasonable reviewer may assumes bothTrue is never truthy
function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}
// But
console.log(Boolean(bothTrue('constructor')));  // true
```

This change uses SafeSet instead of plain-old objects.

----

Rejected alternative:

We could have used object with a `null` prototype as lookup tables
so that `lowerProto` is never treated as a key into `Object.prototype`.

```js
const hostlessProtocol = { __proto__: null, 'javascript': true };
const slashedProtocol = { __proto__: null, 'http': true };

function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}

console.log(Boolean(bothTrue('constructor')));  // false
```
@mikesamuel
Copy link
Contributor Author

@joyeecheung, done

@mikesamuel mikesamuel changed the title url: prototypes are not useful in lookup tables url: use SafeSet to filter known special protocols Nov 29, 2018
@joyeecheung
Copy link
Member

@Trott
Copy link
Member

Trott commented Dec 1, 2018

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 1, 2018
@Trott
Copy link
Member

Trott commented Dec 1, 2018

(Benchmark tradeoffs look perfectly acceptable to me, but second opinion welcome.)

@Trott
Copy link
Member

Trott commented Dec 1, 2018

Trott pushed a commit to Trott/io.js that referenced this pull request Dec 1, 2018
Avoids a maintenance hazard when reviewers assume that
`hostlessProtocol` and `slashedProtocol` are disjoint.

The following may be counter-intuitive:

```js
// These objects seem to have no keys in common
const hostlessProtocol = { 'javascript': true };
const slashedProtocol = { 'http': true };
// A reasonable reviewer may assumes bothTrue is never truthy
function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}
// But
console.log(Boolean(bothTrue('constructor')));  // true
```

This change uses SafeSet instead of plain-old objects.

----

Rejected alternative:

We could have used object with a `null` prototype as lookup tables
so that `lowerProto` is never treated as a key into `Object.prototype`.

```js
const hostlessProtocol = { __proto__: null, 'javascript': true };
const slashedProtocol = { __proto__: null, 'http': true };

function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}

console.log(Boolean(bothTrue('constructor')));  // false
```

PR-URL: nodejs#24703
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member

Trott commented Dec 1, 2018

Landed in 0d23118.

Thanks for the contribution! 🎉

@Trott Trott closed this Dec 1, 2018
@joyeecheung
Copy link
Member

joyeecheung commented Dec 1, 2018

Post-mortem: benchmark results. The most significant ones (with *** are mostly improvements)

Benchmark results
                                                                                                           confidence improvement accuracy (*)    (**)   (***)
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='legacy' type='auth'                                                      3.06 %       ±7.23%  ±9.62% ±12.54%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='legacy' type='dot'                                                       1.94 %       ±5.87%  ±7.80% ±10.16%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='legacy' type='file'                                                      7.54 %      ±10.54% ±14.03% ±18.26%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='legacy' type='idn'                                                      -0.41 %       ±8.30% ±11.05% ±14.38%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='legacy' type='javascript'                                                1.57 %       ±7.29%  ±9.70% ±12.64%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='legacy' type='long'                                                     -0.94 %      ±10.38% ±13.82% ±17.99%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='legacy' type='percent'                                                  -3.72 %       ±8.28% ±11.02% ±14.34%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='legacy' type='short'                                                    -1.87 %       ±6.14%  ±8.18% ±10.67%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='legacy' type='wpt'                                                      -5.51 %       ±5.73%  ±7.62%  ±9.92%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='legacy' type='ws'                                                        4.73 %      ±10.34% ±13.77% ±17.95%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='whatwg' type='auth'                                                      2.97 %       ±6.47%  ±8.63% ±11.27%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='whatwg' type='dot'                                                      -0.11 %      ±11.02% ±14.69% ±19.16%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='whatwg' type='file'                                                      0.29 %       ±7.57% ±10.08% ±13.11%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='whatwg' type='idn'                                                      -3.32 %       ±5.87%  ±7.86% ±10.33%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='whatwg' type='javascript'                                               -3.97 %       ±9.63% ±12.82% ±16.68%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='whatwg' type='long'                                                      1.61 %       ±7.89% ±10.53% ±13.75%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='whatwg' type='percent'                                                   2.45 %       ±8.41% ±11.19% ±14.57%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='whatwg' type='short'                                                     1.73 %       ±7.92% ±10.54% ±13.72%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='whatwg' type='wpt'                                                      -3.93 %       ±6.77%  ±9.01% ±11.73%
 url/legacy-vs-whatwg-url-get-prop.js e=1 method='whatwg' type='ws'                                                        4.93 %       ±8.46% ±11.26% ±14.66%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='auth' withBase='false'                                        2.12 %       ±8.05% ±10.74% ±14.05%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='auth' withBase='true'                                        -1.03 %       ±6.34%  ±8.43% ±10.98%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='dot' withBase='false'                                        -2.48 %       ±6.93%  ±9.23% ±12.04%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='dot' withBase='true'                                         -0.97 %       ±6.88%  ±9.15% ±11.91%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='file' withBase='false'                                        2.18 %       ±6.97%  ±9.28% ±12.10%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='file' withBase='true'                                         2.31 %       ±7.21%  ±9.59% ±12.48%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='idn' withBase='false'                                         1.26 %       ±7.64% ±10.16% ±13.24%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='idn' withBase='true'                                          2.94 %       ±5.29%  ±7.06%  ±9.21%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='javascript' withBase='false'                                  4.28 %       ±7.85% ±10.48% ±13.73%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='javascript' withBase='true'                                   0.57 %       ±6.67%  ±8.88% ±11.58%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='long' withBase='false'                                        1.60 %       ±5.60%  ±7.46%  ±9.72%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='long' withBase='true'                                        -2.44 %       ±3.87%  ±5.18%  ±6.81%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='percent' withBase='false'                                    -4.06 %       ±8.97% ±11.94% ±15.56%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='percent' withBase='true'                                      3.20 %       ±7.45%  ±9.92% ±12.91%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='short' withBase='false'                                       2.14 %       ±5.29%  ±7.04%  ±9.17%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='short' withBase='true'                                        4.66 %       ±7.99% ±10.63% ±13.84%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='wpt' withBase='false'                                        -2.25 %       ±7.75% ±10.32% ±13.43%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='wpt' withBase='true'                                   *     -5.31 %       ±4.38%  ±5.83%  ±7.60%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='ws' withBase='false'                                          4.99 %       ±7.16%  ±9.57% ±12.53%
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='ws' withBase='true'                                          -5.13 %       ±6.06%  ±8.09% ±10.58%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='auth' withBase='false'                                       -4.43 %       ±7.82% ±10.40% ±13.54%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='auth' withBase='true'                                         2.94 %       ±7.23%  ±9.64% ±12.58%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='dot' withBase='false'                                  *     -6.11 %       ±5.94%  ±7.93% ±10.37%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='dot' withBase='true'                                          0.15 %      ±11.89% ±15.86% ±20.73%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='file' withBase='false'                                       -3.35 %       ±6.27%  ±8.35% ±10.86%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='file' withBase='true'                                        -1.54 %       ±6.93%  ±9.25% ±12.10%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='idn' withBase='false'                                        -3.91 %       ±8.43% ±11.23% ±14.65%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='idn' withBase='true'                                         -4.39 %       ±8.24% ±11.01% ±14.41%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='javascript' withBase='false'                                  1.22 %       ±6.84%  ±9.11% ±11.90%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='javascript' withBase='true'                                   4.17 %       ±6.88%  ±9.15% ±11.91%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='long' withBase='false'                                 *      8.84 %       ±8.76% ±11.71% ±15.35%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='long' withBase='true'                                        -2.62 %       ±7.12%  ±9.49% ±12.38%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='percent' withBase='false'                                     1.09 %       ±9.30% ±12.37% ±16.10%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='percent' withBase='true'                                      2.71 %       ±7.25%  ±9.65% ±12.57%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='short' withBase='false'                                       5.65 %       ±7.10%  ±9.49% ±12.44%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='short' withBase='true'                                       -4.33 %       ±8.15% ±10.87% ±14.20%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='wpt' withBase='false'                                        -0.90 %       ±5.88%  ±7.82% ±10.19%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='wpt' withBase='true'                                          1.01 %       ±6.17%  ±8.21% ±10.69%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='ws' withBase='false'                                         -2.23 %       ±7.11%  ±9.46% ±12.32%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='ws' withBase='true'                                           4.18 %       ±6.25%  ±8.33% ±10.86%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='altspaces'                          1.14 %       ±3.58%  ±4.76%  ±6.20%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='encodefake'                        -0.57 %       ±3.38%  ±4.50%  ±5.86%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='encodelast'                         0.25 %       ±2.46%  ±3.28%  ±4.28%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='encodemany'                        -0.17 %       ±3.84%  ±5.11%  ±6.65%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='manyblankpairs'                    -2.97 %       ±5.77%  ±7.74% ±10.22%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='manypairs'                          1.02 %       ±3.32%  ±4.43%  ±5.78%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='multicharsep'                       0.70 %       ±2.20%  ±2.93%  ±3.81%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='multivalue'                         0.29 %       ±4.29%  ±5.71%  ±7.44%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='multivaluemany'              *     -1.32 %       ±1.15%  ±1.53%  ±1.99%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='noencode'                    *     -2.09 %       ±1.83%  ±2.45%  ±3.20%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='altspaces'                          1.62 %       ±3.41%  ±4.54%  ±5.92%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='encodefake'                        -0.95 %       ±1.11%  ±1.48%  ±1.93%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='encodelast'                        -1.68 %       ±2.05%  ±2.73%  ±3.56%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='encodemany'                         0.89 %       ±4.94%  ±6.58%  ±8.58%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='manyblankpairs'                     0.76 %       ±1.68%  ±2.24%  ±2.93%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='manypairs'                          1.91 %       ±2.85%  ±3.81%  ±5.00%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='multicharsep'                       0.84 %       ±4.72%  ±6.29%  ±8.18%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='multivalue'                         0.30 %       ±5.38%  ±7.16%  ±9.33%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='multivaluemany'                    -0.47 %       ±2.27%  ±3.04%  ±4.01%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='noencode'                           0.82 %       ±3.49%  ±4.65%  ±6.05%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='legacy' searchParam='altspaces'               *     -3.32 %       ±3.26%  ±4.38%  ±5.76%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='legacy' searchParam='encodefake'                    -1.47 %       ±1.76%  ±2.34%  ±3.05%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='legacy' searchParam='encodelast'                     0.47 %       ±2.71%  ±3.61%  ±4.71%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='legacy' searchParam='encodemany'                    -3.19 %       ±4.14%  ±5.57%  ±7.37%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='legacy' searchParam='manyblankpairs'                -2.27 %       ±4.89%  ±6.50%  ±8.46%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='legacy' searchParam='manypairs'                      0.45 %       ±1.86%  ±2.48%  ±3.24%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='legacy' searchParam='multicharsep'                   0.67 %       ±2.36%  ±3.16%  ±4.16%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='legacy' searchParam='multivalue'                    -0.39 %       ±1.77%  ±2.36%  ±3.07%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='legacy' searchParam='multivaluemany'                 0.61 %       ±2.43%  ±3.24%  ±4.24%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='legacy' searchParam='noencode'                      -1.95 %       ±3.50%  ±4.70%  ±6.21%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='whatwg' searchParam='altspaces'                      0.18 %       ±3.22%  ±4.31%  ±5.65%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='whatwg' searchParam='encodefake'              *     -1.15 %       ±0.88%  ±1.17%  ±1.52%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='whatwg' searchParam='encodelast'                     1.98 %       ±4.56%  ±6.11%  ±8.04%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='whatwg' searchParam='encodemany'                     0.07 %       ±1.11%  ±1.48%  ±1.93%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='whatwg' searchParam='manyblankpairs'                 3.14 %       ±5.28%  ±7.04%  ±9.19%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='whatwg' searchParam='manypairs'                      1.54 %       ±2.28%  ±3.05%  ±4.00%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='whatwg' searchParam='multicharsep'                   0.73 %       ±1.43%  ±1.90%  ±2.47%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='whatwg' searchParam='multivalue'                    -0.26 %       ±1.43%  ±1.91%  ±2.50%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='whatwg' searchParam='multivaluemany'                -0.19 %       ±3.45%  ±4.60%  ±5.99%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='whatwg' searchParam='noencode'                      -0.49 %       ±1.36%  ±1.81%  ±2.35%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='legacy' type='auth'                                                    -2.89 %       ±5.68%  ±7.57%  ±9.87%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='legacy' type='dot'                                                      3.66 %       ±5.95%  ±7.92% ±10.33%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='legacy' type='file'                                                     3.40 %       ±6.43%  ±8.55% ±11.13%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='legacy' type='idn'                                                      3.92 %       ±6.18%  ±8.23% ±10.72%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='legacy' type='javascript'                                              -0.29 %       ±7.12%  ±9.47% ±12.33%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='legacy' type='long'                                                    -1.76 %       ±5.54%  ±7.38%  ±9.62%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='legacy' type='percent'                                           *      7.46 %       ±5.91%  ±7.90% ±10.36%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='legacy' type='short'                                                    2.66 %       ±6.89%  ±9.17% ±11.95%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='legacy' type='wpt'                                                      1.73 %       ±6.42%  ±8.55% ±11.13%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='legacy' type='ws'                                                       1.43 %       ±5.65%  ±7.52%  ±9.79%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='whatwg' type='auth'                                                     1.66 %       ±6.37%  ±8.49% ±11.08%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='whatwg' type='dot'                                                     -2.56 %       ±6.30%  ±8.39% ±10.96%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='whatwg' type='file'                                                    -2.45 %       ±5.55%  ±7.40%  ±9.65%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='whatwg' type='idn'                                                     -0.95 %       ±5.70%  ±7.59%  ±9.89%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='whatwg' type='javascript'                                               1.59 %       ±6.23%  ±8.30% ±10.82%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='whatwg' type='long'                                                     2.97 %       ±8.08% ±10.75% ±13.99%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='whatwg' type='percent'                                                 -1.31 %       ±4.91%  ±6.55%  ±8.55%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='whatwg' type='short'                                                    0.37 %       ±6.92%  ±9.21% ±12.01%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='whatwg' type='wpt'                                                      2.01 %       ±5.71%  ±7.61%  ±9.95%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='whatwg' type='ws'                                                       0.78 %       ±6.33%  ±8.42% ±10.96%
 url/url-format.js n=25000000 type='file'                                                                         ***     -5.13 %       ±1.35%  ±1.80%  ±2.34%
 url/url-format.js n=25000000 type='slashes'                                                                               1.89 %       ±2.31%  ±3.08%  ±4.01%
 url/url-parse.js n=10000000 type='escaped'                                                                                0.81 %       ±2.22%  ±2.96%  ±3.85%
 url/url-parse.js n=10000000 type='normal'                                                                                 0.13 %       ±2.83%  ±3.77%  ±4.90%
 url/url-resolve.js n=100000 path='down' href='auth'                                                                       1.63 %       ±4.99%  ±6.64%  ±8.64%
 url/url-resolve.js n=100000 path='down' href='dot'                                                                        0.87 %       ±2.08%  ±2.77%  ±3.60%
 url/url-resolve.js n=100000 path='down' href='file'                                                                       1.96 %       ±2.37%  ±3.16%  ±4.12%
 url/url-resolve.js n=100000 path='down' href='idn'                                                                 *      5.19 %       ±4.53%  ±6.06%  ±7.94%
 url/url-resolve.js n=100000 path='down' href='javascript'                                                        ***      3.56 %       ±1.95%  ±2.61%  ±3.41%
 url/url-resolve.js n=100000 path='down' href='long'                                                                      -0.42 %       ±2.09%  ±2.78%  ±3.62%
 url/url-resolve.js n=100000 path='down' href='noscheme'                                                                  -1.97 %       ±2.15%  ±2.86%  ±3.72%
 url/url-resolve.js n=100000 path='down' href='percent'                                                                   -0.73 %       ±4.31%  ±5.77%  ±7.57%
 url/url-resolve.js n=100000 path='down' href='short'                                                                      1.48 %       ±2.33%  ±3.10%  ±4.04%
 url/url-resolve.js n=100000 path='down' href='ws'                                                                         0.66 %       ±2.13%  ±2.83%  ±3.68%
 url/url-resolve.js n=100000 path='foo/bar' href='auth'                                                                    1.82 %       ±3.68%  ±4.90%  ±6.40%
 url/url-resolve.js n=100000 path='foo/bar' href='dot'                                                                    -0.83 %       ±4.57%  ±6.13%  ±8.07%
 url/url-resolve.js n=100000 path='foo/bar' href='file'                                                             *      6.01 %       ±5.64%  ±7.57% ±10.00%
 url/url-resolve.js n=100000 path='foo/bar' href='idn'                                                                     1.09 %       ±4.21%  ±5.63%  ±7.40%
 url/url-resolve.js n=100000 path='foo/bar' href='javascript'                                                      **      4.00 %       ±2.63%  ±3.50%  ±4.56%
 url/url-resolve.js n=100000 path='foo/bar' href='long'                                                                    1.98 %       ±2.97%  ±3.96%  ±5.16%
 url/url-resolve.js n=100000 path='foo/bar' href='noscheme'                                                        **     -3.04 %       ±1.77%  ±2.37%  ±3.11%
 url/url-resolve.js n=100000 path='foo/bar' href='percent'                                                                 3.91 %       ±4.93%  ±6.63%  ±8.76%
 url/url-resolve.js n=100000 path='foo/bar' href='short'                                                            *      5.69 %       ±5.47%  ±7.33%  ±9.63%
 url/url-resolve.js n=100000 path='foo/bar' href='ws'                                                                      4.13 %       ±6.47%  ±8.64% ±11.30%
 url/url-resolve.js n=100000 path='sibling' href='auth'                                                                   -1.32 %       ±3.45%  ±4.59%  ±5.97%
 url/url-resolve.js n=100000 path='sibling' href='dot'                                                                     2.29 %       ±5.88%  ±7.87% ±10.36%
 url/url-resolve.js n=100000 path='sibling' href='file'                                                                    1.98 %       ±4.04%  ±5.40%  ±7.06%
 url/url-resolve.js n=100000 path='sibling' href='idn'                                                                     2.61 %       ±3.22%  ±4.29%  ±5.59%
 url/url-resolve.js n=100000 path='sibling' href='javascript'                                                       *      5.84 %       ±4.42%  ±5.94%  ±7.84%
 url/url-resolve.js n=100000 path='sibling' href='long'                                                                   -2.23 %       ±3.24%  ±4.31%  ±5.62%
 url/url-resolve.js n=100000 path='sibling' href='noscheme'                                                         *     -1.57 %       ±1.38%  ±1.84%  ±2.39%
 url/url-resolve.js n=100000 path='sibling' href='percent'                                                                 3.67 %       ±6.53%  ±8.69% ±11.31%
 url/url-resolve.js n=100000 path='sibling' href='short'                                                                  -0.80 %       ±3.18%  ±4.25%  ±5.57%
 url/url-resolve.js n=100000 path='sibling' href='ws'                                                                     -3.78 %       ±3.90%  ±5.20%  ±6.78%
 url/url-resolve.js n=100000 path='up' href='auth'                                                                         2.88 %       ±4.43%  ±5.92%  ±7.76%
 url/url-resolve.js n=100000 path='up' href='dot'                                                                          0.38 %       ±2.31%  ±3.08%  ±4.01%
 url/url-resolve.js n=100000 path='up' href='file'                                                                         3.15 %       ±3.49%  ±4.68%  ±6.16%
 url/url-resolve.js n=100000 path='up' href='idn'                                                                         -1.19 %       ±3.21%  ±4.28%  ±5.59%
 url/url-resolve.js n=100000 path='up' href='javascript'                                                           **      3.49 %       ±2.31%  ±3.07%  ±4.00%
 url/url-resolve.js n=100000 path='up' href='long'                                                                        -0.53 %       ±3.42%  ±4.57%  ±6.00%
 url/url-resolve.js n=100000 path='up' href='noscheme'                                                                    -3.24 %       ±4.38%  ±5.83%  ±7.60%
 url/url-resolve.js n=100000 path='up' href='percent'                                                                      1.72 %       ±2.54%  ±3.38%  ±4.40%
 url/url-resolve.js n=100000 path='up' href='short'                                                                        1.92 %       ±4.56%  ±6.06%  ±7.89%
 url/url-resolve.js n=100000 path='up' href='ws'                                                                          -4.62 %       ±5.47%  ±7.34%  ±9.68%
 url/url-resolve.js n=100000 path='withscheme' href='auth'                                                                 1.93 %       ±4.82%  ±6.48%  ±8.56%
 url/url-resolve.js n=100000 path='withscheme' href='dot'                                                                  4.20 %       ±4.89%  ±6.52%  ±8.50%
 url/url-resolve.js n=100000 path='withscheme' href='file'                                                        ***      8.63 %       ±4.54%  ±6.09%  ±8.03%
 url/url-resolve.js n=100000 path='withscheme' href='idn'                                                                 -0.34 %       ±5.52%  ±7.35%  ±9.58%
 url/url-resolve.js n=100000 path='withscheme' href='javascript'                                                  ***      4.44 %       ±1.93%  ±2.57%  ±3.35%
 url/url-resolve.js n=100000 path='withscheme' href='long'                                                        ***      8.57 %       ±4.68%  ±6.24%  ±8.15%
 url/url-resolve.js n=100000 path='withscheme' href='noscheme'                                                             1.18 %       ±2.74%  ±3.68%  ±4.83%
 url/url-resolve.js n=100000 path='withscheme' href='percent'                                                     ***      8.06 %       ±3.64%  ±4.85%  ±6.33%
 url/url-resolve.js n=100000 path='withscheme' href='short'                                                       ***      6.61 %       ±3.05%  ±4.08%  ±5.37%
 url/url-resolve.js n=100000 path='withscheme' href='ws'                                                                   2.04 %       ±2.53%  ±3.37%  ±4.40%
 url/url-searchparams-iteration.js n=1000000 loopMethod='forEach'                                                         -0.55 %       ±2.71%  ±3.60%  ±4.69%
 url/url-searchparams-iteration.js n=1000000 loopMethod='iterator'                                                         0.78 %       ±2.72%  ±3.62%  ±4.71%
 url/url-searchparams-read.js n=20000000 param='nonexistent' accessMethod='get'                                           -0.58 %       ±4.56%  ±6.08%  ±7.96%
 url/url-searchparams-read.js n=20000000 param='nonexistent' accessMethod='getAll'                                        -1.67 %       ±2.24%  ±3.00%  ±3.95%
 url/url-searchparams-read.js n=20000000 param='nonexistent' accessMethod='has'                                           -0.52 %       ±2.90%  ±3.85%  ±5.01%
 url/url-searchparams-read.js n=20000000 param='one' accessMethod='get'                                                   -0.52 %       ±1.88%  ±2.51%  ±3.26%
 url/url-searchparams-read.js n=20000000 param='one' accessMethod='getAll'                                                 0.62 %       ±1.25%  ±1.67%  ±2.17%
 url/url-searchparams-read.js n=20000000 param='one' accessMethod='has'                                                    1.95 %       ±4.45%  ±5.98%  ±7.92%
 url/url-searchparams-read.js n=20000000 param='three' accessMethod='get'                                                 -2.26 %       ±3.81%  ±5.07%  ±6.61%
 url/url-searchparams-read.js n=20000000 param='three' accessMethod='getAll'                                              -0.32 %       ±1.82%  ±2.42%  ±3.15%
 url/url-searchparams-read.js n=20000000 param='three' accessMethod='has'                                                 -0.89 %       ±4.28%  ±5.69%  ±7.41%
 url/url-searchparams-read.js n=20000000 param='two' accessMethod='get'                                                   -2.43 %       ±3.14%  ±4.21%  ±5.54%
 url/url-searchparams-read.js n=20000000 param='two' accessMethod='getAll'                                                -1.39 %       ±1.70%  ±2.26%  ±2.95%
 url/url-searchparams-read.js n=20000000 param='two' accessMethod='has'                                            **     -2.93 %       ±1.74%  ±2.32%  ±3.02%
 url/url-searchparams-sort.js n=1000000 type='almostsorted'                                                                0.71 %       ±2.12%  ±2.83%  ±3.70%
 url/url-searchparams-sort.js n=1000000 type='empty'                                                                       3.66 %       ±5.33%  ±7.11%  ±9.29%
 url/url-searchparams-sort.js n=1000000 type='long'                                                                        1.11 %       ±2.21%  ±2.96%  ±3.88%
 url/url-searchparams-sort.js n=1000000 type='random'                                                                      0.69 %       ±2.24%  ±2.98%  ±3.89%
 url/url-searchparams-sort.js n=1000000 type='reversed'                                                                    0.02 %       ±1.45%  ±1.93%  ±2.51%
 url/url-searchparams-sort.js n=1000000 type='short'                                                                      -0.10 %       ±3.74%  ±5.00%  ±6.55%
 url/url-searchparams-sort.js n=1000000 type='sorted'                                                                      0.31 %       ±3.45%  ±4.60%  ±6.02%
 url/url-searchparams-sort.js n=1000000 type='wpt'                                                                         1.89 %       ±4.22%  ±5.61%  ±7.31%
 url/usvstring.js n=50000000 input='allinvalid'                                                                           -1.16 %       ±1.59%  ±2.13%  ±2.80%
 url/usvstring.js n=50000000 input='nonstring'                                                                             0.32 %       ±3.45%  ±4.60%  ±5.99%
 url/usvstring.js n=50000000 input='someinvalid'                                                                          -0.32 %       ±0.75%  ±1.00%  ±1.30%
 url/usvstring.js n=50000000 input='valid'                                                                                -0.75 %       ±2.44%  ±3.24%  ±4.22%
 url/usvstring.js n=50000000 input='validsurr'                                                                            -0.73 %       ±0.99%  ±1.32%  ±1.71%
 url/whatwg-url-idna.js n=5000000 to='ascii' domain='all'                                                                  0.28 %       ±3.27%  ±4.36%  ±5.67%
 url/whatwg-url-idna.js n=5000000 to='ascii' domain='empty'                                                               -4.70 %       ±6.19%  ±8.26% ±10.80%
 url/whatwg-url-idna.js n=5000000 to='ascii' domain='none'                                                                -0.79 %       ±2.07%  ±2.75%  ±3.59%
 url/whatwg-url-idna.js n=5000000 to='ascii' domain='nonstring'                                                           -0.43 %       ±2.67%  ±3.56%  ±4.65%
 url/whatwg-url-idna.js n=5000000 to='ascii' domain='some'                                                                 0.48 %       ±2.40%  ±3.20%  ±4.16%
 url/whatwg-url-idna.js n=5000000 to='unicode' domain='all'                                                               -0.05 %       ±0.88%  ±1.18%  ±1.53%
 url/whatwg-url-idna.js n=5000000 to='unicode' domain='empty'                                                              2.78 %       ±5.31%  ±7.09%  ±9.30%
 url/whatwg-url-idna.js n=5000000 to='unicode' domain='none'                                                               0.22 %       ±2.74%  ±3.65%  ±4.75%
 url/whatwg-url-idna.js n=5000000 to='unicode' domain='nonstring'                                                          1.72 %       ±2.63%  ±3.51%  ±4.56%
 url/whatwg-url-idna.js n=5000000 to='unicode' domain='some'                                                               0.42 %       ±1.26%  ±1.68%  ±2.20%
 url/whatwg-url-properties.js prop='hash' e=1 type='wpt' withBase='false'                                                  0.24 %       ±5.53%  ±7.35%  ±9.57%
 url/whatwg-url-properties.js prop='hash' e=1 type='wpt' withBase='true'                                                  -6.65 %       ±7.63% ±10.15% ±13.21%
 url/whatwg-url-properties.js prop='host' e=1 type='wpt' withBase='false'                                                  1.27 %       ±3.93%  ±5.23%  ±6.80%
 url/whatwg-url-properties.js prop='host' e=1 type='wpt' withBase='true'                                                   0.04 %       ±7.28%  ±9.70% ±12.65%
 url/whatwg-url-properties.js prop='hostname' e=1 type='wpt' withBase='false'                                             -3.24 %       ±6.84%  ±9.13% ±11.92%
 url/whatwg-url-properties.js prop='hostname' e=1 type='wpt' withBase='true'                                              -0.56 %       ±6.71%  ±8.94% ±11.66%
 url/whatwg-url-properties.js prop='href' e=1 type='wpt' withBase='false'                                                 -0.94 %       ±4.89%  ±6.51%  ±8.48%
 url/whatwg-url-properties.js prop='href' e=1 type='wpt' withBase='true'                                                  -3.40 %       ±7.63% ±10.15% ±13.22%
 url/whatwg-url-properties.js prop='origin' e=1 type='wpt' withBase='false'                                               -0.36 %       ±7.54% ±10.04% ±13.07%
 url/whatwg-url-properties.js prop='origin' e=1 type='wpt' withBase='true'                                                -5.37 %       ±7.29%  ±9.70% ±12.63%
 url/whatwg-url-properties.js prop='password' e=1 type='wpt' withBase='false'                                             -1.59 %       ±4.22%  ±5.63%  ±7.35%
 url/whatwg-url-properties.js prop='password' e=1 type='wpt' withBase='true'                                               1.26 %       ±8.33% ±11.10% ±14.48%
 url/whatwg-url-properties.js prop='pathname' e=1 type='wpt' withBase='false'                                              1.72 %       ±4.95%  ±6.61%  ±8.67%
 url/whatwg-url-properties.js prop='pathname' e=1 type='wpt' withBase='true'                                              -3.37 %       ±8.74% ±11.63% ±15.14%
 url/whatwg-url-properties.js prop='port' e=1 type='wpt' withBase='false'                                                  3.10 %       ±6.05%  ±8.05% ±10.48%
 url/whatwg-url-properties.js prop='port' e=1 type='wpt' withBase='true'                                                   7.47 %       ±9.21% ±12.26% ±15.96%
 url/whatwg-url-properties.js prop='protocol' e=1 type='wpt' withBase='false'                                             -3.92 %       ±5.21%  ±6.93%  ±9.03%
 url/whatwg-url-properties.js prop='protocol' e=1 type='wpt' withBase='true'                                              -2.14 %       ±7.93% ±10.56% ±13.74%
 url/whatwg-url-properties.js prop='search' e=1 type='wpt' withBase='false'                                               -3.06 %       ±6.39%  ±8.50% ±11.08%
 url/whatwg-url-properties.js prop='search' e=1 type='wpt' withBase='true'                                                -1.01 %       ±6.44%  ±8.57% ±11.16%
 url/whatwg-url-properties.js prop='searchParams' e=1 type='wpt' withBase='false'                                         -3.26 %       ±4.76%  ±6.34%  ±8.25%
 url/whatwg-url-properties.js prop='searchParams' e=1 type='wpt' withBase='true'                                           2.91 %       ±8.22% ±10.95% ±14.29%
 url/whatwg-url-properties.js prop='username' e=1 type='wpt' withBase='false'                                      **      7.86 %       ±4.88%  ±6.51%  ±8.51%
 url/whatwg-url-properties.js prop='username' e=1 type='wpt' withBase='true'                                              -0.37 %       ±7.47%  ±9.94% ±12.96%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case there are 235 comparisons, you can thus
expect the following amount of false-positive results:
  11.75 false positives, when considering a   5% risk acceptance (*, **, ***),
  2.35 false positives, when considering a   1% risk acceptance (**, ***),
  0.24 false positives, when considering a 0.1% risk acceptance (***)
Notifying upstream projects of job completion
Finished: SUCCESS

Significant impact
                                                                                                           confidence improvement accuracy (*)    (**)   (***)
 url/legacy-vs-whatwg-url-parse.js method='legacy' e=1 type='wpt' withBase='true'                                   *     -5.31 %       ±4.38%  ±5.83%  ±7.60%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='dot' withBase='false'                                  *     -6.11 %       ±5.94%  ±7.93% ±10.37%
 url/legacy-vs-whatwg-url-parse.js method='whatwg' e=1 type='long' withBase='false'                                 *      8.84 %       ±8.76% ±11.71% ±15.35%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='multivaluemany'              *     -1.32 %       ±1.15%  ±1.53%  ±1.99%
 url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='legacy' searchParam='noencode'                    *     -2.09 %       ±1.83%  ±2.45%  ±3.20%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='legacy' searchParam='altspaces'               *     -3.32 %       ±3.26%  ±4.38%  ±5.76%
 url/legacy-vs-whatwg-url-searchparams-serialize.js n=1000000 method='whatwg' searchParam='encodefake'              *     -1.15 %       ±0.88%  ±1.17%  ±1.52%
 url/legacy-vs-whatwg-url-serialize.js e=1 method='legacy' type='percent'                                           *      7.46 %       ±5.91%  ±7.90% ±10.36%
 url/url-format.js n=25000000 type='file'                                                                         ***     -5.13 %       ±1.35%  ±1.80%  ±2.34%
 url/url-resolve.js n=100000 path='down' href='idn'                                                                 *      5.19 %       ±4.53%  ±6.06%  ±7.94%
 url/url-resolve.js n=100000 path='down' href='javascript'                                                        ***      3.56 %       ±1.95%  ±2.61%  ±3.41%
 url/url-resolve.js n=100000 path='foo/bar' href='file'                                                             *      6.01 %       ±5.64%  ±7.57% ±10.00%
 url/url-resolve.js n=100000 path='foo/bar' href='javascript'                                                      **      4.00 %       ±2.63%  ±3.50%  ±4.56%
 url/url-resolve.js n=100000 path='foo/bar' href='noscheme'                                                        **     -3.04 %       ±1.77%  ±2.37%  ±3.11%
 url/url-resolve.js n=100000 path='foo/bar' href='short'                                                            *      5.69 %       ±5.47%  ±7.33%  ±9.63%
 url/url-resolve.js n=100000 path='sibling' href='javascript'                                                       *      5.84 %       ±4.42%  ±5.94%  ±7.84%
 url/url-resolve.js n=100000 path='sibling' href='noscheme'                                                         *     -1.57 %       ±1.38%  ±1.84%  ±2.39%
 url/url-resolve.js n=100000 path='up' href='javascript'                                                           **      3.49 %       ±2.31%  ±3.07%  ±4.00%
 url/url-resolve.js n=100000 path='withscheme' href='file'                                                        ***      8.63 %       ±4.54%  ±6.09%  ±8.03%
 url/url-resolve.js n=100000 path='withscheme' href='javascript'                                                  ***      4.44 %       ±1.93%  ±2.57%  ±3.35%
 url/url-resolve.js n=100000 path='withscheme' href='long'                                                        ***      8.57 %       ±4.68%  ±6.24%  ±8.15%
 url/url-resolve.js n=100000 path='withscheme' href='percent'                                                     ***      8.06 %       ±3.64%  ±4.85%  ±6.33%
 url/url-resolve.js n=100000 path='withscheme' href='short'                                                       ***      6.61 %       ±3.05%  ±4.08%  ±5.37%
 url/url-searchparams-read.js n=20000000 param='two' accessMethod='has'                                            **     -2.93 %       ±1.74%  ±2.32%  ±3.02%
 url/whatwg-url-properties.js prop='username' e=1 type='wpt' withBase='false'                                      **      7.86 %       ±4.88%  ±6.51%  ±8.51%

BridgeAR pushed a commit that referenced this pull request Dec 5, 2018
Avoids a maintenance hazard when reviewers assume that
`hostlessProtocol` and `slashedProtocol` are disjoint.

The following may be counter-intuitive:

```js
// These objects seem to have no keys in common
const hostlessProtocol = { 'javascript': true };
const slashedProtocol = { 'http': true };
// A reasonable reviewer may assumes bothTrue is never truthy
function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}
// But
console.log(Boolean(bothTrue('constructor')));  // true
```

This change uses SafeSet instead of plain-old objects.

----

Rejected alternative:

We could have used object with a `null` prototype as lookup tables
so that `lowerProto` is never treated as a key into `Object.prototype`.

```js
const hostlessProtocol = { __proto__: null, 'javascript': true };
const slashedProtocol = { __proto__: null, 'http': true };

function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}

console.log(Boolean(bothTrue('constructor')));  // false
```

PR-URL: #24703
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Dec 5, 2018
4 tasks
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
Avoids a maintenance hazard when reviewers assume that
`hostlessProtocol` and `slashedProtocol` are disjoint.

The following may be counter-intuitive:

```js
// These objects seem to have no keys in common
const hostlessProtocol = { 'javascript': true };
const slashedProtocol = { 'http': true };
// A reasonable reviewer may assumes bothTrue is never truthy
function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}
// But
console.log(Boolean(bothTrue('constructor')));  // true
```

This change uses SafeSet instead of plain-old objects.

----

Rejected alternative:

We could have used object with a `null` prototype as lookup tables
so that `lowerProto` is never treated as a key into `Object.prototype`.

```js
const hostlessProtocol = { __proto__: null, 'javascript': true };
const slashedProtocol = { __proto__: null, 'http': true };

function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}

console.log(Boolean(bothTrue('constructor')));  // false
```

PR-URL: nodejs#24703
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
Avoids a maintenance hazard when reviewers assume that
`hostlessProtocol` and `slashedProtocol` are disjoint.

The following may be counter-intuitive:

```js
// These objects seem to have no keys in common
const hostlessProtocol = { 'javascript': true };
const slashedProtocol = { 'http': true };
// A reasonable reviewer may assumes bothTrue is never truthy
function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}
// But
console.log(Boolean(bothTrue('constructor')));  // true
```

This change uses SafeSet instead of plain-old objects.

----

Rejected alternative:

We could have used object with a `null` prototype as lookup tables
so that `lowerProto` is never treated as a key into `Object.prototype`.

```js
const hostlessProtocol = { __proto__: null, 'javascript': true };
const slashedProtocol = { __proto__: null, 'http': true };

function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}

console.log(Boolean(bothTrue('constructor')));  // false
```

PR-URL: #24703
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Feb 12, 2019
rvagg pushed a commit that referenced this pull request Feb 28, 2019
Avoids a maintenance hazard when reviewers assume that
`hostlessProtocol` and `slashedProtocol` are disjoint.

The following may be counter-intuitive:

```js
// These objects seem to have no keys in common
const hostlessProtocol = { 'javascript': true };
const slashedProtocol = { 'http': true };
// A reasonable reviewer may assumes bothTrue is never truthy
function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}
// But
console.log(Boolean(bothTrue('constructor')));  // true
```

This change uses SafeSet instead of plain-old objects.

----

Rejected alternative:

We could have used object with a `null` prototype as lookup tables
so that `lowerProto` is never treated as a key into `Object.prototype`.

```js
const hostlessProtocol = { __proto__: null, 'javascript': true };
const slashedProtocol = { __proto__: null, 'http': true };

function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}

console.log(Boolean(bothTrue('constructor')));  // false
```

PR-URL: #24703
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants