Last Comment Bug 661036 - nsWebSocket::SetProtocol allows U+0020
: nsWebSocket::SetProtocol allows U+0020
Product: Core
Classification: Components
Component: Networking: WebSockets (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla7
Assigned To: Patrick McManus [:mcmanus]
: Patrick McManus [:mcmanus]
Depends on:
  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:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

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 User image 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] -
[2] - 5.1.10 page 29
Comment 1 User image 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 User image 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 User image 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 User image Philipp von Weitershausen [:philikon] 2011-06-17 07:41:23 PDT

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