Added UMA measurement for how often we see control characters during cookie parsing.

net/cookies/parsed_cookie.cc

 // Copyright (c) 2012 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.
 
 // Portions of this code based on Mozilla:
 //   (netwerk/cookie/src/nsCookieService.cpp)
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
@@ skipping 27 matching lines @@
  * decision by deleting the provisions above and replace them with the notice
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "net/cookies/parsed_cookie.h"
 
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/string_util.h"
 
+// We are collecting several UMA statistics in this file, and they relate to

[erikwright (departed), 2013/05/28 19:59:05]

nit: Prefix with 'TODO(jww): Remove metrics code after M29.'

[jww, 2013/05/28 20:45:47]

Done.

+// http://crbug.com/238041. We are measuring stats related to control characters
+// in cookies because, currently, we allow control characters in a variety of
+// scenarios where various RFCs theoretically disallow them. These control
+// characters have the potential to cause problems with certain web servers that
+// reject HTTP requests that contain cookies with control characters. We are
+// measuring whether disallowing such cookies would have a notable impact on our
+// users. We want to collect these stats through 1 stable release, so these UMA
+// stats should remain at least through the M29 branch-point.
+
 namespace {
 
 const char kPathTokenName[] = "path";
 const char kDomainTokenName[] = "domain";
 const char kExpiresTokenName[] = "expires";
 const char kMaxAgeTokenName[] = "max-age";
 const char kSecureTokenName[] = "secure";
 const char kHttpOnlyTokenName[] = "httponly";
 const char kPriorityTokenName[] = "priority";
 
@@ skipping 115 matching lines @@
 bool ParsedCookie::IsValid() const {
   return !pairs_.empty();
 }
 
 CookiePriority ParsedCookie::Priority() const {
   return (priority_index_ == 0) ? COOKIE_PRIORITY_DEFAULT :
       StringToCookiePriority(pairs_[priority_index_].second);
 }
 
 bool ParsedCookie::SetName(const std::string& name) {
-  if (!IsValidToken(name))
+  if (!IsValidToken(name)) {
+    UMA_HISTOGRAM_BOOLEAN("Cookies.SetNameInvalidToken", true);
     return false;
+  }
   if (pairs_.empty())
     pairs_.push_back(std::make_pair("", ""));
   pairs_[0].first = name;
+  UMA_HISTOGRAM_BOOLEAN("Cookies.SetNameInvalidToken", false);
   return true;
 }
 
 bool ParsedCookie::SetValue(const std::string& value) {
-  if (!IsValidCookieValue(value))
+  if (!IsValidCookieValue(value)) {
+    UMA_HISTOGRAM_BOOLEAN("Cookies.SetValueInvalidCookieValue", true);
     return false;
+  }
   if (pairs_.empty())
     pairs_.push_back(std::make_pair("", ""));
   pairs_[0].second = value;
+  UMA_HISTOGRAM_BOOLEAN("Cookies.SetValueInvalidCookieValue", false);
   return true;
 }
 
 bool ParsedCookie::SetPath(const std::string& path) {
   return SetString(&path_index_, kPathTokenName, path);
 }
 
 bool ParsedCookie::SetDomain(const std::string& domain) {
   return SetString(&domain_index_, kDomainTokenName, domain);
 }
@@ skipping 121 matching lines @@
   std::string::const_iterator it = value.begin();
   std::string::const_iterator end = FindFirstTerminator(value);
 
   std::string::const_iterator value_start, value_end;
   ParseValue(&it, end, &value_start, &value_end);
   return std::string(value_start, value_end);
 }
 
 // Parse all token/value pairs and populate pairs_.
 void ParsedCookie::ParseTokenValuePairs(const std::string& cookie_line) {
+  bool parsed_invalid_control_char = false;
+  bool parsed_invalid_token = false;
+
   pairs_.clear();
 
   // Ok, here we go.  We should be expecting to be starting somewhere
   // before the cookie line, not including any header name...
   std::string::const_iterator start = cookie_line.begin();
   std::string::const_iterator it = start;
 
   // TODO(erikwright): Make sure we're stripping \r\n in the network code.
   // Then we can log any unexpected terminators.
   std::string::const_iterator end = FindFirstTerminator(cookie_line);
@@ skipping 27 matching lines @@
       pair.first = std::string(token_start, token_end);
       ++it;  // Skip past the '='.
     }
 
     // OK, now try to parse a value.
     std::string::const_iterator value_start, value_end;
     ParseValue(&it, end, &value_start, &value_end);
     // OK, we're finished with a Token/Value.
     pair.second = std::string(value_start, value_end);
 
+    if (!IsValidCookieAttributeValue(pair.second))
+      parsed_invalid_control_char = true;
+    if (!IsValidToken(pair.second))
+      parsed_invalid_token = true;
+
     // From RFC2109: "Attributes (names) (attr) are case-insensitive."
     if (pair_num != 0)
       StringToLowerASCII(&pair.first);
     pairs_.push_back(pair);
 
     // We've processed a token/value pair, we're either at the end of
     // the string or a ValueSeparator like ';', which we want to skip.
     if (it != end)
       ++it;
   }
+
+  UMA_HISTOGRAM_BOOLEAN("Cookies.ParsedInvalidControlCharacter",
+                        parsed_invalid_control_char);
+  UMA_HISTOGRAM_BOOLEAN("Cookies.ParsedInvalidToken", parsed_invalid_token);
 }
 
 void ParsedCookie::SetupAttributes() {
   // We skip over the first token/value, the user supplied one.
   for (size_t i = 1; i < pairs_.size(); ++i) {
     if (pairs_[i].first == kPathTokenName) {
       path_index_ = i;
     } else if (pairs_[i].first == kDomainTokenName) {
       domain_index_ = i;
     } else if (pairs_[i].first == kExpiresTokenName) {
@@ skipping 30 matching lines @@
     ClearAttributePair(*index);
     return true;
   } else {
     return SetAttributePair(index, key, std::string());
   }
 }
 
 bool ParsedCookie::SetAttributePair(size_t* index,
                                     const std::string& key,
                                     const std::string& value) {
-  if (!IsValidToken(key) || !IsValidCookieAttributeValue(value))
+  if (!IsValidToken(key) || !IsValidCookieAttributeValue(value)) {
+    UMA_HISTOGRAM_BOOLEAN("Cookies.SetAttributePairInvalidChars", true);
     return false;
+  }
+  UMA_HISTOGRAM_BOOLEAN("Cookies.SetAttributePairInvalidChars", false);
   if (!IsValid())
     return false;
   if (*index) {
     pairs_[*index].second = value;
   } else {
     pairs_.push_back(std::make_pair(key, value));
     *index = pairs_.size() - 1;
   }
   return true;
 }
@@ skipping 11 matching lines @@
   for (size_t i = 0; i < arraysize(indexes); ++i) {
     if (*indexes[i] == index)
       *indexes[i] = 0;
     else if (*indexes[i] > index)
       --*indexes[i];
   }
   pairs_.erase(pairs_.begin() + index);
 }
 
 }  // namespace