Commit 7106f2d5 authored by Christian Dullweber's avatar Christian Dullweber Committed by Commit Bot

Fix deleting cookies from Site Settings

"Clear & reset" used GURL::DomainIs() to check if a cookie belongs
to a specific domain. This doesn't handle cookies that are scoped
to a domain and all subdomains correctly.
E.g. a cookie for ".example.com" would not be removed if "example.com"
is cleared.
This is fixed by using CanonicalCookie::IsDomainMatch() instead,
which correctly determines if a cookie is visible to a domain.

Bug: 908364
Change-Id: If01fde74845c6c8a547ec86b0a3be8b9076b2d61
Reviewed-on: https://chromium-review.googlesource.com/c/1350634Reviewed-by: default avatarPeter Conn <peconn@chromium.org>
Reviewed-by: default avatarFinnur Thorarinsson <finnur@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611278}
parent 2b0e8e60
...@@ -50,7 +50,10 @@ import java.util.concurrent.Callable; ...@@ -50,7 +50,10 @@ import java.util.concurrent.Callable;
*/ */
@RunWith(ChromeJUnit4ClassRunner.class) @RunWith(ChromeJUnit4ClassRunner.class)
@RetryOnFailure @RetryOnFailure
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE}) @CommandLineFlags.Add({
ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
ContentSwitches.HOST_RESOLVER_RULES + "=MAP * 127.0.0.1",
})
public class SiteSettingsPreferencesTest { public class SiteSettingsPreferencesTest {
@Rule @Rule
public ChromeActivityTestRule<ChromeActivity> mActivityTestRule = public ChromeActivityTestRule<ChromeActivity> mActivityTestRule =
...@@ -387,7 +390,38 @@ public class SiteSettingsPreferencesTest { ...@@ -387,7 +390,38 @@ public class SiteSettingsPreferencesTest {
Assert.assertEquals( Assert.assertEquals(
"\"Foo=Bar\"", mActivityTestRule.runJavaScriptCodeInCurrentTab("getCookie()")); "\"Foo=Bar\"", mActivityTestRule.runJavaScriptCodeInCurrentTab("getCookie()"));
WebsiteAddress address = WebsiteAddress.create(url); resetSite(WebsiteAddress.create(url));
// Load the page again and ensure the cookie is gone.
mActivityTestRule.loadUrl(url);
Assert.assertEquals("\"\"", mActivityTestRule.runJavaScriptCodeInCurrentTab("getCookie()"));
}
/**
* Set cookies for domains and check that they are removed when a site is cleared.
*/
@Test
@SmallTest
@Feature({"Preferences"})
public void testClearDomainCookies() throws Exception {
final String url = mTestServer.getURLWithHostName(
"test.example.com", "/chrome/test/data/android/cookie.html");
mActivityTestRule.loadUrl(url);
Assert.assertEquals("\"\"", mActivityTestRule.runJavaScriptCodeInCurrentTab("getCookie()"));
mActivityTestRule.runJavaScriptCodeInCurrentTab("setCookie(\".example.com\")");
mActivityTestRule.runJavaScriptCodeInCurrentTab("setCookie(\".test.example.com\")");
Assert.assertEquals("\"Foo=Bar; Foo=Bar\"",
mActivityTestRule.runJavaScriptCodeInCurrentTab("getCookie()"));
resetSite(WebsiteAddress.create("test.example.com"));
// Load the page again and ensure the cookie is gone.
mActivityTestRule.loadUrl(url);
Assert.assertEquals("\"\"", mActivityTestRule.runJavaScriptCodeInCurrentTab("getCookie()"));
}
private void resetSite(WebsiteAddress address) {
Website website = new Website(address, address); Website website = new Website(address, address);
final Preferences preferenceActivity = startSingleWebsitePreferences(website); final Preferences preferenceActivity = startSingleWebsitePreferences(website);
ThreadUtils.runOnUiThreadBlocking(() -> { ThreadUtils.runOnUiThreadBlocking(() -> {
...@@ -396,10 +430,6 @@ public class SiteSettingsPreferencesTest { ...@@ -396,10 +430,6 @@ public class SiteSettingsPreferencesTest {
websitePreferences.resetSite(); websitePreferences.resetSite();
}); });
preferenceActivity.finish(); preferenceActivity.finish();
// Load the page again and ensure the cookie is gone.
mActivityTestRule.loadUrl(url);
Assert.assertEquals("\"\"", mActivityTestRule.runJavaScriptCodeInCurrentTab("getCookie()"));
} }
/** /**
...@@ -510,18 +540,7 @@ public class SiteSettingsPreferencesTest { ...@@ -510,18 +540,7 @@ public class SiteSettingsPreferencesTest {
@Feature({"Preferences"}) @Feature({"Preferences"})
public void testResetDoesntCrash() throws Exception { public void testResetDoesntCrash() throws Exception {
WebsiteAddress address = WebsiteAddress.create("example.com"); WebsiteAddress address = WebsiteAddress.create("example.com");
Website website = new Website(address, address); resetSite(address);
final Preferences preferenceActivity = startSingleWebsitePreferences(website);
ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override
public void run() {
SingleWebsitePreferences websitePreferences =
(SingleWebsitePreferences) preferenceActivity.getFragmentForTest();
websitePreferences.resetSite();
}
});
preferenceActivity.finish();
} }
/** /**
......
...@@ -630,7 +630,7 @@ void OnCookiesReceived(network::mojom::CookieManager* cookie_manager, ...@@ -630,7 +630,7 @@ void OnCookiesReceived(network::mojom::CookieManager* cookie_manager,
const GURL& domain, const GURL& domain,
const std::vector<net::CanonicalCookie>& cookies) { const std::vector<net::CanonicalCookie>& cookies) {
for (const auto& cookie : cookies) { for (const auto& cookie : cookies) {
if (domain.DomainIs(cookie.Domain())) { if (cookie.IsDomainMatch(domain.host())) {
cookie_manager->DeleteCanonicalCookie(cookie, base::DoNothing()); cookie_manager->DeleteCanonicalCookie(cookie, base::DoNothing());
} }
} }
......
...@@ -5,7 +5,10 @@ ...@@ -5,7 +5,10 @@
document.cookie = ''; document.cookie = '';
} }
function setCookie() { function setCookie(domain) {
if (domain)
document.cookie = 'Foo=Bar; Domain=' + domain;
else
document.cookie = 'Foo=Bar'; document.cookie = 'Foo=Bar';
} }
......
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