ktor-http: [URLBuilder] Specifying a protocol with a default port value overwrites an already specified port

[bradynpoulsen]

Overview

Library	Version
ktor-http	0.9.4

Given the following execution,

val url = URLBuilder().apply {
    takeFrom(getenv("MYSQL_URL")) // import from "mysql://user:password@localhost:34567/myDb"
    protocol = URLProtocol("mysql", 3306)
}.build()

Expected Outcome

I expected the default port to be used on the protocol only if a port was not explicitly specified

println(url) // "mysql://user:password@localhost:34567/myDb"

Actual Outcome

The port is always overwritten when a protocol is set

println(url) // "mysql://user:password@localhost/myDb" (implying port `3306`)

https://github.com/ktorio/ktor/blob/a65667a6f0ca98128c38025f989bad7cb6da4576/ktor-http/src/io/ktor/http/URLBuilder.kt#L16-L18

Suggested Resolution

https://github.com/ktorio/ktor/blob/a65667a6f0ca98128c38025f989bad7cb6da4576/ktor-http/src/io/ktor/http/URLProtocol.kt#L19

The default port on a URLProtocol that has not already been mapped is -1, represented by the following code:

val url = URLBuilder().apply {
    takeFrom(getenv("MYSQL_URL")) // import from "mysql://user:password@hostUsingDefaultPort/myDb"
}.build()

the following is built

println(url.port) // "-1"

Since this is the case, I propose that the port is only overwritten when the protocol would overwrite a port of -1

var protocol: URLProtocol by observable(protocol) { _, _, value -> 
    if (port == -1) {
        port = value.defaultPort 
    }
} 

Current Workaround

Currently, my solution is to build the Url beforehand and check if a port was specified after applying the protocol

val url = URLBuilder().apply {
    val envUrl = Url(getenv("MYSQL_URL")) // import from "mysql://user:password@localhost:34567/myDb"
    takeFrom(envUrl)
    protocol = URLProtocol("mysql", 3306)
    if (envUrl.port > 0) {
        port = envUrl.port
    }
}.build()

[e5l]

Hi, @bradynpoulsen. It's the expected behavior. If you specify the protocol by hand - it changes the URL port on the protocol default port. In other cases, you have to always explicitly set the port after the protocol switch.

You could just reorder the builder statements:

val url = URLBuilder().apply {
    protocol = URLProtocol("mysql", 3306)
    takeFrom(getenv("MYSQL_URL")) // import from "mysql://user:password@localhost:34567/myDb"
}.build()

[bradynpoulsen]

@e5l That usage would not work either. You still have to explicitly set the port because takeFrom will perform a protocol switch to a default port since a protocol is present in the parsed URL, meaning the port will be set to -1 in the case of MySQL. https://github.com/ktorio/ktor/blob/a65667a6f0ca98128c38025f989bad7cb6da4576/ktor-http/src/io/ktor/http/URLUtils.kt#L21-L24

On the other hand, if my proposal were in effect, when the takeFrom call applies the port from the parsed URL, because the port was set to 3306 by protocol = URLProtocol("mysql", 3306), it would not reset the port back to -1

[bradynpoulsen]

Here's an example of outputting the resulting port if you did not explicitly set the port

val url = URLBuilder().apply {
    protocol = URLProtocol("mysql", 3306)
    takeFrom("mysql://user:pass@host/mySchema")
}.build()
println(url.protocol) // outputs URLProtocol(name=mysql, defaultPort=-1)
println(url.port) // outputs -1

What's the point of even specifying the protocol with a default port if it's going to always wipe out an explicitly set port?

[bradynpoulsen]

Using my original example above (without explicitly defining a port in the takeFrom value) and the proposed changes being in effect:

val url = URLBuilder().apply {
    takeFrom("mysql://user:pass@host/mySchema")
    println(port) // outputs -1
    protocol = URLProtocol("mysql", 3306) // sets the port because it is currently -1 (aka, not defined)
}.build()
println(url.protocol) // outputs URLProtocol(name=mysql, defaultPort=3306)
println(url.port) // outputs 3306

[e5l]

Let use 0 value for a port as the default value for the current protocol. Server use 0 value for port auto selection.