Bug 39864 – Fix WebSocketHandshakeRequest so that it fits the new handshake protocol

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Yuta Kitamura]

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.

[Yuta Kitamura]

Created attachment 57299 [details]
Patch

[Fumitoshi Ukai]

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 ?

[Yuta Kitamura]

(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.

[Yuta Kitamura]

Created attachment 57435 [details]
Patch (Add Key1, 2 and 3)

[Alexey Proskuryakov]

Comment on attachment 57435 [details]
Patch (Add Key1, 2 and 3)

r=me

[Yuta Kitamura]

(In reply to comment #5)
> (From update of attachment 57435 [details])
> r=me

Thank you!

[Hajime Morrita]

Comment on attachment 57435 [details]
Patch (Add Key1, 2 and 3)

I'll land this manually.

[Hajime Morrita]

Committed r61611: <http://trac.webkit.org/changeset/61611>