Commit cdd57771 authored by Lily Chen's avatar Lily Chen Committed by Commit Bot

Pass CookieAccessSemantics to CookieDeletionInfo::Matches()

Bug: 978172
Change-Id: I744d4fd879a0b4588c9194b28d1e5a4905a67306
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1860301Reviewed-by: default avatarMaksim Orlovich <morlovich@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705675}
parent 7aff89ce
......@@ -90,7 +90,8 @@ CookieDeletionInfo& CookieDeletionInfo::operator=(CookieDeletionInfo&& rhs) =
CookieDeletionInfo& CookieDeletionInfo::operator=(
const CookieDeletionInfo& rhs) = default;
bool CookieDeletionInfo::Matches(const CanonicalCookie& cookie) const {
bool CookieDeletionInfo::Matches(const CanonicalCookie& cookie,
CookieAccessSemantics access_semantics) const {
if (session_control != SessionControl::IGNORE_CONTROL &&
(cookie.IsPersistent() !=
(session_control == SessionControl::PERSISTENT_COOKIES))) {
......@@ -117,7 +118,8 @@ bool CookieDeletionInfo::Matches(const CanonicalCookie& cookie) const {
// cookies associated with the URL are deleted.
if (url.has_value() &&
!cookie
.IncludeForRequestURL(url.value(), CookieOptions::MakeAllInclusive())
.IncludeForRequestURL(url.value(), CookieOptions::MakeAllInclusive(),
access_semantics)
.IsInclude()) {
return false;
}
......
......@@ -11,6 +11,7 @@
#include "base/optional.h"
#include "base/time/time.h"
#include "net/cookies/canonical_cookie.h"
#include "net/cookies/cookie_constants.h"
namespace net {
......@@ -87,9 +88,17 @@ struct NET_EXPORT CookieDeletionInfo {
// |creation_range| AND the |cookie| name is equal to |name|, etc. then true
// will be returned. If not false.
//
// |access_semantics| is the access semantics mode of the cookie at the time
// of the attempted match. This is used to determine whether the cookie
// matches a particular URL based on effective SameSite mode. (But the value
// should not matter because the CookieOptions used for this check includes
// all cookies for a URL regardless of SameSite).
//
// All members are used. See comments above other members for specifics
// about how checking is done for that value.
bool Matches(const CanonicalCookie& cookie) const;
bool Matches(const CanonicalCookie& cookie,
CookieAccessSemantics access_semantics =
CookieAccessSemantics::UNKNOWN) const;
// See comment above for TimeRange::Contains() for more info.
TimeRange creation_range;
......
......@@ -4,7 +4,10 @@
#include "net/cookies/cookie_deletion_info.h"
#include "base/test/scoped_feature_list.h"
#include "net/base/features.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
namespace net {
......@@ -84,7 +87,7 @@ TEST(CookieDeletionInfoTest, CookieDeletionInfoMatchSessionControl) {
/*creation=*/base::Time::Now(),
/*expiration=*/base::Time::Max(),
/*last_access=*/base::Time::Now(),
/*secure=*/false,
/*secure=*/true,
/*httponly=*/false,
CookieSameSite::NO_RESTRICTION,
CookiePriority::COOKIE_PRIORITY_DEFAULT);
......@@ -94,7 +97,7 @@ TEST(CookieDeletionInfoTest, CookieDeletionInfoMatchSessionControl) {
/*creation=*/base::Time::Now(),
/*expiration=*/base::Time(),
/*last_access=*/base::Time::Now(),
/*secure=*/false,
/*secure=*/true,
/*httponly=*/false, CookieSameSite::NO_RESTRICTION,
CookiePriority::COOKIE_PRIORITY_DEFAULT);
......@@ -119,7 +122,7 @@ TEST(CookieDeletionInfoTest, CookieDeletionInfoMatchHost) {
/*creation=*/base::Time::Now(),
/*expiration=*/base::Time::Max(),
/*last_access=*/base::Time::Now(),
/*secure=*/false,
/*secure=*/true,
/*httponly=*/false,
CookieSameSite::NO_RESTRICTION,
CookiePriority::COOKIE_PRIORITY_DEFAULT);
......@@ -129,7 +132,7 @@ TEST(CookieDeletionInfoTest, CookieDeletionInfoMatchHost) {
/*creation=*/base::Time::Now(),
/*expiration=*/base::Time::Max(),
/*last_access=*/base::Time::Now(),
/*secure=*/false,
/*secure=*/true,
/*httponly=*/false,
CookieSameSite::NO_RESTRICTION,
CookiePriority::COOKIE_PRIORITY_DEFAULT);
......@@ -160,7 +163,7 @@ TEST(CookieDeletionInfoTest, CookieDeletionInfoMatchName) {
/*creation=*/base::Time::Now(),
/*expiration=*/base::Time::Max(),
/*last_access=*/base::Time::Now(),
/*secure=*/false,
/*secure=*/true,
/*httponly=*/false, CookieSameSite::NO_RESTRICTION,
CookiePriority::COOKIE_PRIORITY_DEFAULT);
CanonicalCookie cookie2("cookie2-name", "cookie2-value",
......@@ -168,7 +171,7 @@ TEST(CookieDeletionInfoTest, CookieDeletionInfoMatchName) {
/*creation=*/base::Time::Now(),
/*expiration=*/base::Time::Max(),
/*last_access=*/base::Time::Now(),
/*secure=*/false,
/*secure=*/true,
/*httponly=*/false, CookieSameSite::NO_RESTRICTION,
CookiePriority::COOKIE_PRIORITY_DEFAULT);
......@@ -184,7 +187,7 @@ TEST(CookieDeletionInfoTest, CookieDeletionInfoMatchValue) {
/*creation=*/base::Time::Now(),
/*expiration=*/base::Time::Max(),
/*last_access=*/base::Time::Now(),
/*secure=*/false,
/*secure=*/true,
/*httponly=*/false, CookieSameSite::NO_RESTRICTION,
CookiePriority::COOKIE_PRIORITY_DEFAULT);
CanonicalCookie cookie2("cookie2-name", "cookie2-value",
......@@ -192,7 +195,7 @@ TEST(CookieDeletionInfoTest, CookieDeletionInfoMatchValue) {
/*creation=*/base::Time::Now(),
/*expiration=*/base::Time::Max(),
/*last_access=*/base::Time::Now(),
/*secure=*/false,
/*secure=*/true,
/*httponly=*/false, CookieSameSite::NO_RESTRICTION,
CookiePriority::COOKIE_PRIORITY_DEFAULT);
......@@ -208,7 +211,7 @@ TEST(CookieDeletionInfoTest, CookieDeletionInfoMatchUrl) {
/*creation=*/base::Time::Now(),
/*expiration=*/base::Time::Max(),
/*last_access=*/base::Time::Now(),
/*secure=*/false,
/*secure=*/true,
/*httponly=*/false, CookieSameSite::NO_RESTRICTION,
CookiePriority::COOKIE_PRIORITY_DEFAULT);
......@@ -236,7 +239,7 @@ TEST(CookieDeletionInfoTest, CookieDeletionInfoDomainMatchesDomain) {
/*creation=*/base::Time::FromDoubleT(kTestMinEpoch + 1),
/*expiration=*/base::Time::Max(),
/*last_access=*/base::Time::FromDoubleT(kTestMinEpoch + 1),
/*secure=*/false,
/*secure=*/true,
/*httponly=*/false,
/*same_site=*/CookieSameSite::NO_RESTRICTION,
/*priority=*/CookiePriority::COOKIE_PRIORITY_DEFAULT);
......@@ -318,4 +321,38 @@ TEST(CookieDeletionInfoTest, CookieDeletionInfoMatchesDomainList) {
EXPECT_FALSE(delete_info.Matches(create_cookie("outside.com")));
}
// Test that Matches() works regardless of the cookie access semantics (because
// the IncludeForRequestURL call uses CookieOptions::MakeAllInclusive).
TEST(CookieDeletionInfoTest, MatchesWithCookieAccessSemantics) {
// Cookie with unspecified SameSite.
auto cookie =
CanonicalCookie::Create(GURL("https://www.example.com"), "cookie=1",
base::Time::Now(), base::nullopt);
{
// With SameSite features off.
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(features::kSameSiteByDefaultCookies);
CookieDeletionInfo delete_info;
delete_info.url = GURL("https://www.example.com/path");
EXPECT_TRUE(delete_info.Matches(*cookie)); // defaults to UNKNOWN
EXPECT_TRUE(delete_info.Matches(*cookie, CookieAccessSemantics::UNKNOWN));
EXPECT_TRUE(delete_info.Matches(*cookie, CookieAccessSemantics::LEGACY));
EXPECT_TRUE(delete_info.Matches(*cookie, CookieAccessSemantics::NONLEGACY));
}
{
// With SameSite features on.
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(features::kSameSiteByDefaultCookies);
CookieDeletionInfo delete_info;
delete_info.url = GURL("https://www.example.com/path");
EXPECT_TRUE(delete_info.Matches(*cookie)); // defaults to UNKNOWN
EXPECT_TRUE(delete_info.Matches(*cookie, CookieAccessSemantics::UNKNOWN));
EXPECT_TRUE(delete_info.Matches(*cookie, CookieAccessSemantics::LEGACY));
EXPECT_TRUE(delete_info.Matches(*cookie, CookieAccessSemantics::NONLEGACY));
}
}
} // namespace net
......@@ -663,7 +663,7 @@ void CookieMonster::DeleteAllMatchingInfo(CookieDeletionInfo delete_info,
CanonicalCookie* cc = curit->second.get();
++it;
if (delete_info.Matches(*cc)) {
if (delete_info.Matches(*cc, GetAccessSemanticsForCookie(*cc))) {
InternalDeleteCookie(curit, true, /*sync_to_store*/
DELETE_COOKIE_EXPLICIT);
++num_deleted;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment