Fix WebSocketServer extension parser.

net/server/web_socket_encoder.cc

Index: net/server/web_socket_encoder.cc
diff --git a/net/server/web_socket_encoder.cc b/net/server/web_socket_encoder.cc
index 1a5431affd8415a23e6ee6f3a553064d1447a803..6d8aec390d2bd64c83be28b3bd762d6fd3c1d11f 100644
--- a/net/server/web_socket_encoder.cc
+++ b/net/server/web_socket_encoder.cc
@@ -4,10 +4,14 @@
 
 #include "net/server/web_socket_encoder.h"
 
+#include <vector>
+
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "net/base/io_buffer.h"
+#include "net/websockets/websocket_deflate_parameters.h"
+#include "net/websockets/websocket_extension.h"
 #include "net/websockets/websocket_extension_parser.h"
 
 namespace net {
@@ -180,144 +184,103 @@ void EncodeFrameHybi17(const std::string& message,
 }  // anonymous namespace
 
 // static
-WebSocketEncoder* WebSocketEncoder::CreateServer(
-    const std::string& request_extensions,
-    std::string* response_extensions) {
-  bool deflate;
-  bool has_client_window_bits;
-  int client_window_bits;
-  int server_window_bits;
-  bool client_no_context_takeover;
-  bool server_no_context_takeover;
-  ParseExtensions(request_extensions, &deflate, &has_client_window_bits,
-                  &client_window_bits, &server_window_bits,
-                  &client_no_context_takeover, &server_no_context_takeover);
-
-  if (deflate) {
-    *response_extensions = base::StringPrintf(
-        "permessage-deflate; server_max_window_bits=%d%s", server_window_bits,
-        server_no_context_takeover ? "; server_no_context_takeover" : "");
-    if (has_client_window_bits) {
-      base::StringAppendF(response_extensions, "; client_max_window_bits=%d",
-                          client_window_bits);
-    } else {
-      DCHECK_EQ(client_window_bits, 15);
-    }
-    return new WebSocketEncoder(true /* is_server */, server_window_bits,
-                                client_window_bits, server_no_context_takeover);
-  } else {
-    *response_extensions = std::string();
-    return new WebSocketEncoder(true /* is_server */);
-  }
+scoped_ptr<WebSocketEncoder> WebSocketEncoder::CreateServer() {
+  return make_scoped_ptr(new WebSocketEncoder(FOR_SERVER, nullptr, nullptr));
 }
 
 // static
-WebSocketEncoder* WebSocketEncoder::CreateClient(
-    const std::string& response_extensions) {
-  bool deflate;
-  bool has_client_window_bits;
-  int client_window_bits;
-  int server_window_bits;
-  bool client_no_context_takeover;
-  bool server_no_context_takeover;
-  ParseExtensions(response_extensions, &deflate, &has_client_window_bits,
-                  &client_window_bits, &server_window_bits,
-                  &client_no_context_takeover, &server_no_context_takeover);
-
-  if (deflate) {
-    return new WebSocketEncoder(false /* is_server */, client_window_bits,
-                                server_window_bits, client_no_context_takeover);
-  } else {
-    return new WebSocketEncoder(false /* is_server */);
+scoped_ptr<WebSocketEncoder> WebSocketEncoder::CreateServer(
+    const std::string& request_extensions,
+    WebSocketDeflateParameters* deflate_parameters) {
+  WebSocketExtensionParser parser;
+  if (!parser.Parse(request_extensions)) {
+    // Failed to parse Sec-WebSocket-Extensions header. We MUST fail the
+    // connection.
+    return nullptr;
   }
-}
-
-// static
-void WebSocketEncoder::ParseExtensions(const std::string& header_value,
-                                       bool* deflate,
-                                       bool* has_client_window_bits,
-                                       int* client_window_bits,
-                                       int* server_window_bits,
-                                       bool* client_no_context_takeover,
-                                       bool* server_no_context_takeover) {
-  *deflate = false;
-  *has_client_window_bits = false;
-  *client_window_bits = 15;
-  *server_window_bits = 15;
-  *client_no_context_takeover = false;
-  *server_no_context_takeover = false;
-
-  if (header_value.empty())
-    return;
 
-  WebSocketExtensionParser parser;
-  if (!parser.Parse(header_value))
-    return;
-  const std::vector<WebSocketExtension>& extensions = parser.extensions();
-  // TODO(tyoshino): Fail if this method is used for parsing a response and
-  // there are multiple permessage-deflate extensions or there are any unknown
-  // extensions.
-  for (const auto& extension : extensions) {
-    if (extension.name() != "permessage-deflate") {
+  WebSocketDeflateParameters offered, response;
+  std::string failure_message;
+  bool found = false;
+  for (const auto& extension : parser.extensions()) {
+    WebSocketDeflateParameters params;
+    if (!params.Initialize(extension, &failure_message) ||
+        !params.IsValidAsRequest(&failure_message)) {
+      // We decline unknown / malformed extensions.
       continue;
     }
-
-    const std::vector<WebSocketExtension::Parameter>& parameters =
-        extension.parameters();
-    for (const auto& param : parameters) {
-      const std::string& name = param.name();
-      // TODO(tyoshino): Fail the connection when an invalid value is given.
-      if (name == "client_max_window_bits") {
-        *has_client_window_bits = true;
-        if (param.HasValue()) {
-          int bits = 0;
-          if (base::StringToInt(param.value(), &bits) && bits >= 8 &&
-              bits <= 15) {
-            *client_window_bits = bits;
-          }
-        }
-      }
-      if (name == "server_max_window_bits" && param.HasValue()) {
-        int bits = 0;
-        if (base::StringToInt(param.value(), &bits) && bits >= 8 && bits <= 15)
-          *server_window_bits = bits;
-      }
-      if (name == "client_no_context_takeover")
-        *client_no_context_takeover = true;
-      if (name == "server_no_context_takeover")
-        *server_no_context_takeover = true;
-    }
-    *deflate = true;
-
+    found = true;
+    offered = params;
     break;
   }
-}
+  if (!found)
+    return make_scoped_ptr(new WebSocketEncoder(FOR_SERVER, nullptr, nullptr));
+
+  response = offered;
+  if (offered.is_client_max_window_bits_specified() &&
+      !offered.has_client_max_window_bits_value()) {
+    // We need to choose one value for the response.
+    response.SetClientMaxWindowBits(8);
+  }
+  DCHECK(response.IsValidAsResponse());
+  DCHECK(offered.IsCompatibleWith(response));
 
-WebSocketEncoder::WebSocketEncoder(bool is_server) : is_server_(is_server) {
+  auto deflater = make_scoped_ptr(
+      new WebSocketDeflater(response.server_context_take_over_mode()));
+  auto inflater = make_scoped_ptr(
+      new WebSocketInflater(kInflaterChunkSize, kInflaterChunkSize));
+  if (!deflater->Initialize(response.PermissiveServerMaxWindowBits()) ||
+      !inflater->Initialize(response.PermissiveClientMaxWindowBits())) {
+    return make_scoped_ptr(new WebSocketEncoder(FOR_SERVER, nullptr, nullptr));
+  }
+  *deflate_parameters = response;
+  return make_scoped_ptr(
+      new WebSocketEncoder(FOR_SERVER, deflater.Pass(), inflater.Pass()));
 }
 
-WebSocketEncoder::WebSocketEncoder(bool is_server,
-                                   int deflate_bits,
-                                   int inflate_bits,
-                                   bool no_context_takeover)
-    : is_server_(is_server) {
-  deflater_.reset(new WebSocketDeflater(
-      no_context_takeover ? WebSocketDeflater::DO_NOT_TAKE_OVER_CONTEXT
-                          : WebSocketDeflater::TAKE_OVER_CONTEXT));
-  inflater_.reset(
-      new WebSocketInflater(kInflaterChunkSize, kInflaterChunkSize));
+// static
+WebSocketEncoder* WebSocketEncoder::CreateClient(
+    const std::string& response_extensions) {
+  // TODO(yhirano): Add a way to return an error.
 
-  if (!deflater_->Initialize(deflate_bits) ||
-      !inflater_->Initialize(inflate_bits)) {
-    // Disable deflate support.
-    deflater_.reset();
-    inflater_.reset();
+  WebSocketExtensionParser parser;
+  if (!parser.Parse(response_extensions)) {
+    // Parse error.
+    return new WebSocketEncoder(FOR_CLIENT, nullptr, nullptr);
+  }
+  if (parser.extensions().size() != 1) {
+    // Only permessage-deflate extension is supported.
+    return new WebSocketEncoder(FOR_CLIENT, nullptr, nullptr);
+  }
+  const auto& extension = parser.extensions()[0];
+  WebSocketDeflateParameters params;
+  std::string failure_message;
+  if (!params.Initialize(extension, &failure_message) ||
+      !params.IsValidAsResponse(&failure_message)) {
+    return new WebSocketEncoder(FOR_CLIENT, nullptr, nullptr);
+  }
+
+  auto deflater = make_scoped_ptr(
+      new WebSocketDeflater(params.client_context_take_over_mode()));
+  auto inflater = make_scoped_ptr(
+      new WebSocketInflater(kInflaterChunkSize, kInflaterChunkSize));
+  if (!deflater->Initialize(params.PermissiveClientMaxWindowBits()) ||
+      !inflater->Initialize(params.PermissiveServerMaxWindowBits())) {
+    return new WebSocketEncoder(FOR_CLIENT, nullptr, nullptr);
   }
-}
 
-WebSocketEncoder::~WebSocketEncoder() {
+  return new WebSocketEncoder(FOR_CLIENT, deflater.Pass(), inflater.Pass());
 }
 
+WebSocketEncoder::WebSocketEncoder(Type type,
+                                   scoped_ptr<WebSocketDeflater> deflater,
+                                   scoped_ptr<WebSocketInflater> inflater)
+    : is_server_(type == FOR_SERVER),

[dgozman, 2015/09/15 17:26:19]

Let's go ahead and change |is_server_| to |type_|.

[yhirano, 2015/09/16 05:28:17]

Done.

+      deflater_(deflater.Pass()),
+      inflater_(inflater.Pass()) {}
+
+WebSocketEncoder::~WebSocketEncoder() {}
+
 WebSocket::ParseResult WebSocketEncoder::DecodeFrame(
     const base::StringPiece& frame,
     int* bytes_consumed,