Commit e23356c2 authored by Randy Smith's avatar Randy Smith Committed by Commit Bot

Small cookie interface conversions away from net::CookieStore.

Specifically:
* Removed unused cookie_store.h include from chrome/browser/net
  and chrome/browser/safe_browsing.
* Switched content/public/test/browser_test_utils.cc over to Mojo accesses
  to cookies.
* The above required making the static CookieStore::BuildCookieLine
  available instead on CanonicalCookie (which is visible to Mojo clients
  when CookieStore is not).

Bug: 721395
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I8565fa795a48996774bd2d1aca21763dc31af3e8
Reviewed-on: https://chromium-review.googlesource.com/764944Reviewed-by: 's avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: 's avatarHelen Li <xunjieli@chromium.org>
Commit-Queue: Randy Smith <rdsmith@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516139}
parent a2365aae
......@@ -5,6 +5,7 @@
#include "chrome/browser/android/download/intercept_download_resource_throttle.h"
#include "base/strings/string_util.h"
#include "net/cookies/canonical_cookie.h"
#include "net/http/http_request_headers.h"
#include "net/http/http_response_headers.h"
#include "net/url_request/url_request.h"
......@@ -81,7 +82,7 @@ void InterceptDownloadResourceThrottle::CheckCookiePolicy(
DownloadInfo info(request_);
if (request_->context()->network_delegate()->CanGetCookies(*request_,
cookie_list)) {
std::string cookie = net::CookieStore::BuildCookieLine(cookie_list);
std::string cookie = net::CanonicalCookie::BuildCookieLine(cookie_list);
if (!cookie.empty())
info.cookie = cookie;
}
......
......@@ -17,7 +17,6 @@
#include "chrome/browser/profiles/profile_io_data.h"
#include "chrome/browser/profiles/storage_partition_descriptor.h"
#include "content/public/browser/browser_thread.h"
#include "net/cookies/cookie_store.h"
using content::BrowserThread;
......
......@@ -77,7 +77,6 @@
#include "content/public/common/content_features.h"
#include "content/public/test/browser_test_utils.h"
#include "crypto/sha2.h"
#include "net/cookies/cookie_store.h"
#include "net/cookies/cookie_util.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h"
......
......@@ -32,6 +32,7 @@
#include "gpu/GLES2/gl2extchromium.h"
#include "mojo/public/cpp/system/message_pipe.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "net/cookies/canonical_cookie.h"
#include "net/cookies/cookie_options.h"
#include "net/cookies/cookie_store.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
......@@ -393,7 +394,7 @@ void RenderFrameMessageFilter::CheckPolicyForCookies(
if (context && GetContentClient()->browser()->AllowGetCookie(
url, site_for_cookies, cookie_list, resource_context_,
render_process_id_, render_frame_id)) {
std::move(callback).Run(net::CookieStore::BuildCookieLine(cookie_list));
std::move(callback).Run(net::CanonicalCookie::BuildCookieLine(cookie_list));
} else {
std::move(callback).Run(std::string());
}
......
......@@ -67,13 +67,15 @@
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/network_service.mojom.h"
#include "content/public/test/test_fileapi_operation_waiter.h"
#include "content/public/test/test_navigation_observer.h"
#include "content/public/test/test_utils.h"
#include "content/test/accessibility_browser_test_utils.h"
#include "ipc/ipc_security_test_util.h"
#include "net/base/filename_util.h"
#include "net/cookies/cookie_store.h"
#include "net/base/io_buffer.h"
#include "net/cookies/canonical_cookie.h"
#include "net/filter/gzip_header.h"
#include "net/filter/gzip_source_stream.h"
#include "net/filter/mock_source_stream.h"
......@@ -81,8 +83,7 @@
#include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h"
#include "net/test/python_utils.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_getter.h"
#include "services/network/public/interfaces/cookie_manager.mojom.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/clipboard/clipboard.h"
......@@ -384,40 +385,15 @@ void SimulateKeyPressImpl(WebContents* web_contents,
}
void GetCookiesCallback(std::string* cookies_out,
base::WaitableEvent* event,
const std::string& cookies) {
*cookies_out = cookies;
event->Signal();
}
void GetCookiesOnIOThread(const GURL& url,
net::URLRequestContextGetter* context_getter,
base::WaitableEvent* event,
std::string* cookies) {
net::CookieStore* cookie_store =
context_getter->GetURLRequestContext()->cookie_store();
cookie_store->GetCookiesWithOptionsAsync(
url, net::CookieOptions(),
base::BindOnce(&GetCookiesCallback, cookies, event));
base::RunLoop* run_loop,
const std::vector<net::CanonicalCookie>& cookies) {
*cookies_out = net::CanonicalCookie::BuildCookieLine(cookies);
run_loop->Quit();
}
void SetCookieCallback(bool* result,
base::WaitableEvent* event,
bool success) {
void SetCookieCallback(bool* result, base::RunLoop* run_loop, bool success) {
*result = success;
event->Signal();
}
void SetCookieOnIOThread(const GURL& url,
const std::string& value,
net::URLRequestContextGetter* context_getter,
base::WaitableEvent* event,
bool* result) {
net::CookieStore* cookie_store =
context_getter->GetURLRequestContext()->cookie_store();
cookie_store->SetCookieWithOptionsAsync(
url, value, net::CookieOptions(),
base::BindOnce(&SetCookieCallback, result, event));
run_loop->Quit();
}
std::unique_ptr<net::test_server::HttpResponse>
......@@ -1229,17 +1205,15 @@ bool ExecuteWebUIResourceTest(WebContents* web_contents,
std::string GetCookies(BrowserContext* browser_context, const GURL& url) {
std::string cookies;
base::WaitableEvent event(base::WaitableEvent::ResetPolicy::MANUAL,
base::WaitableEvent::InitialState::NOT_SIGNALED);
net::URLRequestContextGetter* context_getter =
BrowserContext::GetDefaultStoragePartition(browser_context)->
GetURLRequestContext();
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
base::BindOnce(&GetCookiesOnIOThread, url,
base::RetainedRef(context_getter), &event, &cookies));
event.Wait();
base::RunLoop run_loop;
network::mojom::CookieManagerPtr cookie_manager;
BrowserContext::GetDefaultStoragePartition(browser_context)
->GetNetworkContext()
->GetCookieManager(mojo::MakeRequest(&cookie_manager));
cookie_manager->GetCookieList(
url, net::CookieOptions(),
base::BindOnce(&GetCookiesCallback, &cookies, &run_loop));
run_loop.Run();
return cookies;
}
......@@ -1247,17 +1221,19 @@ bool SetCookie(BrowserContext* browser_context,
const GURL& url,
const std::string& value) {
bool result = false;
base::WaitableEvent event(base::WaitableEvent::ResetPolicy::MANUAL,
base::WaitableEvent::InitialState::NOT_SIGNALED);
net::URLRequestContextGetter* context_getter =
BrowserContext::GetDefaultStoragePartition(browser_context)->
GetURLRequestContext();
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
base::BindOnce(&SetCookieOnIOThread, url, value,
base::RetainedRef(context_getter), &event, &result));
event.Wait();
base::RunLoop run_loop;
network::mojom::CookieManagerPtr cookie_manager;
BrowserContext::GetDefaultStoragePartition(browser_context)
->GetNetworkContext()
->GetCookieManager(mojo::MakeRequest(&cookie_manager));
std::unique_ptr<net::CanonicalCookie> cc(net::CanonicalCookie::Create(
url, value, base::Time::Now(), net::CookieOptions()));
DCHECK(cc.get());
cookie_manager->SetCanonicalCookie(
*cc.get(), true /* secure_source */, true /* modify_http_only */,
base::BindOnce(&SetCookieCallback, &result, &run_loop));
run_loop.Run();
return result;
}
......
......@@ -20,6 +20,7 @@
#include "net/base/net_errors.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "net/base/upload_bytes_element_reader.h"
#include "net/cookies/canonical_cookie.h"
#include "net/cookies/cookie_store.h"
#include "net/http/http_response_headers.h"
#include "net/http/http_util.h"
......@@ -117,7 +118,7 @@ void GenericURLRequestJob::OnCookiesAvailable(
DCHECK(origin_task_runner_->RunsTasksInCurrentSequence());
// TODO(alexclarke): Set user agent.
// Pass cookies, the referrer and any extra headers into the fetch request.
std::string cookie = net::CookieStore::BuildCookieLine(cookie_list);
std::string cookie = net::CanonicalCookie::BuildCookieLine(cookie_list);
if (!cookie.empty())
extra_request_headers_.SetHeader(net::HttpRequestHeaders::kCookie, cookie);
......
......@@ -447,6 +447,23 @@ bool CanonicalCookie::IsCanonical() const {
return true;
}
// static
std::string CanonicalCookie::BuildCookieLine(
const std::vector<CanonicalCookie>& cookies) {
std::string cookie_line;
for (const auto& cookie : cookies) {
if (!cookie_line.empty())
cookie_line += "; ";
// In Mozilla, if you set a cookie like "AAA", it will have an empty token
// and a value of "AAA". When it sends the cookie back, it will send "AAA",
// so we need to avoid sending "=AAA" for a blank token value.
if (!cookie.Name().empty())
cookie_line += cookie.Name() + "=";
cookie_line += cookie.Value();
}
return cookie_line;
}
// static
CanonicalCookie::CookiePrefix CanonicalCookie::GetCookiePrefix(
const std::string& name) {
......
......@@ -160,6 +160,10 @@ class NET_EXPORT CanonicalCookie {
// greater than the last access time.
bool IsCanonical() const;
// Returns the cookie line (e.g. "cookie1=value1; cookie2=value2") represented
// by |cookies|. The string is built in the same order as the given list.
static std::string BuildCookieLine(
const std::vector<CanonicalCookie>& cookies);
private:
FRIEND_TEST_ALL_PREFIXES(CanonicalCookieTest, TestPrefixHistograms);
......
......@@ -12,6 +12,20 @@
namespace net {
namespace {
// Helper for testing BuildCookieLine
void MatchCookieLineToVector(
const std::string& line,
const std::vector<std::unique_ptr<CanonicalCookie>>& cookies) {
std::vector<CanonicalCookie> list;
for (const auto& cookie : cookies)
list.push_back(*cookie);
EXPECT_EQ(line, CanonicalCookie::BuildCookieLine(list));
}
} // namespace
TEST(CanonicalCookieTest, Constructor) {
GURL url("http://www.example.com/test");
base::Time current_time = base::Time::Now();
......@@ -915,4 +929,29 @@ TEST(CanonicalCookieTest, TestPrefixHistograms) {
CanonicalCookie::COOKIE_PREFIX_SECURE, 1);
}
TEST(CanonicalCookieTest, BuildCookieLine) {
std::vector<std::unique_ptr<CanonicalCookie>> cookies;
GURL url("https://example.com/");
CookieOptions options;
base::Time now = base::Time::Now();
MatchCookieLineToVector("", cookies);
cookies.push_back(CanonicalCookie::Create(url, "A=B", now, options));
MatchCookieLineToVector("A=B", cookies);
// Nameless cookies are sent back without a prefixed '='.
cookies.push_back(CanonicalCookie::Create(url, "C", now, options));
MatchCookieLineToVector("A=B; C", cookies);
// Cookies separated by ';'.
cookies.push_back(CanonicalCookie::Create(url, "D=E", now, options));
MatchCookieLineToVector("A=B; C; D=E", cookies);
// BuildCookieLine doesn't reorder the list, it relies on the caller to do so.
cookies.push_back(CanonicalCookie::Create(
url, "F=G", now - base::TimeDelta::FromSeconds(1), options));
MatchCookieLineToVector("A=B; C; D=E; F=G", cookies);
// BuildCookieLine doesn't deduplicate.
cookies.push_back(CanonicalCookie::Create(
url, "D=E", now - base::TimeDelta::FromSeconds(2), options));
MatchCookieLineToVector("A=B; C; D=E; F=G; D=E", cookies);
}
} // namespace net
......@@ -16,22 +16,7 @@ bool CookieStore::ChangeCauseIsDeletion(CookieStore::ChangeCause cause) {
return cause != CookieStore::ChangeCause::INSERTED;
}
std::string CookieStore::BuildCookieLine(
const std::vector<CanonicalCookie>& cookies) {
std::string cookie_line;
for (const auto& cookie : cookies) {
if (!cookie_line.empty())
cookie_line += "; ";
// In Mozilla, if you set a cookie like "AAA", it will have an empty token
// and a value of "AAA". When it sends the cookie back, it will send "AAA",
// so we need to avoid sending "=AAA" for a blank token value.
if (!cookie.Name().empty())
cookie_line += cookie.Name() + "=";
cookie_line += cookie.Value();
}
return cookie_line;
}
// Keep in sync with CanonicalCookie::BuildCookieLine.
std::string CookieStore::BuildCookieLine(
const std::vector<CanonicalCookie*>& cookies) {
std::string cookie_line;
......
......@@ -90,10 +90,10 @@ class NET_EXPORT CookieStore {
// Returns the cookie line (e.g. "cookie1=value1; cookie2=value2") represented
// by |cookies|. The string is built in the same order as the given list.
//
// TODO(mkwst): We really should standardize on either
// 'std::vector<CanonicalCookie>' or 'std::vector<CanonicalCookie*>'.
static std::string BuildCookieLine(
const std::vector<CanonicalCookie>& cookies);
// Deprecated; use CanonicalCookie::BuildCookieLine(
// const std::vector<CanonicalCookie>& cookies) instead.
// TODO(http://crbug.com/588081#c3): Believed to only be used (directly
// and indirectly) by tests; should be removed.
static std::string BuildCookieLine(
const std::vector<CanonicalCookie*>& cookies);
......
......@@ -21,13 +21,6 @@ namespace {
void MatchCookieLineToVector(
const std::string& line,
const std::vector<std::unique_ptr<CanonicalCookie>>& cookies) {
// Test the std::vector<CanonicalCookie> variant
// ('CookieMonster::CookieList'):
std::vector<CanonicalCookie> list;
for (const auto& cookie : cookies)
list.push_back(*cookie);
EXPECT_EQ(line, CookieStore::BuildCookieLine(list));
// Test the std::vector<CanonicalCookie*> variant
// ('CookieMonster::CanonicalCookieVector' (yes, this is absurd)):
std::vector<CanonicalCookie*> ptr_list;
......@@ -38,6 +31,8 @@ void MatchCookieLineToVector(
} // namespace
// TODO(rdsmith): Keep in sync with CanonicalCookieTest.BuildCookieLine;
// remove when the need for CookieStore::BuildCookieLine is done.
TEST(CookieStoreBaseTest, BuildCookieLine) {
std::vector<std::unique_ptr<CanonicalCookie>> cookies;
GURL url("https://example.com/");
......
......@@ -119,7 +119,7 @@ class CookieStoreTest : public testing::Test {
url, options,
base::Bind(&GetCookieListCallback::Run, base::Unretained(&callback)));
callback.WaitUntilDone();
return CookieStore::BuildCookieLine(callback.cookies());
return CanonicalCookie::BuildCookieLine(callback.cookies());
}
CookieList GetCookieListWithOptions(CookieStore* cs,
......
......@@ -34,6 +34,7 @@
#include "net/base/trace_constants.h"
#include "net/base/url_util.h"
#include "net/cert/cert_status_flags.h"
#include "net/cookies/canonical_cookie.h"
#include "net/cookies/cookie_store.h"
#include "net/filter/brotli_source_stream.h"
#include "net/filter/filter_source_stream.h"
......@@ -614,7 +615,8 @@ void URLRequestHttpJob::SetCookieHeaderAndStart(const CookieList& cookie_list) {
LogCookieAgeForNonSecureRequest(cookie_list, *request_);
request_info_.extra_headers.SetHeader(
HttpRequestHeaders::kCookie, CookieStore::BuildCookieLine(cookie_list));
HttpRequestHeaders::kCookie,
CanonicalCookie::BuildCookieLine(cookie_list));
// Disable privacy mode as we are sending cookies anyway.
request_info_.privacy_mode = PRIVACY_MODE_DISABLED;
}
......
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