Last Comment Bug 675919 - Websockets - FAIL connection on recv of any unknown opcode
: Websockets - FAIL connection on recv of any unknown opcode
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla8
Assigned To: Patrick McManus [:mcmanus]
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


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

Description 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 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]
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] 2011-08-02 07:50:59 PDT
Created attachment 550074 [details] [diff] [review]
patch 1
Comment 3 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 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 Jason Duell [:jduell] (needinfo? me) 2011-08-04 22:50:01 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/cd0106ceb8ce
Comment 6 Marco Bonardo [::mak] 2011-08-05 09:08:40 PDT
http://hg.mozilla.org/mozilla-central/rev/cd0106ceb8ce

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