Last Comment Bug 661036 - nsWebSocket::SetProtocol allows U+0020
: nsWebSocket::SetProtocol allows U+0020
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking: WebSockets (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla7
Assigned To: Patrick McManus [:mcmanus]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-31 16:15 PDT by David Chan [:dchan]
Modified: 2011-06-17 07:41 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
no spaces 1 (17.65 KB, patch)
2011-06-01 10:22 PDT, Patrick McManus [:mcmanus]
brian: review+
cbiesinger: review+
Details | Diff | Splinter Review

Description David Chan [:dchan] 2011-05-31 16:15:10 PDT
SetProtocol checks that the supplied nsString contains only characters between 0x0020 and 0x007E inclusive [1]. The spec defines valid characters as between 0x0021 and 0x007E inclusive [2]. 

This likely won't cause any functional issues due to 0x0020 being the space character.


[1] - http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsWebSocket.cpp#1060
[2] - http://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-07 5.1.10 page 29
Comment 1 Patrick McManus [:mcmanus] 2011-05-31 16:40:15 PDT
That's a good catch - that code is leftover from the -76 implementation when the space was legal.

Unfortunately most of our tests use sub-protocols with spaces in them :(
Comment 2 Patrick McManus [:mcmanus] 2011-06-01 10:22:37 PDT
Created attachment 536653 [details] [diff] [review]
no spaces 1

1] fixes issue
2] updates existing tests that accidentally violated that clause
3] adds new test that intentionally violates that clause (part of test-5)
Comment 3 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-06-15 11:02:59 PDT
Comment on attachment 536653 [details] [diff] [review]
no spaces 1

This looks good to me. Sorry I didn't review it earlier; for some reason I got no email about the review request.
Comment 4 Philipp von Weitershausen [:philikon] 2011-06-17 07:41:23 PDT
http://hg.mozilla.org/mozilla-central/rev/53418bef40e9

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