711886 - Refuse connection if server replies with non-matching subprotocol

Status: RESOLVED FIXED

(Core :: Networking: WebSockets, defect)

Description

[Jason Duell]

Attachment: v1, aka bug 666349 attachment 582496

Splitting out from bug 666349 attachment 582496 [details] [diff] [review] (carring forward patrick's +r)

The test for this is currently failing on some tryservers (mainly linux, but I've seen windows too). The symptom is that the WS connection hangs instead of failing.  

I suspect some sort of pywebsocket issue (maybe python version specific?) is causing the server to barf w/o failing the connection, so we hang.

As an extra piece of fun, the mochitest harness reports this as 

 ERROR TEST-UNEXPECTED-FAIL | test_websocket.html | Test timed out.
 TEST-END | /tests/content/base/test/test_websocket.html | finished in 325819ms
 ERROR TEST-UNEXPECTED-FAIL | test_websocket_basic.html | Should get error in test-47!

i.e it's wrongly reporting the test as being in test_websocket_basic.html.

I'm gonna try this on python 2.5 and see what happens.

Comment 1

[Takeshi Yoshino]

Hi Jason, do you still see the bug of pywebsocket which prevented this change from being committed?

Comment 2

[Andrew Cook]

Bug still present in 37

Comment 3

[Patrick McManus [:mcmanus]]

jason, we should get this landed - right?

Comment 4

[Takeshi Yoshino]

In HyBi ML, there we're seeing rough rough consensus on that subprotocol comparison should be made case-sensitively.

See this thread http://www.ietf.org/mail-archive/web/hybi/current/threads.html#10719

I summarized interoperability issue between Chromium and Firefox in this post.
http://www.ietf.org/mail-archive/web/hybi/current/msg10727.html

Comment 5

[Jason Duell]

Attachment: v2: rebased and made subprotocol comparison case-sensitive

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/af74581564bc

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for bustage: https://treeherder.mozilla.org/logviewer.html#?job_id=27674070&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/2da14459ec0f

Comment 8

[Jason Duell]

Michal, please take a look at this at some point when you've got cycles.  IIRC if you push this to try again you'll see that we have a web spec test that for some reason thinks that the comparison should be case-insensitive, in contrast to comment 4.  I.e. I think it may just be a test change needed to land this.

Comment 9

[Michal Novotny [:michal]]

Junior is going to take a look at it.

Comment 10

[Junior [inactive]]

Attachment: v3: rebased and made subprotocol comparison case-sensitive

Fix bustage, carry r+
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f04c8081f344c2bb3db76ff15178ea9ed3d6c10e

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a3a29341427
Fail Websocket if server replies with non-matching subprotocol, r=mcmanus

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9a3a29341427