Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No proper way to specify subprotocol in WebSocket client without breaking install(Auth) #940

Closed
volkert-fastned opened this issue Feb 11, 2019 · 11 comments
Assignees
Labels
ux User Experience issue
Milestone

Comments

@volkert-fastned
Copy link

volkert-fastned commented Feb 11, 2019

Ktor Version

1.1.2

Ktor Engine Used(client or server and name)

CIO (currently the only available WebSocket client provided by Ktor)

JVM Version, Operating System and Relevant Context

Java 11, Linux

Feedback

Currently, as far as I could find, there is apparently no convenient way in the Ktor WebSocket client API to specify the subprotocol(s) to connect with. I'm talking about the value of the "Sec-WebSocket-Protocol" header. Ktor provides a way to specify this when you set up a WebSocket server, through the protocol argument in Route.webSocket(). On the client side however, there is no such argument. I would expect HttpClient.ws and HttpClient.wss to have a similar "protocol" argument as well, but they don't.

As a workaround you can explicitly pass on an HttpRequestBuilder instance to those functions as the "request" argument, in which you then explicitly set the "Sec-WebSocket-Protocol" header (available as the value HttpHeaders.SecWebSocketProtocol in the Ktor API), but as soon as you do so, this overrides any HTTP authentication that you installed in the HttpClient by invoking install(Auth). So subsequently, you then also have to manually encode the username and password in the case of basic authentication. I expect that you'd have to jump through even more hoops in the case of HTTP Digest authentication.

Ideally, I would like to see an optional protocol (String?) argument added to the HttpClient.ws and HttpClient.wss functions to take the "Sec-WebSocket-Protocol" stuff out of our hands, while still being compatible with the install(Auth) function when configuring the HttpClient.

Below is code that reproduces the issue I'm describing here. If you comment out the "header(HttpHeaders.Authorization, ...)" part, you'll see that the "install(Auth) { basic { ... } }" stuff will be ignored when performing a WebSocket upgrade. Be sure to change the "host" and "path" argument to point to some actually running WebSocket server (and if necessary change "client.wss" to "client.ws") to run this code.

import io.ktor.client.HttpClient
import io.ktor.client.features.auth.Auth
import io.ktor.client.features.auth.providers.basic
import io.ktor.client.features.websocket.WebSockets
import io.ktor.client.features.websocket.wss
import io.ktor.client.request.header
import io.ktor.http.HttpHeaders
import io.ktor.http.HttpMethod
import io.ktor.http.cio.websocket.Frame
import io.ktor.http.cio.websocket.readText
import kotlinx.coroutines.channels.filterNotNull
import kotlinx.coroutines.channels.map
import kotlinx.coroutines.runBlocking
import java.util.Base64

val httpBasicAuthName = "username"
val httpBasicPassword = "password"

private val client = HttpClient().config {

    // NOTE: install(Auth) doesn't actually work when passing on a custom request argument to client.wss().
    install(Auth) {
        basic {
            username = httpBasicAuthName
            password = httpBasicPassword
        }

        install(WebSockets)
    }

}

runBlocking {
    client.wss(
        method = HttpMethod.Get,
        host = "somedomain.com",
        path = "/some-endpoint",
        request = {

            // See https://www.iana.org/assignments/websocket/websocket.xml#subprotocol-name
            header(HttpHeaders.SecWebSocketProtocol, "ocpp2.0,ocpp1.6")

            /* NOTE: Using install(Auth) in HttpClient().config doesn't work here for HTTP Basic Authentication,
             * since we're passing in this explicit request lambda argument, because we need to specify the
             * "Sec-WebSocket-Version" header.
             */
            header(
                HttpHeaders.Authorization,
                "Basic ${Base64.getEncoder().encodeToString(
                    "$httpBasicAuthName:$httpBasicPassword".toByteArray(
                        Charsets.UTF_8
                    )
                )}"
            )

        }) {
        // this: DefaultClientWebSocketSession
        send(Frame.Text("Hello World"))

        for (message in incoming.map { it as? Frame.Text }.filterNotNull()) {
            println(message.readText())
        }
    }
}
@e5l e5l self-assigned this Feb 12, 2019
@volkert-fastned
Copy link
Author

volkert-fastned commented Feb 12, 2019

@e5l I have some interesting follow-up information. Apparently, if the username and password that are manually encoded in the "request" lambda passed on to the "client.wss" are incorrect, the WebSocket client will then immediately retry to authenticate, this time using the credentials specified in the "install(Auth)" block. It will keep retrying (with the credentials in "install(Auth)") until the (default?) max send count of 20 is reached:

io.ktor.client.features.SendCountExceedException: Max send count 20 exceeded

Apparently, it preserves the "Sec-WebSocket-Protocol" header value of the initial request in the retries, but switches back to "install(Auth)" for the credentials. Hopefully this detail is useful to you.

@e5l e5l added the ux User Experience issue label Feb 12, 2019
@e5l
Copy link
Member

e5l commented Feb 12, 2019

Hi @volkert-fastned, thanks for the report.
I'm investigating the problem, the fix is planned for 1.2.0

@e5l e5l added this to the 1.2.0 milestone Feb 12, 2019
@volkert-fastned
Copy link
Author

@e5l Any update on this issue? I see 1.2.0 was just released.

@e5l e5l modified the milestones: 1.2.0, 1.3.0 May 17, 2019
@e5l
Copy link
Member

e5l commented May 17, 2019

Sorry for the delay.
There is a design issue with combining Auth and WebSockets.
The solution is delayed for now.

@volkert-fastned
Copy link
Author

No need to apologize. 🙂 I appreciate the hard work you're all doing on constantly improving this amazing framework.

I'm not nearly as well-versed in the internals of Ktor as you are, but if you think I can help you with this in any way, let me know.

@cy6erGn0m cy6erGn0m modified the milestones: 1.3.0, 1.3.1 Dec 18, 2019
@e5l
Copy link
Member

e5l commented Jan 28, 2020

Hi @volkert-fastned, could you try to use sendWithoutRequest flag in the auth provider?

@volkert-fastned
Copy link
Author

@e5l I'll be able to test this on Friday. Is that okay?

@e5l
Copy link
Member

e5l commented Jan 28, 2020

Sure

@e5l e5l modified the milestones: 1.3.1, 1.4.0 Feb 5, 2020
@volkert-fastned
Copy link
Author

volkert-fastned commented Feb 22, 2020

@e5l Hi, sorry for the late reply. I just tried it with Ktor 1.3.1 and with sendWithoutRequest set to true, but even then, I have to explicitly set the basic authentication header when I'm sending a subprotocol header. So unfortunately that does not appear to fix or work around this problem.

By the way, I noticed that in newer versions of Ktor, nesting install(WebSockets) inside the lambda of install(Auth), as I did in the example above, causes WebSocket upgrades to fail as if the WebSocket provider wasn't installed, with an error message like this:

io.ktor.client.features.ClientRequestException: Client request(wss://somedomain.com:443/some-endpoint) invalid: 404 Not Found

Installing the providers outside of each other, like this, causes WebSocket upgrades to work again:

private val client = HttpClient().config {

    install(WebSockets)
    install(HttpSend)

    // NOTE: install(Auth) doesn't actually work when passing on a custom request argument to client.wss().
    install(Auth) {
        basic {
            username = httpBasicAuthName
            password = httpBasicPassword
            // The line below does not seem to solve the issue.
            this.sendWithoutRequest = true
        }
    }

}

By the way, does the order in which you install these providers matter in any way?

Thanks again.

@oleg-larshin
Copy link

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

@Stexxe
Copy link
Contributor

Stexxe commented Jul 15, 2021

I cannot reproduce this problem with Ktor 1.6.1 when having sendWithoutRequest { true } in a basic authentication config.

@Stexxe Stexxe closed this as completed Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ux User Experience issue
Projects
None yet
Development

No branches or pull requests

5 participants