@spaceone What do you think?

And... WTH? Travis-CI doesn't have Python 2.7 anymore?!

@ri0t Thanks for doing this!

The Sec-WebSocket-Protocol request header is a list of values while the response header must be a single value. I guess the best would be to have a static list of supported values or a functtion method pick_subprotocol(...). If no request header is set we also don't need to send a response header. I will have a look this week if I can improove this and test why chrome does such an API change.

@spaceone What did you find? Shall we merge this PR in as-is anyway? :) If it solves the immediate problem I don't see why not :) We can always iterate and improve upon it alter!

There is no regression in Chromium - at least not in Chromium 51. @ri0t which chromium version do you use?
Can you provide the javascript code you are testing this with?
I think you added the optional second parameter 'protocol' to the WebSocket() initialization - which causes it to not working:
https://developer.mozilla.org/en-US/docs/Web/API/WebSocket

You can always add a handler on your own which picks a subprotocol.

See spaceone@7f46e45

Can we merge this or is this a regression in Chromium?

@spaceone: sorry for the late reply. The behaviour is the same no matter which chromium-version i use, even the android ones exhibit the same. I just checked with 50.
With origin/master of circuits, i get:

WebSocket connection to 'ws://localhost/websocket' failed: Error during WebSocket handshake: Sent non-empty 'Sec-WebSocket-Protocol' header but no response was received

Whereas, when i echo back the protocols like in the PR, i have no problem. The connection is established thus:

this.sock = new WebSocket('ws://' + this.host + ':' + this.port + '/websocket', 'HFOS');

And yes, you're completely right, when leaving out the subprotocol, it works as expected. Did not know, one could just leave that out :D
So, in my opinion we could probably close this PR.

What's happenning with this?

IMHO either WONTFIX/INVALID or merge my fix which adds a function to select the subprotocol: master...spaceone:websocket_subprotocol

I added my commit also to circuits.http: https://github.com/spaceone/circuits.http/blob/260bf3bc810fd5e281319aefb86110c75f30eaa0/circuits/http/server/connection/websocket.py

I thought we merged your fix already?

James Mills / prologic

E: prologic@shortcircuit.net.au
W: prologic.shortcircuit.net.au

On Sun, Nov 6, 2016 at 7:18 PM, ⁣ Florian Best notifications@github.com
wrote:

IMHO either WONTFIX or merge my fix which adds a function to select the
subprotocol: master...spaceone:websocket_subprotocol
master...spaceone:websocket_subprotocol

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#161 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABOv-hZrVmXDM1j9wTNUYCd6I1JpBuVgks5q7phtgaJpZM4Izyg7
.

Please don't confuse this issue with another issue. Maybe you should read the messages here again.

Sorry :) I'll have a closer look at this later.

James Mills / prologic

E: prologic@shortcircuit.net.au
W: prologic.shortcircuit.net.au

On Sun, Nov 6, 2016 at 7:35 PM, ⁣ Florian Best notifications@github.com
wrote:

Please don't confuse this issue with another issue. Maybe you should read
the messages here again.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#161 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABOv-oDCW64DgxHy_UB31OLqm2UuGFV-ks5q7pxygaJpZM4Izyg7
.

Merged in my fix: #201