WebSocket over SPDY: handshake support for both of SPDY/2 and SPDY/3

net/websockets/websocket_handshake_handler.cc

Index: net/websockets/websocket_handshake_handler.cc
diff --git a/net/websockets/websocket_handshake_handler.cc b/net/websockets/websocket_handshake_handler.cc
index 507b53a32cf840013c5e52dfa57acc25b1507b50..ddec849d140d9ad524407e44dfa02aaa9c4cd45f 100644
--- a/net/websockets/websocket_handshake_handler.cc
+++ b/net/websockets/websocket_handshake_handler.cc
@@ -266,13 +266,23 @@ HttpRequestInfo WebSocketHandshakeRequestHandler::GetRequestInfo(
 }
 
 bool WebSocketHandshakeRequestHandler::GetRequestHeaderBlock(
-    const GURL& url, SpdyHeaderBlock* headers, std::string* challenge) {
+    const GURL& url,
+    SpdyHeaderBlock* headers,
+    std::string* challenge,
+    NextProto protocol_negotiated) {
   // Construct opening handshake request headers as a SPDY header block.
   // For details, see WebSocket Layering over SPDY/3 Draft 8.
-  (*headers)["path"] = url.path();
-  (*headers)["version"] =
+  if (protocol_negotiated == kProtoSPDY2) {

[Yuta Kitamura, 2012/08/03 07:31:51]

Can we assume |protocol_negotiated| is either kProtoSPDY2 or kProtoSPDY3? Or it
may take other values like kProtoUnknown, kProtoHTTP11 or kProtoSPDY1?

[Takashi Toyoshima, 2012/08/06 13:01:53]

Good point.
It doesn't seem to be kProtoHTTP11 or kProtoSPDY1 because these negotiation
never create right spdy session for WebSocket. But, kProtoUnknown can be.

SpdySession want to use g_default_protocol for kProtoUnknown to initiate a new
session. I guess this default value is kProtoSPDY2, or kProtoSPDY3 if it is
enabled. So using this value looks right way. I'll fix this change to hold this
initiating protocol version, and query it from WebSocketJob to SpdySession.

ps. SpdySession::GetProtocolVersion looks right function to know the actually
used protocol version.

+    (*headers)["path"] = url.path();
+    (*headers)["version"] =
       base::StringPrintf("%s%d", "WebSocket/", protocol_version_);
-  (*headers)["scheme"] = url.scheme();
+    (*headers)["scheme"] = url.scheme();
+  } else {
+    (*headers)[":path"] = url.path();
+    (*headers)[":version"] =
+      base::StringPrintf("%s%d", "WebSocket/", protocol_version_);
+    (*headers)[":scheme"] = url.scheme();
+  }
 
   HttpUtil::HeadersIterator iter(headers_.begin(), headers_.end(), "\r\n");
   while (iter.GetNext()) {
@@ -291,10 +301,29 @@ bool WebSocketHandshakeRequestHandler::GetRequestHeaderBlock(
       *challenge = iter.values();
       // Sec-WebSocket-Key is not sent to a server.
       continue;
+    } else if (LowerCaseEqualsASCII(iter.name_begin(),
+                                    iter.name_end(),
+                                    "host") ||
+               LowerCaseEqualsASCII(iter.name_begin(),
+                                    iter.name_end(),
+                                    "origin") ||
+               LowerCaseEqualsASCII(iter.name_begin(),
+                                    iter.name_end(),
+                                    "sec-websocket-protocol") ||
+               LowerCaseEqualsASCII(iter.name_begin(),
+                                    iter.name_end(),
+                                    "sec-websocket-extensions")) {
+      // TODO(toyoshim): Some WebSocket extensions may be not not compatible

[Yuta Kitamura, 2012/08/03 07:31:51]

"may be not not" -> "may not be"

[Takashi Toyoshima, 2012/08/06 13:01:53]

Thanks.

+      // with SPDY. We should omit them from a Sec-WebSocket-Extension header.
+      std::string name;
+      if (protocol_negotiated == kProtoSPDY2)
+        name = StringToLowerASCII(iter.name());
+      else
+        name = ":" + StringToLowerASCII(iter.name());
+      (*headers)[name] = iter.values();
+      continue;
     }
     // Others should be sent out to |headers|.
-    // TODO(toyoshim): Some WebSocket extensions are not compatible with SPDY.
-    // We should remove them from a Sec-WebSocket-Extension header.
     std::string name = StringToLowerASCII(iter.name());
     SpdyHeaderBlock::iterator found = headers->find(name);
     if (found == headers->end()) {
@@ -427,11 +456,16 @@ bool WebSocketHandshakeResponseHandler::ParseResponseInfo(
 
 bool WebSocketHandshakeResponseHandler::ParseResponseHeaderBlock(
     const SpdyHeaderBlock& headers,
-    const std::string& challenge) {
-  std::string response_message;
-  SpdyHeaderBlock::const_iterator status = headers.find("status");
+    const std::string& challenge,
+    NextProto protocol_negotiated) {
+  SpdyHeaderBlock::const_iterator status;
+  if (protocol_negotiated == kProtoSPDY2)
+    status = headers.find("status");
+  else
+    status = headers.find(":status");
   if (status == headers.end())
     return false;
+  std::string response_message;
   response_message =
       base::StringPrintf("%s%s\r\n", "HTTP/1.1 ", status->second.c_str());
   response_message += "Upgrade: websocket\r\n";
@@ -449,7 +483,10 @@ bool WebSocketHandshakeResponseHandler::ParseResponseHeaderBlock(
     // For each value, if the server sends a NUL-separated list of values,
     // we separate that back out into individual headers for each value
     // in the list.
-    if (LowerCaseEqualsASCII(iter->first, "status")) {
+    if ((protocol_negotiated == kProtoSPDY2 &&
+        LowerCaseEqualsASCII(iter->first, "status")) ||

[Yuta Kitamura, 2012/08/03 07:31:51]

nit: This line needs to align the opening paren, i.e.:

  if ((foo &&
       bar) ||
      baz) {
  }

[Takashi Toyoshima, 2012/08/06 13:01:53]

Done.

+        (protocol_negotiated != kProtoSPDY2 &&
+        LowerCaseEqualsASCII(iter->first, ":status"))) {

[Yuta Kitamura, 2012/08/03 07:31:51]

Ditto.

[Takashi Toyoshima, 2012/08/06 13:01:53]

Done.

       // The status value is already handled as the first line of
       // |response_message|. Just skip here.
       continue;
@@ -464,7 +501,12 @@ bool WebSocketHandshakeResponseHandler::ParseResponseHeaderBlock(
         tval = value.substr(start, (end - start));
       else
         tval = value.substr(start);
-      response_message += iter->first + ": " + tval + "\r\n";
+      if (protocol_negotiated != kProtoSPDY2 &&
+          (LowerCaseEqualsASCII(iter->first, ":sec-websocket-protocol") ||
+          LowerCaseEqualsASCII(iter->first, ":sec-websocket-extensions")))

[Yuta Kitamura, 2012/08/03 07:31:51]

Ditto.

[Takashi Toyoshima, 2012/08/06 13:01:53]

Done.

+        response_message += iter->first.substr(1) + ": " + tval + "\r\n";
+      else
+        response_message += iter->first + ": " + tval + "\r\n";
       start = end + 1;
     } while (end != std::string::npos);
   }