Skip to content

url: use SafeSet to filter known special protocols

Rodrigo Muino Tomonari requested to merge github/fork/mikesamuel/patch-1 into master
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

Merge request reports

Loading