Bug 39864 - Fix WebSocketHandshakeRequest so that it fits the new handshake protocol
Summary: Fix WebSocketHandshakeRequest so that it fits the new handshake protocol
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yuta Kitamura
URL:
Keywords:
Depends on:
Blocks: 38831 40768
  Show dependency tree / graph
 
Reported: 2010-05-27 23:55 PDT by Yuta Kitamura
Modified: 2010-06-22 02:00 PDT (History)
3 users (show)

See Also:

Attachments
Patch (7.42 KB, patch)
2010-05-28 00:01 PDT, Yuta Kitamura 		no flags Details | Formatted Diff | Diff
Patch (Add Key1, 2 and 3) (8.46 KB, patch)
2010-05-30 23:05 PDT, Yuta Kitamura 		ap: review+
morrita: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yuta Kitamura 2010-05-27 23:55:46 PDT 
The new WebSocket specification (draft 76 and later) allows a browser to send header fields in arbitrary order. Thus we can use a HTTPHeaderMap to store header fields instead of Vector of pairs of a field name and a field value.
Comment 1 Yuta Kitamura 2010-05-28 00:01:29 PDT 
Created attachment 57299 [details]
Patch
Comment 2 Fumitoshi Ukai 2010-05-28 00:57:36 PDT 
Comment on attachment 57299 [details]
Patch


> diff --git a/WebCore/websockets/WebSocketHandshake.cpp b/WebCore/websockets/WebSocketHandshake.cpp
> index ea4f5e5657c53c2532e6d90aec410e0d79976be3..9b1fa591278af51db482e4885adce0a7ec511e3a 100644
> --- a/WebCore/websockets/WebSocketHandshake.cpp
> +++ b/WebCore/websockets/WebSocketHandshake.cpp
> @@ -279,14 +279,20 @@ WebSocketHandshakeRequest WebSocketHandshake::clientHandshakeRequest() const
>      // Keep the following consistent with clientHandshakeMessage().
>      // FIXME: do we need to store m_secWebSocketKey1, m_secWebSocketKey2 and
>      // m_key3 in WebSocketHandshakeRequest?
> -    WebSocketHandshakeRequest request(m_url, clientOrigin(), m_clientProtocol);
> +    WebSocketHandshakeRequest request("GET", m_url);
> +    request.addHeaderField("Upgrade", "WebSocket");
> +    request.addHeaderField("Connection", "Upgrade");
> +    request.addHeaderField("Host", hostName(m_url, m_secure));
> +    request.addHeaderField("Origin", clientOrigin());
> +    if (!m_clientProtocol.isEmpty())
> +        request.addHeaderField("Sec-WebSocket-Protocol:", m_clientProtocol);

Don't we want to add Sec-WebSocket-Key1, Key2 ?
How about key3 ?
Comment 3 Yuta Kitamura 2010-05-30 19:34:47 PDT 
(In reply to comment #2)
> (From update of attachment 57299 [details])
> 
> > diff --git a/WebCore/websockets/WebSocketHandshake.cpp b/WebCore/websockets/WebSocketHandshake.cpp
> > index ea4f5e5657c53c2532e6d90aec410e0d79976be3..9b1fa591278af51db482e4885adce0a7ec511e3a 100644
> > --- a/WebCore/websockets/WebSocketHandshake.cpp
> > +++ b/WebCore/websockets/WebSocketHandshake.cpp
> > @@ -279,14 +279,20 @@ WebSocketHandshakeRequest WebSocketHandshake::clientHandshakeRequest() const
> >      // Keep the following consistent with clientHandshakeMessage().
> >      // FIXME: do we need to store m_secWebSocketKey1, m_secWebSocketKey2 and
> >      // m_key3 in WebSocketHandshakeRequest?
> > -    WebSocketHandshakeRequest request(m_url, clientOrigin(), m_clientProtocol);
> > +    WebSocketHandshakeRequest request("GET", m_url);
> > +    request.addHeaderField("Upgrade", "WebSocket");
> > +    request.addHeaderField("Connection", "Upgrade");
> > +    request.addHeaderField("Host", hostName(m_url, m_secure));
> > +    request.addHeaderField("Origin", clientOrigin());
> > +    if (!m_clientProtocol.isEmpty())
> > +        request.addHeaderField("Sec-WebSocket-Protocol:", m_clientProtocol);
> 
> Don't we want to add Sec-WebSocket-Key1, Key2 ?
> How about key3 ?

I think we should have them. Let me update the patch.
Comment 4 Yuta Kitamura 2010-05-30 23:05:48 PDT 
Created attachment 57435 [details]
Patch (Add Key1, 2 and 3)
Comment 5 Alexey Proskuryakov 2010-06-17 10:35:56 PDT 
Comment on attachment 57435 [details]
Patch (Add Key1, 2 and 3)

r=me
Comment 6 Yuta Kitamura 2010-06-17 19:32:11 PDT 
(In reply to comment #5)
> (From update of attachment 57435 [details])
> r=me

Thank you!
Comment 7 Hajime Morrita 2010-06-22 01:47:13 PDT 
Comment on attachment 57435 [details]
Patch (Add Key1, 2 and 3)

I'll land this manually.
Comment 8 Hajime Morrita 2010-06-22 02:00:29 PDT 
Committed r61611: <http://trac.webkit.org/changeset/61611>