Customize Listen Address

[mroethke]

This allows to specify the listen address on the cli. I Have also changed the default to localhost because in my opinion the default should never be 0.0.0.0. But I will change it back if necessary.

I have also narrowed the data type for the port from usize to u16. This change slightly improves the error message if an out of range port number is specified

[djmitche]

I'm not sure why I chose usize for a port!

I appreciate the addition of --listen. And, I take your point regarding the default. However, the change in default would be disruptive to users. I don't think 0.0.0.0 is such a bad default that the disruption is justified. For example, it would break the current docker-based deployment.

So, please restore that to default to 0.0.0.0 and add a note about the default in the help output. It is probably also worth adding mention of the new option to README.md. Finally, if there is an update to docker-compose.yml that would work for this purpose, please make that change as well.

[djmitche]

(and formatting, of course!)

[ryneeverett]

I appreciate the addition of --listen. And, I take your point regarding the default. However, the change in default would be disruptive to users. I don't think 0.0.0.0 is such a bad default that the disruption is justified. For example, it would break the current docker-based deployment.

Are there really so many users that we're already locked into a suboptimal default at v0.5.0? Furthermore, wouldn't it only affect users who are running the sync server on a private network without a reverse proxy (possibly nobody at all) and users who are deploying it on a public network without a tls reverse proxy (who might benefit from being disrupted). We could make the change obvious by making --listen a required parameter until v1.0.0.

[djmitche]

I remain unconvinced that all-interfaces is a particularly bad default. However, I'm happy to be overridden by the majority here!

Making the parameter required (in which case --port should be required as well) seems like a good way to make this default change un-missable for users.

[mroethke]

Listening on all interfaces by default brought us years of leaks from a certain database. And this project is still sub 1.0 so it is not entirely unexpected that things break occasionally.

I'm not against making --listen required, but what would we gain? Users that depend on it listening on all interfaces will notice either way, others don't care.

[mroethke]

I have only fixed the style issues, the docker-compose file and added the option to the readme for now.

[djmitche]

127.0.0.1 as a default for the listen address is essentially never useful -- it's functionally equivalent to not listening at all, meaning that the default is for the server to run but not actually serve. That could take admins some debugging when upgrading the sever, and as we've seen often upgrades "just happen" e.g., via distros or via dependency ranges.

I'd prefer to have the option fail "loudly" in this new version, rather than leaving admins looking for a cause.

[ryneeverett]

I'm not against making --listen required, but what would we gain? Users that depend on it listening on all interfaces will notice either way, others don't care.

I agree with Dustin here that we want it to fail loudly. IMO the distinction is that by making --listen required, the failure occurs on the machine hosting taskchampion-sync-server rather than on the machines running task sync where the cause of the failure might not be obvious.

127.0.0.1 as a default for the listen address is essentially never useful -- it's functionally equivalent to not listening at all, meaning that the default is for the server to run but not actually serve.

😕 The README recommends using a reverse proxy and in that case one would generally only listen on localhost unless one is using network namespaces such as via docker.

[djmitche]

😕 The README recommends using a reverse proxy and in that case one would generally only listen on localhost unless one is using network namespaces such as via docker.

Ok, fair point.

[mroethke]

Alright. One more idea: Since we are breaking things anyway, how about getting rid of --port and specifying the port together with the interface via --listen? e.g. --listen localhost:8080

[djmitche]

That sounds good to me!

[mroethke]

done

[djmitche]

Very nice! Thanks!