Last Comment Bug 675919 - Websockets - FAIL connection on recv of any unknown opcode
: Websockets - FAIL connection on recv of any unknown opcode
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: x86_64 Linux
-- normal (vote)
: mozilla8
Assigned To: Patrick McManus [:mcmanus]
: Patrick McManus [:mcmanus]
Depends on:
Blocks: 664344
  Show dependency treegraph
Reported: 2011-08-02 06:39 PDT by Patrick McManus [:mcmanus]
Modified: 2011-08-05 09:08 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch 1 (1.68 KB, patch)
2011-08-02 07:50 PDT, Patrick McManus [:mcmanus]
jduell.mcbugs: review+
Details | Diff | Splinter Review

Description User image Patrick McManus [:mcmanus] 2011-08-02 06:39:33 PDT
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 User image Patrick McManus [:mcmanus] 2011-08-02 07:48:21 PDT
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]
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 User image Patrick McManus [:mcmanus] 2011-08-02 07:50:59 PDT
Created attachment 550074 [details] [diff] [review]
patch 1
Comment 3 User image Tobias Oberstein 2011-08-02 07:54:44 PDT
;) 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 User image Jason Duell [:jduell] (needinfo me) 2011-08-04 22:32:39 PDT
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 User image Jason Duell [:jduell] (needinfo me) 2011-08-04 22:50:01 PDT
Comment 6 User image Marco Bonardo [::mak] 2011-08-05 09:08:40 PDT

Note You need to log in before you can comment on or make changes to this bug.