Implement and test CanonicalCookie::IsCanonical()

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 32 matching lines @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "net/cookies/canonical_cookie.h"
 
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "net/base/url_util.h"
 #include "net/cookies/cookie_util.h"
 #include "net/cookies/parsed_cookie.h"
 #include "url/gurl.h"
 #include "url/url_canon.h"
+#include "url/url_util.h"
 
 using base::Time;
 using base::TimeDelta;
 
 namespace net {
 
 namespace {
 
 const int kVlogSetCookies = 7;
 
@@ skipping 154 matching lines @@
       url, parsed_cookie.HasPath() ? parsed_cookie.Path() : std::string());
 
   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);
 
-  CookiePrefix prefix = CanonicalCookie::GetCookiePrefix(parsed_cookie.Name());
-  bool is_cookie_valid =
-      CanonicalCookie::IsCookiePrefixValid(prefix, url, parsed_cookie);
-  CanonicalCookie::RecordCookiePrefixMetrics(prefix, is_cookie_valid);
+  CookiePrefix prefix = GetCookiePrefix(parsed_cookie.Name());
+  bool is_cookie_valid = IsCookiePrefixValid(prefix, url, parsed_cookie);
+  RecordCookiePrefixMetrics(prefix, is_cookie_valid);
   if (!is_cookie_valid) {
     VLOG(kVlogSetCookies)
         << "Create() failed because the cookie violated prefix rules.";
     return nullptr;
   }
 
-  return base::WrapUnique(new CanonicalCookie(
+  std::unique_ptr<CanonicalCookie> cc(base::MakeUnique<CanonicalCookie>(
       parsed_cookie.Name(), parsed_cookie.Value(), cookie_domain, cookie_path,
       creation_time, cookie_expires, creation_time, parsed_cookie.IsSecure(),
       parsed_cookie.IsHttpOnly(), parsed_cookie.SameSite(),
       parsed_cookie.Priority()));
+  DCHECK(cc->IsCanonical());
+  return cc;
 }
 
 bool CanonicalCookie::IsEquivalentForSecureCookieMatching(
     const CanonicalCookie& ecc) const {
   return (name_ == ecc.Name() && (ecc.IsDomainMatch(DomainWithoutDot()) ||
                                   IsDomainMatch(ecc.DomainWithoutDot())) &&
           ecc.IsOnPath(Path()));
 }
 
 bool CanonicalCookie::IsOnPath(const std::string& url_path) const {
@@ skipping 136 matching lines @@
 
   if (IsSecure() != other.IsSecure())
     return IsSecure();
 
   if (IsHttpOnly() != other.IsHttpOnly())
     return IsHttpOnly();
 
   return Priority() < other.Priority();
 }
 
+bool CanonicalCookie::IsCanonical() const {
+  if (ParsedCookie::ParseTokenString(name_) != name_ ||
+      ParsedCookie::ParseValueString(value_) != value_ ||
+      ParsedCookie::ParseValueString(domain_) != domain_ ||
+      ParsedCookie::ParseValueString(path_) != path_ ||
+      !ParsedCookie::IsValidCookieAttributeValue(name_) ||
+      !ParsedCookie::IsValidCookieAttributeValue(value_) ||
+      !ParsedCookie::IsValidCookieAttributeValue(domain_) ||
+      !ParsedCookie::IsValidCookieAttributeValue(path_)) {
+    return false;
+  }
+
+  if (!last_access_date_.is_null() &&
+      (creation_date_.is_null() || creation_date_ > last_access_date_)) {

[mmenke, 2017/06/15 21:30:24]

Also, if there's a time change, creation date could be before
last_access_date....yea, I think it makes sense to remove it.

[Randy Smith (Not in Mondays), 2017/06/16 15:25:21]

Done.  I left the requirement that last_access_date_ be null if creation date
is.

+    return false;
+  }
+
+  url::CanonHostInfo canon_host_info;
+  std::string canonical_domain(CanonicalizeHost(domain_, &canon_host_info));
+  // TODO(rdsmith): This specifically allows for empty domains.  The spec
+  // suggests this is invalid (if a domain attribute is empty, the cookie's
+  // domain is set to the canonicalized request host; see
+  // https://tools.ietf.org/html/rfc6265#section-5.3).  However, it is
+  // needed for Chrome extension cookies.
+  // See http://crbug.com/730633 for more information.
+  if (canonical_domain != domain_)
+    return false;
+
+  if (path_.empty() || path_[0] != '/')
+    return false;
+
+  if (GetCookiePrefix(name_) == COOKIE_PREFIX_HOST &&

[mmenke, 2017/06/15 21:30:24]

Check if IsSecure(), and also check if IsSecure() if the prefix is
CanonicalCookie::COOKIE_PREFIX_SECURE?

[Randy Smith (Not in Mondays), 2017/06/16 15:25:21]

Whoops, didn't realize host cookies had to be secure.  Done.
Also good catch.  Done.

(I may have been over clever in how I wrote the tests--feel free to tell me to
break it out into clear, separate clauses.)

+      (path_ != "/" || domain_.empty() || domain_[0] == '.')) {
+    return false;
+  }
+
+  return true;
+}
+
 // static
 CanonicalCookie::CookiePrefix CanonicalCookie::GetCookiePrefix(
     const std::string& name) {
   const char kSecurePrefix[] = "__Secure-";
   const char kHostPrefix[] = "__Host-";
   if (base::StartsWith(name, kSecurePrefix, base::CompareCase::SENSITIVE))
     return CanonicalCookie::COOKIE_PREFIX_SECURE;
   if (base::StartsWith(name, kHostPrefix, base::CompareCase::SENSITIVE))
     return CanonicalCookie::COOKIE_PREFIX_HOST;
   return CanonicalCookie::COOKIE_PREFIX_NONE;
@@ skipping 30 matching lines @@
   return true;
 }
 
 std::string CanonicalCookie::DomainWithoutDot() const {
   if (domain_.empty() || domain_[0] != '.')
     return domain_;
   return domain_.substr(1);
 }
 
 }  // namespace net