Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(813)

Unified Diff: net/cookies/parsed_cookie.cc

Issue 15897003: Added UMA measurement for how often we see control characters during cookie parsing. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Added comment to file explaining UMA collection and fixed erikwright nit. Created 7 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/cookies/parsed_cookie.cc
diff --git a/net/cookies/parsed_cookie.cc b/net/cookies/parsed_cookie.cc
index 8d382152b32c7e12e2df45834521c4bb39c56111..d82a091a58316fddef65844ca6a3c023e97b9c49 100644
--- a/net/cookies/parsed_cookie.cc
+++ b/net/cookies/parsed_cookie.cc
@@ -45,8 +45,19 @@
#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 a
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";
@@ -182,20 +193,26 @@ CookiePriority ParsedCookie::Priority() const {
}
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;
}
@@ -337,6 +354,9 @@ std::string ParsedCookie::ParseValueString(const std::string& value) {
// 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
@@ -384,6 +404,11 @@ void ParsedCookie::ParseTokenValuePairs(const std::string& cookie_line) {
// 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);
@@ -394,6 +419,10 @@ void ParsedCookie::ParseTokenValuePairs(const std::string& cookie_line) {
if (it != end)
++it;
}
+
+ UMA_HISTOGRAM_BOOLEAN("Cookies.ParsedInvalidControlCharacter",
+ parsed_invalid_control_char);
+ UMA_HISTOGRAM_BOOLEAN("Cookies.ParsedInvalidToken", parsed_invalid_token);
}
void ParsedCookie::SetupAttributes() {
@@ -444,8 +473,11 @@ bool ParsedCookie::SetBool(size_t* index,
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) {
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698