swift-server / http

:warning: Historical HTTP API - please use https://github.com/swift-server/async-http-client instead
Apache License 2.0
702 stars 48 forks source link

In HTTPServer specification, `Port:0` give error, should be `Port:nil` for kernel assigned ports #88

Open gtaban opened 7 years ago

gtaban commented 7 years ago

Based on comments from @helje5: Don't do port: 0, should raise an error. Do port: nil for kernel assigned, or a real port

gtaban commented 7 years ago

Based on discussions in #81

@seabaylea :

For 0 vs nil, the trade off is that allowing nil means using an Optional and having to do a whole bunch of nil checks. Rather than supporting 0 for random port (which is wrong as 0 is actually a reserved port), I think the question is whether there's a real need for kernel assigned port.

The kernel assign port is probably useful for testing, but is it required there (or for any other use cases)?

Helge:

Kernel assigned ports are very useful, I use them all the time for plenty of scenarios (backend servers, on-device per-app servers, etc.). It is more like the other way around - assigned ports are only really useful for front facing servers. And whether Swift will replace Apache/NGINX/your-load-balance-of-choice soon is yet to be seen ;-) I mean,. we don't even have sendfile support (!), so you need something to deliver static resources efficiently anyways.

helje5 commented 7 years ago

Just to be clear as this is missing from this issue: I don't care that much whether 0 or nil. But neither should be used, but addresses (which then may have similar issues).

seabaylea commented 7 years ago

I think we got down to staying with 0 where you're setting point (in the convenience case vs. a full address), because it means we don't have to deal with Optionals and nil checks, and there's precedent with what's done elsewhere (Node.js, Golang, etc).

gtaban commented 7 years ago

C programmers are used to using 0 for randomly assigned ports by bind, however I can see why having a 0 might be confusing to a Swift programmer, new to kernel programming. However, making 0 give errors can be confusing to the C programmer. I don't think the nil checks are that bad and they don't happen too frequently.

In all, given the changes proposed in #89 , this issue probably won't get us very much, so I propose we close it.

gtaban commented 7 years ago

Closing based on the above discussions.

carlbrown commented 7 years ago

Referencing relevant comment from #81 :

https://github.com/swift-server/http/pull/81#issuecomment-343612743