Add IncludeForRequestURL method to CanonicalCookie.

Move the code for checking whether to include a cookie or not from the CookieMonster to the new method in CanonicalCookie. This means the new method IncludeForRequestURL returns true when the cookie should be included for the given |url| and cookie options.
Change the CookieMonster to use the IncludeForRequestURL method of CanonicalCookie to test whether a cookie should be included or not.

TEST=CanonicalCookieTest.IncludeForRequestURL  (net_unittests)
BUG=none

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@172668 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 82 insertions, 18 deletions

diff --git a/net/cookies/canonical_cookie.cc b/net/cookies/canonical_cookie.cc
index 83e0e60..4b864db 100644
--- a/net/cookies/canonical_cookie.cc
+++ b/net/cookies/canonical_cookie.cc
@@ -371,6 +371,26 @@ bool CanonicalCookie::IsDomainMatch(const std::string& host) const {
                        domain_.length(), domain_) == 0);
 }
 
+bool CanonicalCookie::IncludeForRequestURL(const GURL& url,
+                                           const CookieOptions& options) const {
+  // Filter out HttpOnly cookies, per options.
+  if (options.exclude_httponly() && IsHttpOnly())
+    return false;
+  // Secure cookies should not be included in requests for URLs with an
+  // insecure scheme.
+  if (IsSecure() && !url.SchemeIsSecure())
+    return false;
+  // Don't include cookies for requests that don't apply to the cookie domain.
+  if (!IsDomainMatch(url.host()))
+    return false;
+  // Don't include cookies for requests with a url path that does not path
+  // match the cookie-path.
+  if (!IsOnPath(url.path()))
+    return false;
+
+  return true;
+}
+
 std::string CanonicalCookie::DebugString() const {
   return base::StringPrintf(
       "name: %s value: %s domain: %s path: %s creation: %"
diff --git a/net/cookies/canonical_cookie.h b/net/cookies/canonical_cookie.h
index 0612991..614b8e9 100644
--- a/net/cookies/canonical_cookie.h
+++ b/net/cookies/canonical_cookie.h
@@ -110,12 +110,24 @@ class NET_EXPORT CanonicalCookie {
     last_access_date_ = date;
   }
 
+  // Returns true if the given |url_path| path-matches the cookie-path as
+  // described in section 5.1.4 in RFC 6265.
   bool IsOnPath(const std::string& url_path) const;
+
+  // Returns true if the cookie domain matches the given |host| as described in
+  // section 5.1.3 of RFC 6265.
   bool IsDomainMatch(const std::string& host) const;
 
+  // Returns true if the cookie should be included for the given request |url|.
+  // HTTP only cookies can be filter by using appropriate cookie |options|.
+  // PLEASE NOTE that this method does not check whether a cookie is expired or
+  // not!
+  bool IncludeForRequestURL(const GURL& url,
+                            const CookieOptions& options) const;
+
   std::string DebugString() const;
 
-  // Returns the cookie source when cookies are set for |url|.  This function
+  // Returns the cookie source when cookies are set for |url|. This function
   // is public for unit test purposes only.
   static std::string GetCookieSourceFromURL(const GURL& url);
   static std::string CanonPath(const GURL& url, const ParsedCookie& pc);
diff --git a/net/cookies/canonical_cookie_unittest.cc b/net/cookies/canonical_cookie_unittest.cc
index e4f0048..3c31316 100644
--- a/net/cookies/canonical_cookie_unittest.cc
+++ b/net/cookies/canonical_cookie_unittest.cc
@@ -271,4 +271,48 @@ TEST(CanonicalCookieTest, IsOnPath) {
   EXPECT_TRUE(cookie->IsOnPath("/test/sample/bar.html"));
 }
 
+TEST(CanonicalCookieTest, IncludeForRequestURL) {
+  GURL url("http://www.example.com");
+  base::Time creation_time = base::Time::Now();
+  CookieOptions options;
+
+  scoped_ptr<CanonicalCookie> cookie(
+      CanonicalCookie::Create(url, "A=2", creation_time, options));
+  EXPECT_TRUE(cookie->IncludeForRequestURL(url, options));
+  EXPECT_TRUE(cookie->IncludeForRequestURL(
+      GURL("http://www.example.com/foo/bar"), options));
+  EXPECT_TRUE(cookie->IncludeForRequestURL(
+      GURL("https://www.example.com/foo/bar"), options));
+  EXPECT_FALSE(cookie->IncludeForRequestURL(GURL("https://sub.example.com"),
+                                            options));
+  EXPECT_FALSE(cookie->IncludeForRequestURL(GURL("https://sub.www.example.com"),
+                                            options));
+
+  // Test that cookie with a cookie path that does not match the url path are
+  // not included.
+  cookie.reset(CanonicalCookie::Create(url, "A=2; Path=/foo/bar", creation_time,
+                                       options));
+  EXPECT_FALSE(cookie->IncludeForRequestURL(url, options));
+  EXPECT_TRUE(cookie->IncludeForRequestURL(
+      GURL("http://www.example.com/foo/bar/index.html"), options));
+
+  // Test that a secure cookie is not included for a non secure URL.
+  GURL secure_url("https://www.example.com");
+  cookie.reset(CanonicalCookie::Create(secure_url, "A=2; Secure", creation_time,
+                                       options));
+  EXPECT_TRUE(cookie->IsSecure());
+  EXPECT_TRUE(cookie->IncludeForRequestURL(secure_url, options));
+  EXPECT_FALSE(cookie->IncludeForRequestURL(url, options));
+
+  // Test that http only cookies are only included if the include httponly flag
+  // is set on the cookie options.
+  options.set_include_httponly();
+  cookie.reset(
+      CanonicalCookie::Create(url, "A=2; HttpOnly", creation_time, options));
+  EXPECT_TRUE(cookie->IsHttpOnly());
+  EXPECT_TRUE(cookie->IncludeForRequestURL(url, options));
+  options.set_exclude_httponly();
+  EXPECT_FALSE(cookie->IncludeForRequestURL(url, options));
+}
+
 }  // namespace net
diff --git a/net/cookies/cookie_monster.cc b/net/cookies/cookie_monster.cc
index 3397628..0285a55 100644
--- a/net/cookies/cookie_monster.cc
+++ b/net/cookies/cookie_monster.cc
@@ -1602,9 +1602,6 @@ void CookieMonster::FindCookiesForKey(
     std::vector<CanonicalCookie*>* cookies) {
   lock_.AssertAcquired();
 
-  const std::string host(url.host());
-  bool secure = url.SchemeIsSecure();
-
   for (CookieMapItPair its = cookies_.equal_range(key);
        its.first != its.second; ) {
     CookieMap::iterator curit = its.first;
@@ -1617,22 +1614,13 @@ void CookieMonster::FindCookiesForKey(
       continue;
     }
 
-    // Filter out HttpOnly cookies, per options.
-    if (options.exclude_httponly() && cc->IsHttpOnly())
-      continue;
-
-    // Filter out secure cookies unless we're https.
-    if (!secure && cc->IsSecure())
-      continue;
-
-    // Filter out cookies that don't apply to this domain.
-    if (!cc->IsDomainMatch(host))
-      continue;
-
-    if (!cc->IsOnPath(url.path()))
+    // Filter out cookies that should not be included for a request to the
+    // given |url|. HTTP only cookies are filtered depending on the passed
+    // cookie |options|.
+    if (!cc->IncludeForRequestURL(url, options))
       continue;
 
-    // Add this cookie to the set of matching cookies.  Update the access
+    // Add this cookie to the set of matching cookies. Update the access
     // time if we've been requested to do so.
     if (update_access_time) {
       InternalUpdateCookieAccessTime(cc, current);