Add cookie prefix metrics

net/cookies/canonical_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 29 matching lines @@
  * 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/canonical_cookie.h"
 
 #include "base/basictypes.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
 #include "net/cookies/cookie_util.h"
 #include "net/cookies/parsed_cookie.h"
 #include "url/gurl.h"
 #include "url/url_canon.h"
 
 using base::Time;
 using base::TimeDelta;
 
 namespace net {
@@ skipping 49 matching lines @@
   if (diff != 0)
     return diff;
 
   diff = a.Domain().compare(b.Domain());
   if (diff != 0)
     return diff;
 
   return a.Path().compare(b.Path());
 }
 
-// Returns true if the cookie does not violate any constraints imposed
-// by the cookie name's prefix, as described in
-// https://tools.ietf.org/html/draft-west-cookie-prefixes
-bool IsCookiePrefixValid(const GURL& url, const ParsedCookie& parsed_cookie) {
-  const char kSecurePrefix[] = "$Secure-";
-  const char kHostPrefix[] = "$Host-";
-  if (parsed_cookie.Name().find(kSecurePrefix) == 0)
-    return parsed_cookie.IsSecure() && url.SchemeIsCryptographic();
-  if (parsed_cookie.Name().find(kHostPrefix) == 0) {
-    return parsed_cookie.IsSecure() && url.SchemeIsCryptographic() &&
-           !parsed_cookie.HasDomain() && parsed_cookie.Path() == "/";
-  }
-  return true;
-}
-
 }  // namespace
 
 CanonicalCookie::CanonicalCookie()
     : secure_(false),
       httponly_(false) {
 }
 
 CanonicalCookie::CanonicalCookie(const GURL& url,
                                  const std::string& name,
                                  const std::string& value,
@@ skipping 112 matching lines @@
 
   std::string cookie_path = CanonicalCookie::CanonPath(url, parsed_cookie);
   Time server_time(creation_time);
   if (options.has_server_time())
     server_time = options.server_time();
 
   Time cookie_expires = CanonicalCookie::CanonExpiration(parsed_cookie,
                                                          creation_time,
                                                          server_time);
 
-  if (options.enforce_prefixes() && !IsCookiePrefixValid(url, parsed_cookie)) {
+  CookiePrefix prefix = CanonicalCookie::GetCookiePrefix(parsed_cookie.Name());
+  bool is_cookie_valid =
+      CanonicalCookie::IsCookiePrefixValid(prefix, url, parsed_cookie);
+  CanonicalCookie::RecordCookiePrefixMetrics(prefix, is_cookie_valid);
+  if (options.enforce_prefixes() && !is_cookie_valid) {
     VLOG(kVlogSetCookies)
         << "Create() failed because the cookie violated prefix rules.";
     return nullptr;
   }
 
   return new CanonicalCookie(
       url, parsed_cookie.Name(), parsed_cookie.Value(), cookie_domain,
       cookie_path, creation_time, cookie_expires, creation_time,
       parsed_cookie.IsSecure(), parsed_cookie.IsHttpOnly(),
       parsed_cookie.IsFirstPartyOnly(), parsed_cookie.Priority());
@@ skipping 185 matching lines @@
 
   if (IsSecure() != other.IsSecure())
     return IsSecure();
 
   if (IsHttpOnly() != other.IsHttpOnly())
     return IsHttpOnly();
 
   return Priority() < other.Priority();
 }
 
+// static
+CanonicalCookie::CookiePrefix CanonicalCookie::GetCookiePrefix(
+    const std::string& name) {
+  const char kSecurePrefix[] = "$Secure-";
+  const char kHostPrefix[] = "$Host-";
+  if (name.find(kSecurePrefix) == 0)
+    return CanonicalCookie::COOKIE_PREFIX_SECURE;
+  if (name.find(kHostPrefix) == 0)
+    return CanonicalCookie::COOKIE_PREFIX_HOST;
+  return CanonicalCookie::COOKIE_PREFIX_NONE;
+}
+
+// static
+void CanonicalCookie::RecordCookiePrefixMetrics(
+    CanonicalCookie::CookiePrefix prefix,
+    bool is_cookie_valid) {
+  const char kCookiePrefixHistogram[] = "Cookie.CookiePrefix";
+  const char kCookiePrefixBlockedHistogram[] = "Cookie.CookiePrefixBlocked";
+  UMA_HISTOGRAM_ENUMERATION(kCookiePrefixHistogram, prefix,
+                            CanonicalCookie::COOKIE_PREFIX_LAST);
+  if (!is_cookie_valid) {
+    UMA_HISTOGRAM_ENUMERATION(kCookiePrefixBlockedHistogram, prefix,
+                              CanonicalCookie::COOKIE_PREFIX_LAST);

[Mike West, 2015/11/20 14:43:58]

It might be easier to compare numbers if these were all in one histogram. That
is, "Of all the cookies we saw, X had this prefix and would have been blocked. Y
had this prefix and would have been allowed. Z had no prefix."

I'm totally fine with gathering that information out of two histograms; it's
easy, just marginally more annoying than having all the data in one place.

[estark, 2015/11/20 18:19:32]

It might be because I started trying to write code at 6am but doing it in the
same histogram seemed like it led to slightly uglier code, so I decided to go
for cleaner code at the expense of slightly more annoying histogram.

+  }
+}
+
+// Returns true if the cookie does not violate any constraints imposed
+// by the cookie name's prefix, as described in
+// https://tools.ietf.org/html/draft-west-cookie-prefixes
+//
+// static
+bool CanonicalCookie::IsCookiePrefixValid(CanonicalCookie::CookiePrefix prefix,
+                                          const GURL& url,
+                                          const ParsedCookie& parsed_cookie) {
+  if (prefix == CanonicalCookie::COOKIE_PREFIX_SECURE)
+    return parsed_cookie.IsSecure() && url.SchemeIsCryptographic();
+  if (prefix == CanonicalCookie::COOKIE_PREFIX_HOST) {
+    return parsed_cookie.IsSecure() && url.SchemeIsCryptographic() &&
+           !parsed_cookie.HasDomain() && parsed_cookie.Path() == "/";
+  }
+  return true;
+}
+
 }  // namespace net