MostAwesomeDude / txWS

Twisted WebSockets
Other
94 stars 29 forks source link

add origin, location, protocol and accept headers to HyBi-07 preamble #18

Closed scollinson closed 10 years ago

scollinson commented 11 years ago

When connecting to a txWS via chome, I get the following error:

Error during WebSocket handshake: Sec-WebSocket-Protocol mismatch

This appears to be because the Sec-WebSocket-Protocol header is not sent in the server response. This pull request fixes that, plus adds origin, location and accept headers.

ekohl commented 11 years ago

:+1: Can confirm this fixes it for Chrome 30.

MostAwesomeDude commented 11 years ago

Requires more investigation. The header in question is optional according to RFC 6455, and if it's used, there are strict requirements for its contents which the attached patch doesn't really handle.

I think that Twisted issue 4173's current branch handles this correctly. Given that that branch's code is eventually what we want to support, the issue should be worked on there.

ekohl commented 11 years ago

That's https://twistedmatrix.com/trac/ticket/4173 for the lazy like me.

scollinson commented 11 years ago

Ok, I've removed the non-existent headers and added a check to only send the header if the codec is set, which will only happen when the request contains the Sec-WebSocket-Protocol header.

chancez commented 10 years ago

This seems to resolve issues for me with the latest version of chrome.

@MostAwesomeDude what would you like to have done here? I believe this resolves problems we're having at the lab.

Also, are you planning on maintaining txWS or would you like the OSL to take ownership on that front? Just let me know what's easiest for you.

MostAwesomeDude commented 10 years ago

I suppose I'll merge this and release a new version.

ralphbean commented 10 years ago

@MostAwesomeDude, at Pycon, you suggested that you didn't want to support txWS anymore and only wanted to focus on getting this integrated with Twisted proper. Something along the lines of:

I think that Twisted issue 4173's current branch handles this correctly. Given that that branch's code is eventually what we want to support, the issue should be worked on there.

That sounds great. I, however, have apps using txWS. Those apps could be ported to use the 4173 code when it is eventually merged and released, but I also have deployments on old systems that won't get to see a new Twisted update for a long time.

Therefore, if OSL is interested in maintaining txWS for the foreseeable future, I'd welcome it. I'm guessing you would too; your burden would evaporate. ;)

Either way, cheers!