Possibly invalid checks for WHATWG URL in https.request

[JonathanWilbur]

• Version: v8.10.0 (The AWS runtime used in local Serverless Framework invocations.)
• Platform: Windows Professional, 64-Bit
• Subsystem: lib/internal/url.js

First off, I find it questionable that this is a bug at all; I am not partial to this getting resolved. I just wanted developers to take a second look at this to ensure that it is proper.

I ran into an unusual situation where a third-party library would attempt to clone a WHATWG URL object with Object.assign(), and since the properties like hostname or path are getters, they are not copied. (Actually, the MDN documentation says this is supposed to happen, but that is obviously what is happening.) The resulting clone, therefore, has no properties except the enumerable symbols of the URL class: URLContext and URLSearchParams. This makes it pass these checks in https.js to determine if the argument is of type URL:

  } else if (args[0] && args[0][searchParamsSymbol] &&
             args[0][searchParamsSymbol][searchParamsSymbol]) {
    // url.URL instance
    options = urlToOptions(args.shift());
  }

Then, when urlToOptions() is called with this deceptive object, a TypeError is thrown: TypeError: Cannot read property 'startsWith' of undefined. See urlToOptions() below:

function urlToOptions(url) {
  var options = {
    protocol: url.protocol,
    hostname: url.hostname.startsWith('[') ?
      url.hostname.slice(1, -1) :
      url.hostname,
    hash: url.hash,
    search: url.search,
    pathname: url.pathname,
    path: `${url.pathname}${url.search}`,
    href: url.href
  };
  if (url.port !== '') {
    options.port = Number(url.port);
  }
  if (url.username || url.password) {
    options.auth = `${url.username}:${url.password}`;
  }
  return options;
}

My question, then, is: is it proper for a function in the standard library to throw a TypeError as seen above? Should there be checks to ensure that all of the necessary properties are present in url before continuing to use them?

[JonathanWilbur]

If you have further questions for me, it may be instructive to see the bug I wrote up for the particular third-party library: TooTallNate/node-agent-base#26.

[lpinca]

I think we should have better validation to make sure that a proper URL instance is passed to it.

[lpinca]

Fixed by #26226.