Skip to content

Commit

Permalink
Remove unused emergency settings for SameSite cookie rollout
Browse files Browse the repository at this point in the history
This removes the feature and param for an emergency setting that was
never used. This is now obsolete because SameSite-by-default cookies is
now enabled by default.

Bug: 1045122
Change-Id: I6528907276c986c8de7c69e3bad71c08b4f3d6d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2495352
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820792}
  • Loading branch information
chlily1 authored and Commit Bot committed Oct 26, 2020
1 parent ab3cd2c commit 5ce8156
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 89 deletions.
35 changes: 2 additions & 33 deletions services/network/cookie_settings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,55 +8,24 @@

#include "base/bind.h"
#include "base/callback.h"
#include "base/strings/string_split.h"
#include "components/content_settings/core/common/content_settings_utils.h"
#include "net/base/net_errors.h"
#include "net/cookies/cookie_util.h"
#include "net/cookies/static_cookie_policy.h"
#include "services/network/public/cpp/features.h"

namespace network {
namespace {

bool IsDefaultSetting(const ContentSettingPatternSource& setting) {
return setting.primary_pattern.MatchesAllHosts() &&
setting.secondary_pattern.MatchesAllHosts();
}

void AppendEmergencyLegacyCookieAccess(
ContentSettingsForOneType* settings_for_legacy_cookie_access) {
if (!base::FeatureList::IsEnabled(features::kEmergencyLegacyCookieAccess))
return;

std::vector<std::string> patterns =
SplitString(features::kEmergencyLegacyCookieAccessParam.Get(), ",",
base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);

for (const auto& pattern_str : patterns) {
// Only primary pattern and the setting actually looked at here.
settings_for_legacy_cookie_access->push_back(ContentSettingPatternSource(
ContentSettingsPattern::FromString(pattern_str),
ContentSettingsPattern::Wildcard(),
/* legacy, see CookieSettingsBase::GetCookieAccessSemanticsForDomain */
base::Value::FromUniquePtrValue(
content_settings::ContentSettingToValue(CONTENT_SETTING_ALLOW)),
std::string(), false));
}
}

} // namespace

CookieSettings::CookieSettings() {
AppendEmergencyLegacyCookieAccess(&settings_for_legacy_cookie_access_);
}
CookieSettings::CookieSettings() = default;

CookieSettings::~CookieSettings() = default;

void CookieSettings::set_content_settings_for_legacy_cookie_access(
const ContentSettingsForOneType& settings) {
settings_for_legacy_cookie_access_ = settings;
AppendEmergencyLegacyCookieAccess(&settings_for_legacy_cookie_access_);
}

DeleteCookiePredicate CookieSettings::CreateDeleteCookieOnExitPredicate()
const {
if (!HasSessionOnlyOrigins())
Expand Down
4 changes: 3 additions & 1 deletion services/network/cookie_settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CookieSettings
}

void set_content_settings_for_legacy_cookie_access(
const ContentSettingsForOneType& settings);
const ContentSettingsForOneType& settings) {
settings_for_legacy_cookie_access_ = settings;
}

void set_storage_access_grants(const ContentSettingsForOneType& settings) {
storage_access_grants_ = settings;
Expand Down
40 changes: 0 additions & 40 deletions services/network/cookie_settings_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -502,45 +502,5 @@ TEST_F(CookieSettingsTest,
}
}

TEST_F(CookieSettingsTest, CookieAccessSemanticsEmergencyOverride) {
base::test::ScopedFeatureList feature_list;

feature_list.InitWithFeaturesAndParameters(
{{net::features::kSameSiteByDefaultCookies, {}},
{features::kEmergencyLegacyCookieAccess,
{{features::kEmergencyLegacyCookieAccessParamName,
"example.org, [*.]example.gov"}}}},
{} /* disabled_features*/);
CookieSettings settings;
settings.set_content_settings_for_legacy_cookie_access(
{CreateSetting(kDomainWildcardPattern, "*", CONTENT_SETTING_ALLOW)});

const struct {
net::CookieAccessSemantics status;
std::string cookie_domain;
} kTestCases[] = {
// These three test cases are LEGACY because they match the setting.
{net::CookieAccessSemantics::LEGACY, kDomain},
{net::CookieAccessSemantics::LEGACY, kDotDomain},
// Subdomain also matches pattern.
{net::CookieAccessSemantics::LEGACY, kSubDomain},
// This test case defaults into NONLEGACY.
{net::CookieAccessSemantics::NONLEGACY, kOtherDomain},

// things that got pushed via experiment config.
{net::CookieAccessSemantics::LEGACY, "example.org"},
{net::CookieAccessSemantics::NONLEGACY, "sub.example.org"},
{net::CookieAccessSemantics::LEGACY, "example.gov"},
{net::CookieAccessSemantics::LEGACY, "sub.example.gov"},
{net::CookieAccessSemantics::NONLEGACY, "example.gov.uk"},
};

for (const auto& test : kTestCases) {
EXPECT_EQ(test.status,
settings.GetCookieAccessSemanticsForDomain(test.cookie_domain))
<< test.cookie_domain;
}
}

} // namespace
} // namespace network
8 changes: 0 additions & 8 deletions services/network/public/cpp/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,6 @@ const base::Feature
"DeriveOriginFromUrlForNeitherGetNorHeadRequestWhenHavingSpecialAccess",
base::FEATURE_DISABLED_BY_DEFAULT};

// Emergency switch for legacy cookie access semantics on given patterns, as
// specified by the param, comma separated.
const base::Feature kEmergencyLegacyCookieAccess{
"EmergencyLegacyCookieAccess", base::FEATURE_DISABLED_BY_DEFAULT};
const char kEmergencyLegacyCookieAccessParamName[] = "Patterns";
const base::FeatureParam<std::string> kEmergencyLegacyCookieAccessParam{
&kEmergencyLegacyCookieAccess, kEmergencyLegacyCookieAccessParamName, ""};

// Controls whether a |request_initiator| that mismatches
// |request_initiator_origin_lock| leads to 1) failing the HTTP request and 2)
// calling mojo::ReportBadMessage (on desktop platforms, where NetworkService
Expand Down
7 changes: 0 additions & 7 deletions services/network/public/cpp/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,6 @@ extern const base::Feature kDisableKeepaliveFetch;
COMPONENT_EXPORT(NETWORK_CPP)
extern const base::Feature
kDeriveOriginFromUrlForNeitherGetNorHeadRequestWhenHavingSpecialAccess;
COMPONENT_EXPORT(NETWORK_CPP)
extern const base::Feature kEmergencyLegacyCookieAccess;
COMPONENT_EXPORT(NETWORK_CPP)
extern const char kEmergencyLegacyCookieAccessParamName[];
COMPONENT_EXPORT(NETWORK_CPP)
extern const base::FeatureParam<std::string> kEmergencyLegacyCookieAccessParam;

COMPONENT_EXPORT(NETWORK_CPP)
extern const base::Feature kRequestInitiatorSiteLockEnfocement;
COMPONENT_EXPORT(NETWORK_CPP)
Expand Down

0 comments on commit 5ce8156

Please sign in to comment.