Bug 65367

Summary: WebSocket: Receive URL and subprotocol in WebSocketChannel::connect()
Product: WebKit Reporter: Yuta Kitamura <yutak>
Component: WebCore Misc.Assignee: Yuta Kitamura <yutak>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 65247    
Attachments:
Description Flags
Patch
none
Patch v2
none
Patch v3 none

Yuta Kitamura
Reported 2011-07-29 04:37:53 PDT
Receive URL and subprotocol in WebSocketChannel::connect(), rather than WebSocketChannel constructor. We need to check the arguments of WebSocket constructor (in WebSocket::connect()). This should be done between WebSocketChannel construction and WebSocketChannel::conncect() call, because we need to know which WebSocket protocol is used via WebSocketChannel to check the arguments, and then create a connection after checking the arguments. This is part of bug 65247.
Attachments
Patch (21.31 KB, patch)
2011-07-29 05:00 PDT, Yuta Kitamura 		no flags
DetailsFormatted DiffDiff
Patch v2 (22.31 KB, patch)
2011-08-01 01:50 PDT, Yuta Kitamura 		no flags
DetailsFormatted DiffDiff
Patch v3 (22.45 KB, patch)
2011-08-01 02:21 PDT, Yuta Kitamura 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Yuta Kitamura
Comment 1 2011-07-29 05:00:41 PDT
Created attachment 102348 [details] Patch
Kent Tamura
Comment 2 2011-07-31 19:37:48 PDT
Comment on attachment 102348 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=102348&action=review > Source/WebCore/ChangeLog:9 > + WebSocket::connect() needs to check the value of subprotocols provided by the script depending > + on the value of WebSocketChannel::useHixie76Protocol(), thus WebSocketChannel has to receive Isn't The subprotocol check implemented in connect() yet? > Source/WebCore/ChangeLog:36 > + * websockets/ThreadableWebSocketChannel.cpp: > + (WebCore::ThreadableWebSocketChannel::create): > + * websockets/ThreadableWebSocketChannel.h: > + * websockets/WebSocket.cpp: > + (WebCore::WebSocket::connect): > + * websockets/WebSocketChannel.cpp: > + (WebCore::WebSocketChannel::WebSocketChannel): > + (WebCore::WebSocketChannel::connect): > + * websockets/WebSocketChannel.h: > + (WebCore::WebSocketChannel::create): > + * websockets/WorkerThreadableWebSocketChannel.cpp: > + (WebCore::WorkerThreadableWebSocketChannel::WorkerThreadableWebSocketChannel): > + (WebCore::WorkerThreadableWebSocketChannel::connect): > + (WebCore::WorkerThreadableWebSocketChannel::Peer::Peer): > + (WebCore::WorkerThreadableWebSocketChannel::Peer::connect): > + (WebCore::WorkerThreadableWebSocketChannel::Bridge::mainThreadCreateWebSocketChannel): > + (WebCore::WorkerThreadableWebSocketChannel::Bridge::Bridge): > + (WebCore::WorkerThreadableWebSocketChannel::mainThreadConnect): > + (WebCore::WorkerThreadableWebSocketChannel::Bridge::connect): > + * websockets/WorkerThreadableWebSocketChannel.h: > + (WebCore::WorkerThreadableWebSocketChannel::create): > + (WebCore::WorkerThreadableWebSocketChannel::Peer::create): > + (WebCore::WorkerThreadableWebSocketChannel::Bridge::create): You should write at least: * Removed the URL argument and the protocol argument for factory functions and constructors * Add the URL argument and the protocol argument for connect() functions.
Yuta Kitamura
Comment 3 2011-08-01 01:50:14 PDT
Created attachment 102494 [details] Patch v2
Yuta Kitamura
Comment 4 2011-08-01 01:52:45 PDT
Comment on attachment 102348 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=102348&action=review >> Source/WebCore/ChangeLog:9 >> + on the value of WebSocketChannel::useHixie76Protocol(), thus WebSocketChannel has to receive > > Isn't The subprotocol check implemented in connect() yet? Not yet. It will be implemented later. Rephrased the description completely to make sure the point becomes clearer. >> Source/WebCore/ChangeLog:36 >> + (WebCore::WorkerThreadableWebSocketChannel::Bridge::create): > > You should write at least: > * Removed the URL argument and the protocol argument > for factory functions and constructors > * Add the URL argument and the protocol argument > for connect() functions. Done.
Kent Tamura
Comment 5 2011-08-01 02:00:29 PDT
(In reply to comment #4) > (From update of attachment 102348 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=102348&action=review > > >> Source/WebCore/ChangeLog:9 > >> + on the value of WebSocketChannel::useHixie76Protocol(), thus WebSocketChannel has to receive > > > > Isn't The subprotocol check implemented in connect() yet? > > Not yet. It will be implemented later. Rephrased the description completely to make sure the point becomes clearer. ok, so we should have a FIXME comment in connect().
Yuta Kitamura
Comment 6 2011-08-01 02:21:27 PDT
Created attachment 102498 [details] Patch v3
Kent Tamura
Comment 7 2011-08-01 02:22:23 PDT
Comment on attachment 102498 [details] Patch v3 ok
Yuta Kitamura
Comment 8 2011-08-01 02:22:41 PDT
Comment on attachment 102348 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=102348&action=review >>>> Source/WebCore/ChangeLog:9 >>>> + on the value of WebSocketChannel::useHixie76Protocol(), thus WebSocketChannel has to receive >>> >>> Isn't The subprotocol check implemented in connect() yet? >> >> Not yet. It will be implemented later. Rephrased the description completely to make sure the point becomes clearer. > > ok, so we should have a FIXME comment in connect(). Added FIXME in WebSocket::connect().
WebKit Review Bot
Comment 9 2011-08-01 04:16:31 PDT
Comment on attachment 102498 [details] Patch v3 Clearing flags on attachment: 102498 Committed r92116: <http://trac.webkit.org/changeset/92116>
WebKit Review Bot
Comment 10 2011-08-01 04:16:36 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.