Skip to content

Commit

Permalink
[Sheriff] Fix CookieMonster tests
Browse files Browse the repository at this point in the history 
A lot of cookie tests use Apr 18th of this year as hard-coded date for
future cookie expiration. To silence all the bots that are failing
without a real issue in our implementation, increase that date to 2062.

A time based on base::Time::Now should be done as follow-up.

(cherry picked from commit f57c8fe)

Bug: 1317522
Change-Id: Iaa491be103ccb14294adbd531f120b869f7d0814
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3593012
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Owners-Override: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#993633}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3597514
Commit-Queue: Steven Bingler <bingler@chromium.org>
Auto-Submit: Steven Bingler <bingler@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5005@{#54}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
  • Loading branch information
@FHorschig
FHorschig authored and Chromium LUCI CQ committed Apr 20, 2022
1 parent fcbe61f commit 55133ed
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 38 deletions.
41 changes: 20 additions & 21 deletions net/cookies/cookie_monster_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -569,8 +569,8 @@ class CookieMonsterTestBase : public CookieStoreTest<T> {
EXPECT_EQ(expected_secure, num_secure);

// Validate each priority.
size_t expected_count[3] = {
expected_low_count, expected_medium_count, expected_high_count};
size_t expected_count[3] = {expected_low_count, expected_medium_count,
expected_high_count};
for (int i = 0; i < 3; ++i) {
size_t num_for_priority =
surviving_id_list[0][i].size() + surviving_id_list[1][i].size();
Expand Down Expand Up @@ -825,8 +825,7 @@ class CookieMonsterTestBase : public CookieStoreTest<T> {
//
// Round 1 => 10LN; round 2 => none; round 3 => none.
// Round 4 => 21HN; round 5 => none; round 6 => none.
TestPriorityCookieCase(cm.get(), "39LN 1LS 141HN", 30U, 0U, 120U, 149U,
1U);
TestPriorityCookieCase(cm.get(), "39LN 1LS 141HN", 30U, 0U, 120U, 149U, 1U);
// Round 1 => none; round 2 => none; round 3 => 10MN.
// Round 4 => none; round 5 => none; round 6 => 21HS.
TestPriorityCookieCase(cm.get(), "29LN 1LS 59MN 1MS 91HS", 30U, 50U, 70U,
Expand Down Expand Up @@ -1062,7 +1061,7 @@ class DeferredCookieTaskTest : public CookieMonsterTest {

TEST_F(DeferredCookieTaskTest, DeferredGetCookieList) {
DeclareLoadedCookie(http_www_foo_.url(),
"X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=1; path=/; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(3));

GetCookieListCallback call1;
Expand Down Expand Up @@ -1152,7 +1151,7 @@ TEST_F(DeferredCookieTaskTest, DeferredSetAllCookies) {

TEST_F(DeferredCookieTaskTest, DeferredGetAllCookies) {
DeclareLoadedCookie(http_www_foo_.url(),
"X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=1; path=/; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(3));

GetAllCookiesCallback call1;
Expand All @@ -1174,7 +1173,7 @@ TEST_F(DeferredCookieTaskTest, DeferredGetAllCookies) {

TEST_F(DeferredCookieTaskTest, DeferredGetAllForUrlCookies) {
DeclareLoadedCookie(http_www_foo_.url(),
"X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=1; path=/; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(3));

GetCookieListCallback call1;
Expand All @@ -1200,7 +1199,7 @@ TEST_F(DeferredCookieTaskTest, DeferredGetAllForUrlCookies) {

TEST_F(DeferredCookieTaskTest, DeferredGetAllForUrlWithOptionsCookies) {
DeclareLoadedCookie(http_www_foo_.url(),
"X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=1; path=/; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(3));

GetCookieListCallback call1;
Expand All @@ -1226,7 +1225,7 @@ TEST_F(DeferredCookieTaskTest, DeferredGetAllForUrlWithOptionsCookies) {

TEST_F(DeferredCookieTaskTest, DeferredDeleteAllCookies) {
DeclareLoadedCookie(http_www_foo_.url(),
"X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=1; path=/; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(3));

ResultSavingCookieCallback<uint32_t> call1;
Expand Down Expand Up @@ -1363,7 +1362,7 @@ TEST_F(DeferredCookieTaskTest, DeferredDeleteSessionCookies) {
TEST_F(DeferredCookieTaskTest, DeferredTaskOrder) {
cookie_monster_->SetPersistSessionCookies(true);
DeclareLoadedCookie(http_www_foo_.url(),
"X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=1; path=/; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(3));

bool get_cookie_list_callback_was_run = false;
Expand Down Expand Up @@ -1440,7 +1439,7 @@ TEST_F(CookieMonsterTest, TestCookieDeleteAll) {
// Create a persistent cookie.
EXPECT_TRUE(SetCookie(
cm.get(), http_www_foo_.url(),
std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-22 22:50:13 GMT"));
std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-62 22:50:13 GMT"));
ASSERT_EQ(1u, store->commands().size());
EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[0].type);

Expand Down Expand Up @@ -2240,37 +2239,37 @@ TEST_F(CookieMonsterTest, DontImportDuplicateCookies) {
// the import.

AddCookieToList(GURL("http://www.foo.com"),
"X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=1; path=/; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(3), &initial_cookies);

AddCookieToList(GURL("http://www.foo.com"),
"X=2; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=2; path=/; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(1), &initial_cookies);

// ===> This one is the WINNER (biggest creation time). <====
AddCookieToList(GURL("http://www.foo.com"),
"X=3; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=3; path=/; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(4), &initial_cookies);

AddCookieToList(GURL("http://www.foo.com"),
"X=4; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=4; path=/; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now(), &initial_cookies);

// Insert 2 cookies with name "X" on path "/2", with varying creation
// dates. We expect only the most recent one to be preserved the import.

// ===> This one is the WINNER (biggest creation time). <====
AddCookieToList(GURL("http://www.foo.com"),
"X=a1; path=/2; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=a1; path=/2; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(9), &initial_cookies);

AddCookieToList(GURL("http://www.foo.com"),
"X=a2; path=/2; expires=Mon, 18-Apr-22 22:50:14 GMT",
"X=a2; path=/2; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(2), &initial_cookies);

// Insert 1 cookie with name "Y" on path "/".
AddCookieToList(GURL("http://www.foo.com"),
"Y=a; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
"Y=a; path=/; expires=Mon, 18-Apr-62 22:50:14 GMT",
Time::Now() + base::Days(10), &initial_cookies);

// Inject our initial cookies into the mock PersistentCookieStore.
Expand Down Expand Up @@ -3177,7 +3176,7 @@ TEST_F(CookieMonsterTest, PersisentCookieStorageTest) {

// Add a cookie.
EXPECT_TRUE(SetCookie(cm.get(), http_www_foo_.url(),
"A=B; expires=Mon, 18-Apr-22 22:50:13 GMT"));
"A=B; expires=Mon, 18-Apr-62 22:50:13 GMT"));
this->MatchCookieLines("A=B", GetCookies(cm.get(), http_www_foo_.url()));
ASSERT_EQ(1u, store->commands().size());
EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[0].type);
Expand All @@ -3190,13 +3189,13 @@ TEST_F(CookieMonsterTest, PersisentCookieStorageTest) {

// Add a cookie.
EXPECT_TRUE(SetCookie(cm.get(), http_www_foo_.url(),
"A=B; expires=Mon, 18-Apr-22 22:50:13 GMT"));
"A=B; expires=Mon, 18-Apr-62 22:50:13 GMT"));
this->MatchCookieLines("A=B", GetCookies(cm.get(), http_www_foo_.url()));
ASSERT_EQ(3u, store->commands().size());
EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[2].type);
// Overwrite it.
EXPECT_TRUE(SetCookie(cm.get(), http_www_foo_.url(),
"A=Foo; expires=Mon, 18-Apr-22 22:50:14 GMT"));
"A=Foo; expires=Mon, 18-Apr-62 22:50:14 GMT"));
this->MatchCookieLines("A=Foo", GetCookies(cm.get(), http_www_foo_.url()));
ASSERT_EQ(5u, store->commands().size());
EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[3].type);
Expand Down
34 changes: 17 additions & 17 deletions net/cookies/cookie_store_unittest.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,14 +424,14 @@ class CookieStoreTest : public testing::Test {
base::Milliseconds(
CookieStoreTestTraits::creation_time_granularity_in_ms);

while (!matched && base::Time::Now() <= polling_end_date) {
while (!matched && base::Time::Now() <= polling_end_date) {
base::PlatformThread::Sleep(base::Milliseconds(10));
cookies = GetCookies(cs, url);
matched = (TokenizeCookieLine(line) == TokenizeCookieLine(cookies));
}

EXPECT_TRUE(matched) << "\"" << cookies
<< "\" does not match \"" << line << "\"";
EXPECT_TRUE(matched) << "\"" << cookies << "\" does not match \"" << line
<< "\"";
}

const CookieURLHelper http_www_foo_;
Expand Down Expand Up @@ -1393,7 +1393,7 @@ TYPED_TEST_P(CookieStoreTest, TestCookieDeletion) {
// Create a persistent cookie.
EXPECT_TRUE(this->SetCookie(
cs, this->http_www_foo_.url(),
std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-22 22:50:13 GMT"));
std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-62 22:50:13 GMT"));

this->MatchCookieLines("A=B",
this->GetCookies(cs, this->http_www_foo_.url()));
Expand All @@ -1406,7 +1406,7 @@ TYPED_TEST_P(CookieStoreTest, TestCookieDeletion) {
// Create a persistent cookie.
EXPECT_TRUE(this->SetCookie(
cs, this->http_www_foo_.url(),
std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-22 22:50:13 GMT"));
std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-62 22:50:13 GMT"));
this->MatchCookieLines("A=B",
this->GetCookies(cs, this->http_www_foo_.url()));
// Delete it via Expires.
Expand All @@ -1419,13 +1419,13 @@ TYPED_TEST_P(CookieStoreTest, TestCookieDeletion) {
// Create a persistent cookie.
EXPECT_TRUE(this->SetCookie(
cs, this->http_www_foo_.url(),
std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-22 22:50:13 GMT"));
std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-62 22:50:13 GMT"));
this->MatchCookieLines("A=B",
this->GetCookies(cs, this->http_www_foo_.url()));
// Check that it is not deleted with significant enough clock skew.
base::Time server_time;
EXPECT_TRUE(base::Time::FromString("Sun, 17-Apr-1977 22:50:13 GMT",
&server_time));
EXPECT_TRUE(
base::Time::FromString("Sun, 17-Apr-1977 22:50:13 GMT", &server_time));
EXPECT_TRUE(this->SetCookieWithServerTime(
cs, this->http_www_foo_.url(),
std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-1977 22:50:13 GMT",
Expand All @@ -1436,7 +1436,7 @@ TYPED_TEST_P(CookieStoreTest, TestCookieDeletion) {
// Create a persistent cookie.
EXPECT_TRUE(this->SetCookie(
cs, this->http_www_foo_.url(),
std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-22 22:50:13 GMT"));
std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-62 22:50:13 GMT"));
this->MatchCookieLines("A=B",
this->GetCookies(cs, this->http_www_foo_.url()));
// Delete it via Expires, with a unix epoch of 0.
Expand All @@ -1456,7 +1456,7 @@ TYPED_TEST_P(CookieStoreTest, TestDeleteAll) {

// Set a persistent cookie.
EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(),
"C=D; expires=Mon, 18-Apr-22 22:50:13 GMT"));
"C=D; expires=Mon, 18-Apr-62 22:50:13 GMT"));

EXPECT_EQ(2u, this->GetAllCookies(cs).size());

Expand Down Expand Up @@ -1602,12 +1602,12 @@ TYPED_TEST_P(CookieStoreTest, OverwritePersistentCookie) {
// Insert a cookie "a" for path "/path1"
EXPECT_TRUE(this->SetCookie(cs, url_foo,
"a=val1; path=/path1; "
"expires=Mon, 18-Apr-22 22:50:13 GMT"));
"expires=Mon, 18-Apr-62 22:50:13 GMT"));

// Insert a cookie "b" for path "/path1"
EXPECT_TRUE(this->SetCookie(cs, url_foo,
"b=val1; path=/path1; "
"expires=Mon, 18-Apr-22 22:50:14 GMT"));
"expires=Mon, 18-Apr-62 22:50:14 GMT"));

// Insert a cookie "b" for path "/path1", that is httponly. This should
// overwrite the non-http-only version.
Expand All @@ -1617,26 +1617,26 @@ TYPED_TEST_P(CookieStoreTest, OverwritePersistentCookie) {
net::CookieOptions::SameSiteCookieContext::MakeInclusive());
EXPECT_TRUE(this->CreateAndSetCookie(cs, url_foo,
"b=val2; path=/path1; httponly; "
"expires=Mon, 18-Apr-22 22:50:14 GMT",
"expires=Mon, 18-Apr-62 22:50:14 GMT",
allow_httponly));

// Insert a cookie "a" for path "/path1". This should overwrite.
EXPECT_TRUE(this->SetCookie(cs, url_foo,
"a=val33; path=/path1; "
"expires=Mon, 18-Apr-22 22:50:14 GMT"));
"expires=Mon, 18-Apr-62 22:50:14 GMT"));

// Insert a cookie "a" for path "/path2". This should NOT overwrite
// cookie "a", since the path is different.
EXPECT_TRUE(this->SetCookie(cs, url_foo,
"a=val9; path=/path2; "
"expires=Mon, 18-Apr-22 22:50:14 GMT"));
"expires=Mon, 18-Apr-62 22:50:14 GMT"));

// Insert a cookie "a" for path "/path1", but this time for "chromium.org".
// Although the name and path match, the hostnames do not, so shouldn't
// overwrite.
EXPECT_TRUE(this->SetCookie(cs, url_chromium,
"a=val99; path=/path1; "
"expires=Mon, 18-Apr-22 22:50:14 GMT"));
"expires=Mon, 18-Apr-62 22:50:14 GMT"));

if (TypeParam::supports_http_only) {
this->MatchCookieLines(
Expand Down Expand Up @@ -1871,7 +1871,7 @@ TYPED_TEST_P(CookieStoreTest, DeleteSessionCookie) {
EXPECT_TRUE(this->SetCookie(
cs, this->http_www_foo_.url(),
this->http_www_foo_.Format("C=D; path=/; domain=%D;"
"expires=Mon, 18-Apr-22 22:50:13 GMT")));
"expires=Mon, 18-Apr-62 22:50:13 GMT")));
this->MatchCookieLines("A=B; C=D",
this->GetCookies(cs, this->http_www_foo_.url()));
// Delete the session cookie.
Expand Down

0 comments on commit 55133ed

Please sign in to comment.