http icon indicating copy to clipboard operation
http copied to clipboard

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

Open gtaban opened this issue 7 years ago • 6 comments

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 avatar Nov 08 '17 14:11 gtaban

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.

gtaban avatar Nov 08 '17 14:11 gtaban

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).

helje5 avatar Nov 08 '17 16:11 helje5

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).

seabaylea avatar Nov 08 '17 19:11 seabaylea

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 avatar Nov 08 '17 19:11 gtaban

Closing based on the above discussions.

gtaban avatar Nov 10 '17 22:11 gtaban

Referencing relevant comment from #81 :

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

carlbrown avatar Nov 10 '17 23:11 carlbrown