dont listen on all devices

[yoshuawuyts]

This makes it so we don't listen on all interfaces, which
prevents some security issues.

First reported here:

https://twitter.com/badboy_/status/1003266217803730945

Thanks!

[kamalmarhubi]

This change seems wrong to me. It makes this crate only useful for applications that only listen locally, which is—at least for me—a minority?

I'm wondering if the better thing to do is make this actually be a BindAddress type that adds --port and --address flags.

[killercup]

I agree it is a better default, but also a pretty big blocker for production use, as @kamalmarhubi already mentioned.

Can we simply add an "address" field of type SocketAddr? This is not optimal (because instead of rather typtical --port 8080 --host 0.0.0.0 you'd need to use --address 0.0.0.0:8080) but should work for now while we figure out how to represent the other variants in structopt.

[yoshuawuyts]

I've added a --hostname flag per the WhatWG URL spec, defaulting to 127.0.0.1 which is the same as localhost as per the upcoming Ipv4Addr::localhost() method.

This is different from --address which was proposed, but I thought it'd closer reflect what we meant exactly (good overview about the different pieces that make up a URL here).

Usage would be:

$ ./main --port 8080 --hostname 0.0.0.0
$ ./main --port 8080 -H 0.0.0.0
$ ./main --port 8080   # defaults to 127.0.0.1

Unresolved Questions

--hostname isn't part of any match group, but instead relies on a default setting. Ideally we could make it mutually exclusive with LISTEN_FD. If both LISTEN_FD and --hostname are set, --hostname is ignored. I feel the chance that this might come to bite someone is relatively low, but regardless we should patch this once structopt has the capability to handle this scenario.

Other Changes

• included a reading section for the specs
• updated the example to accept the Result type, which adds less noise when testing out error handling.

[killercup]

s/Was/was

[killercup]

This PR is good. The following does not need to be addressed here but I'd like to see it addressed (implemented or dismissed) in a future PR though :)

• Calling it --hostname and passing an IP seems weirds to me, but is not a big deal.
• I'd also add documentation saying that the fd takes precedence over the hostname if both are present (IIUC)
• maybe a doc comments on the fields?
• in these doc comments i'd add that the hostname field can be any string that can be parsed to an IpAddr, including IPv6 addresses (that we default to IPv4 localhost is fine though)

[yoshuawuyts]

Calling it --hostname and passing an IP seems weirds to me, but is not a big deal.

Heh yeah I guess that's fair. But if you think of domain names as fancy aliases for IP addresses it probably makes more sense. It does adhere to the url spec, so we can outsource the weirdness 😛.

maybe a doc comments on the fields?

Oh yeah, good one!

in these doc comments i'd add that the hostname field can be any string that can be parsed to an IpAddr, including IPv6 addresses (that we default to IPv4 localhost is fine though)

Yeah, I agree that's important information - but typically this is information I'd like to see in a man page, and less so in the --help output. Too much output makes it hard to read, which kind of defeats the purpose.

Right now I'm not sure how to hide information in doc comments from appearing in --help output, so I'm leaving it out for now. Opened an issue to remind ourselves #6.

[yoshuawuyts]

Implemented all feedback! 🎉

[kamalmarhubi]

This is possibly too late, but re address vs hostname: my suggestion of address was intentional. This isn't a URL, and address is the name used in the socket API and the TCP RFC.