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

Select an arbitrary websocket subprotocol proposed by the client #8860

Merged
merged 4 commits into from Dec 9, 2018

Conversation

Projects
None yet
4 participants
@raboof
Copy link
Member

raboof commented Dec 6, 2018

Simple (on par with Netty) solution to #7895. Later we could extend our API to
allow the framework user to select a subprotocol instead.

*
* See https://github.com/playframework/playframework/issues/7895
*/
def handleWebSocket(upgrade: UpgradeToWebSocket, flow: Flow[Message, Message, _], bufferLimit: Int): HttpResponse =

This comment has been minimized.

@gmethvin

gmethvin Dec 6, 2018

Member

I think we should at least deprecate this. I'm not even sure this class should be considered part of the public API.

@raboof

This comment has been minimized.

Copy link
Member Author

raboof commented Dec 6, 2018

Not really AFAICS, as it seems WebSocketClient doesn't allow setting that header...

@marcospereira

This comment has been minimized.

Copy link
Member

marcospereira commented Dec 7, 2018

I think we can change connect method to also have a headers: Seq[(String, String)] = Seq.empty and later use the value here:

val handshaker = WebSocketClientHandshakerFactory.newHandshaker(tgt, version, null, false, new DefaultHttpHeaders())

Instead of just using an empty DefaultHttpHeaders. Do you think that would be enough to then add some tests?

@marcospereira
Copy link
Member

marcospereira left a comment

LGTM!

Thank you, @raboof.

@marcospereira marcospereira merged commit 78b41e0 into master Dec 9, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
typesafe-cla-validator All users have signed the CLA
Details

@marcospereira marcospereira deleted the includeArbitraryProtocolInWebsocketUpgrade branch Dec 9, 2018

marcospereira added a commit that referenced this pull request Dec 9, 2018

Select an arbitrary websocket subprotocol proposed by the client (#8860)
* Select an arbitrary websocket subprotocol proposed by the client

* Mark method without subprotocol as deprecated

* Add integration test
@marcospereira

This comment has been minimized.

Copy link
Member

marcospereira commented Dec 9, 2018

Backport to 2.7.x: 9d1f9fe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment