Skip to content

Commit

Permalink
Remove control character cookie UMA stats.
Browse files Browse the repository at this point in the history
BUG=238041

Review URL: https://chromiumcodereview.appspot.com/23503084

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@224583 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
jww@chromium.org committed Sep 21, 2013
1 parent 4d64f28 commit b3d3b4d
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 41 deletions.
44 changes: 3 additions & 41 deletions net/cookies/parsed_cookie.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,8 @@
#include "net/cookies/parsed_cookie.h"

#include "base/logging.h"
#include "base/metrics/histogram.h"
#include "base/strings/string_util.h"

// TODO(jww): We are collecting several UMA statistics in this file, and they
// relate to 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";
Expand Down Expand Up @@ -198,9 +186,7 @@ CookiePriority ParsedCookie::Priority() const {
}

bool ParsedCookie::SetName(const std::string& name) {
bool valid_token = IsValidToken(name);
UMA_HISTOGRAM_BOOLEAN("Cookie.SetNameVaildity", valid_token);
if (!valid_token)
if (!IsValidToken(name))
return false;
if (pairs_.empty())
pairs_.push_back(std::make_pair("", ""));
Expand All @@ -209,10 +195,7 @@ bool ParsedCookie::SetName(const std::string& name) {
}

bool ParsedCookie::SetValue(const std::string& value) {
bool valid_cookie_value = IsValidCookieValue(value);
UMA_HISTOGRAM_BOOLEAN("Cookie.SetValueCookieValueValidity",
valid_cookie_value);
if (!valid_cookie_value)
if (!IsValidCookieValue(value))
return false;
if (pairs_.empty())
pairs_.push_back(std::make_pair("", ""));
Expand Down Expand Up @@ -358,15 +341,6 @@ std::string ParsedCookie::ParseValueString(const std::string& value) {

// Parse all token/value pairs and populate pairs_.
void ParsedCookie::ParseTokenValuePairs(const std::string& cookie_line) {
enum ParsedCookieStatus {
PARSED_COOKIE_STATUS_NOTHING = 0x0,
PARSED_COOKIE_STATUS_CONTROL_CHAR = 0x1,
PARSED_COOKIE_STATUS_INVALID = 0x2,
PARSED_COOKIE_STATUS_BOTH =
PARSED_COOKIE_STATUS_CONTROL_CHAR | PARSED_COOKIE_STATUS_INVALID
};
int parsed_cookie_status = PARSED_COOKIE_STATUS_NOTHING;

pairs_.clear();

// Ok, here we go. We should be expecting to be starting somewhere
Expand Down Expand Up @@ -415,11 +389,6 @@ 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_cookie_status |= PARSED_COOKIE_STATUS_CONTROL_CHAR;
if (!IsValidToken(pair.second))
parsed_cookie_status |= PARSED_COOKIE_STATUS_INVALID;

// From RFC2109: "Attributes (names) (attr) are case-insensitive."
if (pair_num != 0)
StringToLowerASCII(&pair.first);
Expand All @@ -438,9 +407,6 @@ void ParsedCookie::ParseTokenValuePairs(const std::string& cookie_line) {
if (it != end)
++it;
}

UMA_HISTOGRAM_ENUMERATION("Cookie.ParsedCookieStatus", parsed_cookie_status,
PARSED_COOKIE_STATUS_BOTH + 1);
}

void ParsedCookie::SetupAttributes() {
Expand Down Expand Up @@ -491,11 +457,7 @@ bool ParsedCookie::SetBool(size_t* index,
bool ParsedCookie::SetAttributePair(size_t* index,
const std::string& key,
const std::string& value) {
bool valid_attribute_pair = IsValidToken(key) &&
IsValidCookieAttributeValue(value);
UMA_HISTOGRAM_BOOLEAN("Cookie.SetAttributePairCharsValidity",
valid_attribute_pair);
if (!valid_attribute_pair)
if (!(IsValidToken(key) && IsValidCookieAttributeValue(value)))
return false;
if (!IsValid())
return false;
Expand Down
20 changes: 20 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1381,6 +1381,10 @@ other types of suffix sets.
</histogram>

<histogram name="Cookie.ParsedCookieStatus" enum="ParsedCookieStatus">
<obsolete>
Deprecated as of 9/2013. Experiment to measure control characters in cookies
is finished.
</obsolete>
<summary>
When parsing a cookie, indicates if control characters were present in any
of the cookie values and if any of the cookie values were invalid.
Expand All @@ -1400,6 +1404,10 @@ other types of suffix sets.
</histogram>

<histogram name="Cookie.SetAttributePairCharsValidity" enum="BooleanValid">
<obsolete>
Deprecated as of 9/2013. Experiment to measure control characters in cookies
is finished.
</obsolete>
<summary>
Indicates whether a cookie attribute pair was set with both a valid key and
a valid attribute value or not. For the key, this implies that it was a
Expand All @@ -1412,6 +1420,10 @@ other types of suffix sets.
</histogram>

<histogram name="Cookie.SetNameValidity" enum="BooleanValid">
<obsolete>
Deprecated as of 9/2013. Experiment to measure control characters in cookies
is finished.
</obsolete>
<summary>
Indicates whether a cookie name was set with a valid token. A valid token is
defined in Section 2.2 of RFC2616 which specifies a token must have no
Expand All @@ -1422,6 +1434,10 @@ other types of suffix sets.
</histogram>

<histogram name="Cookie.SetValueCookieValueValidity" enum="BooleanValid">
<obsolete>
Deprecated as of 9/2013. Experiment to measure control characters in cookies
is finished.
</obsolete>
<summary>
Indicates whether a cookie value was valid or invalid when there was an
attempt to set it, where a valid value is defined in RFC 6265 as ASCII
Expand Down Expand Up @@ -23967,6 +23983,10 @@ other types of suffix sets.
</enum>

<enum name="ParsedCookieStatus" type="int">
<obsolete>
Deprecated as of 9/2013. Experiment to measure control characters in cookies
is finished.
</obsolete>
<int value="0" label="All cookie values valid and without control chars"/>
<int value="1" label="Cookie contains control chars"/>
<int value="2" label="Cookie is invalid"/>
Expand Down

0 comments on commit b3d3b4d

Please sign in to comment.