net: support passing a host with port to Server.prototype.listen

[Zirak]

Distinguish between a host and hostname in the
Server.prototype.listen argument, to better align with how browsers
and other places in node distinguish between host and hostname.

For more context, see #16712.

This commit:

• Broadens the meaning of the host key to also potentially include a port (i.e. hostname:port)
  • Given such a host option, one can choose to not supply a port parameter
• Allows the user to pass in a hostname key, which behaves just like host in present-day node.

The following three are now equivalent:

server.listen({
  host: 'localhost:5000',
})

server.listen({
  host: 'localhost',
  port: 5000,
})

server.listen({
  hostname: 'localhost',
  port: 5000
})

server.listen(new URL('http://localhost:5000'))

Backwards-compatibility

An important aspect was keeping this change backwards-compatible. As such, the host option is parsed into its hostname and port components.

In case of a conflict between a host component and one of the other options, an exception is thrown:

server.listen({
  host: 'localhost:5000',
  hostname: '127.0.0.1'
}) // => TypeError

server.listen({
  host: 'localhost:5000',
  port: 8080
}) // => TypeError

Caveats

My goal here was to mock-up a PoC which implements the spirit of the
proposal in a backwards-compatible matter. Below are some caveats with
how I wrote the code and some tradeoffs I took to keep this a PoC. If
this proves to be interesting to the project, they'll be fixed, no
worries :)

• Right now, host is parsed using a hack around new URL. It's
  probably not the smartest way of going about this. Regard it as
  temporary until this gets more traction.

• Similarly, the new errors thrown here should be internal and worded
  better overall. So far they serve as an example.

• Since ipv6 addresses in urls must be enclosed in square brackets
  (i.e. [80:fe::]), and the rest of the code expects them to be free
  outside their cages, we need to trim those. To maintain current
  behaviour, we need to only trim those if the value inside is an ipv6
  address. Current node:

net.createServer().listen({ host: '[foo].com', port: 0 });

After a tick throws Error: getaddrinfo ENOTFOUND [foo].com. Having
said that, new URL('http://[foo].com') throws an exception, and
Firefox doesn't believes it's a real URL.

• This pollutes the options variable with an extra hostname key
  the user might not have specified. The options variable gets
  printed in some cases, and this may not be ideal.

  • The bespoken error message need also be updated.

• Right now, a known bug is when you pass a host containing a port and
  you also pass the port key as a string (i.e. listen({ host: 'whatever:123', port: '123' })), an error will be thrown about
  mismatching port values. One potential fix is moving the casting up,
  from the argument of lookupAndListen or listenInCluster to
  somewhere handling the arguments.

• The tests are fairly verbose and can be compacted. I opted to making
  them very explicit and long-form at this point for clarity.

• There are currently no tests for the lonely hostname option.

node questions

• Is there a reason to try and identify invalid IPv6 addresses (like
  1::2::3 or :::)? Currently, they fall back on trying to be
  resolved via dns and failing.

• Did I put the tests in the correct file? Is there a better place for
  it? e.g. test-net-server-address?

---

Sorry for the extra long PR comment, and thank you for your time.

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

[benjamingr]

This looks reasonable to me - @nodejs/http @nodejs/collaborators can this get some opinions/reviews?

[BridgeAR]

@Zirak is it intentional that this is kept as draft?

[bnoordhuis]

Rather than throwing TypeErrors directly, can you use the infrastructure from lib/internal/errors.js?

[Zirak]

Absolutely. As mentioned in the initial post, if this is an interesting change this is one of the things that will be addressed.

[Zirak]

@Zirak is it intentional that this is kept as draft?

So far I haven't seen any team-member approval, or that this PR is of any interest. If there is any, and the concerns in the main post are no problem, then I'll gladly pick it back up.

[ronkorving]

What's the ESLint rule the line below breaks?

[bnb]

@nodejs/http any chance we can get reviews on this? ❤️