Introduce Local Port Routing Builder #1392
Conversation
hosamaly
changed the title
|
I dont think its correct to check that the port is outside an ephemeral port range. I believe the range is configurable and different on many systems. Also a separate pr maybe but I feel an ip version of the selector ought to be there also. |
|
Thanks for the feedback, @spand. I'm happy to remove the check for ephemeral ports; perhaps it was too specific to my design decisions. I'll change this now. As for the IP version, I agree that it's better left to a separate PR. |
See discussion at: ktorio#1392 (comment)
|
Hi, @spand. How does this PR look now from your perspective? |
| */ | ||
| @KtorExperimentalAPI | ||
| fun Route.localPort(port: Int, build: Route.() -> Unit): Route { | ||
| require(port in 1..49151) { "Port $port must be a positive number within the range of non-ephemeral ports" } | ||
| require(port in 1..65535) { "Port $port must be a positive number between 1 and 65,535" } |
There was a problem hiding this comment.
Entering bike shedding territory here but afaik 0 is also valid. A quick look in InetSocketAddress has:
private static int checkPort(int port) {
if (port < 0 || port > 0xFFFF)
throw new IllegalArgumentException("port out of range:" + port);
return port;
}
There was a problem hiding this comment.
According to Wikipedia, 0 is reserved. It also adds that:
in programming APIs (not in communication between hosts), [it] requests a system-allocated (dynamic) port
That wouldn't be useful as a filter.
|
I put in a comment but looks good to me.. though I am just a nobody in this project :) |
* Introduce Local Port Route Builder See discussion at: #1392 (comment)
Subsystem
ktor-server
Motivation
I'm building an application that has some functions that should be exposed on a separate port (e.g. metrics). The existing (experimental)
HostRouteSelector(introduced in #835) is geared towards multi-tenant/VirtualHost-style configurations, allowing a client to send aHostheader to get information that perhaps shouldn't be exposed to them.Solution
I suggest a simple
RouteSelectorbased on thelocal: RequestConnectionPointport, whose documentation says (emphasis mine):Sadly, some implementations don't abide by this rule. Particularly noteworthy is the
TestApplicationRequest, which extracts the port from headers. I think that it should be changed to enable setting the port in the request builder, but this could be left for another PR.