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

Side by Side 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 TODO to comment regarding UMA stats collection Created 7 years, 6 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 unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 // Portions of this code based on Mozilla: 5 // Portions of this code based on Mozilla:
6 // (netwerk/cookie/src/nsCookieService.cpp) 6 // (netwerk/cookie/src/nsCookieService.cpp)
7 /* ***** BEGIN LICENSE BLOCK ***** 7 /* ***** BEGIN LICENSE BLOCK *****
8 * Version: MPL 1.1/GPL 2.0/LGPL 2.1 8 * Version: MPL 1.1/GPL 2.0/LGPL 2.1
9 * 9 *
10 * The contents of this file are subject to the Mozilla Public License Version 10 * The contents of this file are subject to the Mozilla Public License Version
(...skipping 27 matching lines...) Expand all
38 * decision by deleting the provisions above and replace them with the notice 38 * decision by deleting the provisions above and replace them with the notice
39 * and other provisions required by the GPL or the LGPL. If you do not delete 39 * and other provisions required by the GPL or the LGPL. If you do not delete
40 * the provisions above, a recipient may use your version of this file under 40 * the provisions above, a recipient may use your version of this file under
41 * the terms of any one of the MPL, the GPL or the LGPL. 41 * the terms of any one of the MPL, the GPL or the LGPL.
42 * 42 *
43 * ***** END LICENSE BLOCK ***** */ 43 * ***** END LICENSE BLOCK ***** */
44 44
45 #include "net/cookies/parsed_cookie.h" 45 #include "net/cookies/parsed_cookie.h"
46 46
47 #include "base/logging.h" 47 #include "base/logging.h"
48 #include "base/metrics/histogram.h"
48 #include "base/string_util.h" 49 #include "base/string_util.h"
49 50
51 // TODO(jww): We are collecting several UMA statistics in this file, and they
52 // relate to http://crbug.com/238041. We are measuring stats related to control
53 // characters in cookies because, currently, we allow control characters in a
54 // variety of scenarios where various RFCs theoretically disallow them. These
55 // control characters have the potential to cause problems with certain web
56 // servers that reject HTTP requests that contain cookies with control
57 // characters. We are measuring whether disallowing such cookies would have a
58 // notable impact on our users. We want to collect these stats through 1 stable
59 // release, so these UMA stats should remain at least through the M29
60 // branch-point.
61
50 namespace { 62 namespace {
51 63
52 const char kPathTokenName[] = "path"; 64 const char kPathTokenName[] = "path";
53 const char kDomainTokenName[] = "domain"; 65 const char kDomainTokenName[] = "domain";
54 const char kExpiresTokenName[] = "expires"; 66 const char kExpiresTokenName[] = "expires";
55 const char kMaxAgeTokenName[] = "max-age"; 67 const char kMaxAgeTokenName[] = "max-age";
56 const char kSecureTokenName[] = "secure"; 68 const char kSecureTokenName[] = "secure";
57 const char kHttpOnlyTokenName[] = "httponly"; 69 const char kHttpOnlyTokenName[] = "httponly";
58 const char kPriorityTokenName[] = "priority"; 70 const char kPriorityTokenName[] = "priority";
59 71
(...skipping 115 matching lines...) Expand 10 before | Expand all | Expand 10 after
175 bool ParsedCookie::IsValid() const { 187 bool ParsedCookie::IsValid() const {
176 return !pairs_.empty(); 188 return !pairs_.empty();
177 } 189 }
178 190
179 CookiePriority ParsedCookie::Priority() const { 191 CookiePriority ParsedCookie::Priority() const {
180 return (priority_index_ == 0) ? COOKIE_PRIORITY_DEFAULT : 192 return (priority_index_ == 0) ? COOKIE_PRIORITY_DEFAULT :
181 StringToCookiePriority(pairs_[priority_index_].second); 193 StringToCookiePriority(pairs_[priority_index_].second);
182 } 194 }
183 195
184 bool ParsedCookie::SetName(const std::string& name) { 196 bool ParsedCookie::SetName(const std::string& name) {
185 if (!IsValidToken(name)) 197 if (!IsValidToken(name)) {
198 UMA_HISTOGRAM_BOOLEAN("Cookies.SetNameInvalidToken", true);
186 return false; 199 return false;
200 }
187 if (pairs_.empty()) 201 if (pairs_.empty())
188 pairs_.push_back(std::make_pair("", "")); 202 pairs_.push_back(std::make_pair("", ""));
189 pairs_[0].first = name; 203 pairs_[0].first = name;
204 UMA_HISTOGRAM_BOOLEAN("Cookies.SetNameInvalidToken", false);
190 return true; 205 return true;
191 } 206 }
192 207
193 bool ParsedCookie::SetValue(const std::string& value) { 208 bool ParsedCookie::SetValue(const std::string& value) {
194 if (!IsValidCookieValue(value)) 209 if (!IsValidCookieValue(value)) {
210 UMA_HISTOGRAM_BOOLEAN("Cookies.SetValueInvalidCookieValue", true);
195 return false; 211 return false;
212 }
196 if (pairs_.empty()) 213 if (pairs_.empty())
197 pairs_.push_back(std::make_pair("", "")); 214 pairs_.push_back(std::make_pair("", ""));
198 pairs_[0].second = value; 215 pairs_[0].second = value;
216 UMA_HISTOGRAM_BOOLEAN("Cookies.SetValueInvalidCookieValue", false);
199 return true; 217 return true;
200 } 218 }
201 219
202 bool ParsedCookie::SetPath(const std::string& path) { 220 bool ParsedCookie::SetPath(const std::string& path) {
203 return SetString(&path_index_, kPathTokenName, path); 221 return SetString(&path_index_, kPathTokenName, path);
204 } 222 }
205 223
206 bool ParsedCookie::SetDomain(const std::string& domain) { 224 bool ParsedCookie::SetDomain(const std::string& domain) {
207 return SetString(&domain_index_, kDomainTokenName, domain); 225 return SetString(&domain_index_, kDomainTokenName, domain);
208 } 226 }
(...skipping 121 matching lines...) Expand 10 before | Expand all | Expand 10 after
330 std::string::const_iterator it = value.begin(); 348 std::string::const_iterator it = value.begin();
331 std::string::const_iterator end = FindFirstTerminator(value); 349 std::string::const_iterator end = FindFirstTerminator(value);
332 350
333 std::string::const_iterator value_start, value_end; 351 std::string::const_iterator value_start, value_end;
334 ParseValue(&it, end, &value_start, &value_end); 352 ParseValue(&it, end, &value_start, &value_end);
335 return std::string(value_start, value_end); 353 return std::string(value_start, value_end);
336 } 354 }
337 355
338 // Parse all token/value pairs and populate pairs_. 356 // Parse all token/value pairs and populate pairs_.
339 void ParsedCookie::ParseTokenValuePairs(const std::string& cookie_line) { 357 void ParsedCookie::ParseTokenValuePairs(const std::string& cookie_line) {
358 bool parsed_invalid_control_char = false;
359 bool parsed_invalid_token = false;
360
340 pairs_.clear(); 361 pairs_.clear();
341 362
342 // Ok, here we go. We should be expecting to be starting somewhere 363 // Ok, here we go. We should be expecting to be starting somewhere
343 // before the cookie line, not including any header name... 364 // before the cookie line, not including any header name...
344 std::string::const_iterator start = cookie_line.begin(); 365 std::string::const_iterator start = cookie_line.begin();
345 std::string::const_iterator it = start; 366 std::string::const_iterator it = start;
346 367
347 // TODO(erikwright): Make sure we're stripping \r\n in the network code. 368 // TODO(erikwright): Make sure we're stripping \r\n in the network code.
348 // Then we can log any unexpected terminators. 369 // Then we can log any unexpected terminators.
349 std::string::const_iterator end = FindFirstTerminator(cookie_line); 370 std::string::const_iterator end = FindFirstTerminator(cookie_line);
(...skipping 27 matching lines...) Expand all
377 pair.first = std::string(token_start, token_end); 398 pair.first = std::string(token_start, token_end);
378 ++it; // Skip past the '='. 399 ++it; // Skip past the '='.
379 } 400 }
380 401
381 // OK, now try to parse a value. 402 // OK, now try to parse a value.
382 std::string::const_iterator value_start, value_end; 403 std::string::const_iterator value_start, value_end;
383 ParseValue(&it, end, &value_start, &value_end); 404 ParseValue(&it, end, &value_start, &value_end);
384 // OK, we're finished with a Token/Value. 405 // OK, we're finished with a Token/Value.
385 pair.second = std::string(value_start, value_end); 406 pair.second = std::string(value_start, value_end);
386 407
408 if (!IsValidCookieAttributeValue(pair.second))
409 parsed_invalid_control_char = true;
410 if (!IsValidToken(pair.second))
411 parsed_invalid_token = true;
412
387 // From RFC2109: "Attributes (names) (attr) are case-insensitive." 413 // From RFC2109: "Attributes (names) (attr) are case-insensitive."
388 if (pair_num != 0) 414 if (pair_num != 0)
389 StringToLowerASCII(&pair.first); 415 StringToLowerASCII(&pair.first);
390 pairs_.push_back(pair); 416 pairs_.push_back(pair);
391 417
392 // We've processed a token/value pair, we're either at the end of 418 // We've processed a token/value pair, we're either at the end of
393 // the string or a ValueSeparator like ';', which we want to skip. 419 // the string or a ValueSeparator like ';', which we want to skip.
394 if (it != end) 420 if (it != end)
395 ++it; 421 ++it;
396 } 422 }
423
424 UMA_HISTOGRAM_BOOLEAN("Cookies.ParsedInvalidControlCharacter",
425 parsed_invalid_control_char);
426 UMA_HISTOGRAM_BOOLEAN("Cookies.ParsedInvalidToken", parsed_invalid_token);
397 } 427 }
398 428
399 void ParsedCookie::SetupAttributes() { 429 void ParsedCookie::SetupAttributes() {
400 // We skip over the first token/value, the user supplied one. 430 // We skip over the first token/value, the user supplied one.
401 for (size_t i = 1; i < pairs_.size(); ++i) { 431 for (size_t i = 1; i < pairs_.size(); ++i) {
402 if (pairs_[i].first == kPathTokenName) { 432 if (pairs_[i].first == kPathTokenName) {
403 path_index_ = i; 433 path_index_ = i;
404 } else if (pairs_[i].first == kDomainTokenName) { 434 } else if (pairs_[i].first == kDomainTokenName) {
405 domain_index_ = i; 435 domain_index_ = i;
406 } else if (pairs_[i].first == kExpiresTokenName) { 436 } else if (pairs_[i].first == kExpiresTokenName) {
(...skipping 30 matching lines...) Expand all
437 ClearAttributePair(*index); 467 ClearAttributePair(*index);
438 return true; 468 return true;
439 } else { 469 } else {
440 return SetAttributePair(index, key, std::string()); 470 return SetAttributePair(index, key, std::string());
441 } 471 }
442 } 472 }
443 473
444 bool ParsedCookie::SetAttributePair(size_t* index, 474 bool ParsedCookie::SetAttributePair(size_t* index,
445 const std::string& key, 475 const std::string& key,
446 const std::string& value) { 476 const std::string& value) {
447 if (!IsValidToken(key) || !IsValidCookieAttributeValue(value)) 477 if (!IsValidToken(key) || !IsValidCookieAttributeValue(value)) {
478 UMA_HISTOGRAM_BOOLEAN("Cookies.SetAttributePairInvalidChars", true);
448 return false; 479 return false;
480 }
481 UMA_HISTOGRAM_BOOLEAN("Cookies.SetAttributePairInvalidChars", false);
449 if (!IsValid()) 482 if (!IsValid())
450 return false; 483 return false;
451 if (*index) { 484 if (*index) {
452 pairs_[*index].second = value; 485 pairs_[*index].second = value;
453 } else { 486 } else {
454 pairs_.push_back(std::make_pair(key, value)); 487 pairs_.push_back(std::make_pair(key, value));
455 *index = pairs_.size() - 1; 488 *index = pairs_.size() - 1;
456 } 489 }
457 return true; 490 return true;
458 } 491 }
(...skipping 11 matching lines...) Expand all
470 for (size_t i = 0; i < arraysize(indexes); ++i) { 503 for (size_t i = 0; i < arraysize(indexes); ++i) {
471 if (*indexes[i] == index) 504 if (*indexes[i] == index)
472 *indexes[i] = 0; 505 *indexes[i] = 0;
473 else if (*indexes[i] > index) 506 else if (*indexes[i] > index)
474 --*indexes[i]; 507 --*indexes[i];
475 } 508 }
476 pairs_.erase(pairs_.begin() + index); 509 pairs_.erase(pairs_.begin() + index);
477 } 510 }
478 511
479 } // namespace 512 } // namespace
OLDNEW
« 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