675919 - Websockets - FAIL connection on recv of any unknown opcode

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Patrick McManus [:mcmanus]]

bug 664344 points out that we fail the connection on recv of unknown rsv bits, and unknown ctrl opcodes, but not other opcodes. Section 4.2 of ietf-10 makes it clear that fail is the right behavior.

Comment 1

[Patrick McManus [:mcmanus]]

bug 664344 says "FF (correctly) closes a connection upon receiving reserved control frame opcodes, but not reserved data frame opcodes. The latter will just be silently ignored." and points to tests

Frame with reserved control frame opcode (3)[pass]
and
Frame with reserved data frame opcode (11)[fail]

I think you have those backwards. According to -10 
"%x3-7 are reserved for further non-control frames" and " %xB-F are reserved for further control frames". So it is actually the reserved control code (11) that is not failing the connection.

So the FF issue is that we don't fail on unknown control opcodes.

So we each have a bug :)

Comment 2

[Patrick McManus [:mcmanus]]

Attachment: patch 1

Comment 3

[Tobias Oberstein]

;) fair enough. btw: a chrome developer contacted me .. chrome 14 supports -10, which is good. especially, since I now can compare two major implementations and the spec. - rest assured: they also have bugs (others though) ;). Currently, I'm extending the stuff for full automation, so that I can run - not only browser - against a broad suite of test cases. I let you know when it's in some usable state.

Comment 4

[Jason Duell]

Comment on attachment 550074 [details] [diff] [review]
patch 1

Review of attachment 550074 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/websocket/WebSocketChannel.cpp
@@ +991,5 @@
>          // opcode kPong: the mere act of receiving the packet is all we need
>          // to do for the pong to trigger the activity timers
>          LOG(("WebSocketChannel:: pong received\n"));
>        }
> +      else {

put else on same line, i.e. "} else {"

Comment 5

[Jason Duell]

http://hg.mozilla.org/integration/mozilla-inbound/rev/cd0106ceb8ce

Comment 6

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/cd0106ceb8ce