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

KTOR-7363 Fix ProtocolException when Websockets is used with Auth #4338

Merged

Conversation

marychatte
Copy link
Member

Subsystem
Websockets

Motivation
KTOR-7363

@marychatte marychatte self-assigned this Sep 24, 2024
@marychatte marychatte force-pushed the marychatte/KTOR-7363-Websockets-and-Auth-ProtocolException branch from a072139 to 5deeb5f Compare September 25, 2024 15:35
@marychatte marychatte force-pushed the marychatte/KTOR-7363-Websockets-and-Auth-ProtocolException branch from 5deeb5f to 5078569 Compare September 26, 2024 08:48
@marychatte marychatte force-pushed the marychatte/KTOR-7363-Websockets-and-Auth-ProtocolException branch from 5078569 to 377bbe4 Compare September 26, 2024 09:23
Comment on lines +371 to +377
test { client ->
client.webSocket("$TEST_WEBSOCKET_SERVER/auth/websocket") {
val frame = incoming.receive() as Frame.Text
assertEquals("Hello from server", frame.readText())
}
client.close()
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to include also test cases for a) valid initial token, and b) fully invalid token.

@@ -350,4 +352,28 @@ class WebSocketTest : ClientLoader() {
}
}
}

@Test
fun testWithAuthPlugin() = clientTests(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a more specific name here, like "testAuthenticationWithRefreshTokens".

@marychatte marychatte merged commit 994aa31 into main Oct 2, 2024
12 of 14 checks passed
@marychatte marychatte deleted the marychatte/KTOR-7363-Websockets-and-Auth-ProtocolException branch October 2, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants