Support parsing a Sec-WebSocket-Extensions header with multiple elements

net/websockets/websocket_extension_parser.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/websockets/websocket_extension_parser.h"
 
 #include "base/strings/string_util.h"
 
 namespace net {
 
 WebSocketExtensionParser::WebSocketExtensionParser() {}
 
 WebSocketExtensionParser::~WebSocketExtensionParser() {}
 
 void WebSocketExtensionParser::Parse(const char* data, size_t size) {
   current_ = data;
   end_ = data + size;
   has_error_ = false;
+  extensions_.clear();
 
-  ConsumeExtension(&extension_);
-  if (has_error_) return;
-  ConsumeSpaces();
-  has_error_ = has_error_ || (current_ != end_);
+  while (true) {
+    WebSocketExtension extension;
+    ConsumeExtension(&extension);
+    extensions_.push_back(extension);
+    if (has_error_) {

[eroman, 2015/03/31 21:11:12]

Should this be done before the call to extensions_push()?

As an aside, while reading this I find the has_error_ member to be a bit
fragile. I wonder if it would be clearer to have each of the "Consume*()"
methods instead return their success as a boolean, and mark the return value as
something that cannot be ignored to the compiler.

[tyoshino (SeeGerritForStatus), 2015/04/01 04:57:57]

Done.
Will do. Added TODO

+      extensions_.clear();
+      return;

[eroman, 2015/03/31 21:11:12]

I suggest doing a break instead, and leaving the check at the bottom to clear
extensions_ (so it is just done in one place).

[tyoshino (SeeGerritForStatus), 2015/04/01 04:57:57]

Done.

+    }
+
+    ConsumeSpaces();
+    DCHECK(!has_error_);
+
+    if (!ConsumeIfMatch(',')) {
+      break;
+    }
+  }
+
+  has_error_ = has_error_ || current_ != end_;
+  if (has_error_)
+    extensions_.clear();
 }
 
 void WebSocketExtensionParser::Consume(char c) {
   DCHECK(!has_error_);
   ConsumeSpaces();
   DCHECK(!has_error_);
   if (current_ == end_ || c != current_[0]) {
     has_error_ = true;
     return;
   }
@@ skipping 115 matching lines @@
   return (0 <= c && c <= 31) || c == 127;
 }
 
 // static
 bool WebSocketExtensionParser::IsSeparator(char c) {
   const char separators[] = "()<>@,;:\\\"/[]?={} \t";
   return strchr(separators, c) != NULL;
 }
 
 }  // namespace net