bcarrella
/
cef
mirror of https://bitbucket.org/chromiumembedded/cef
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Don't save or load cookies for non-cookieable scheme requests. 

This fixes an IsCanonical() DCHECK failure triggered by calling
CanonicalCookie::Create for a non-cookieable URL.

This change also adds unit test coverage for cross-origin cookie
behavior with sub-resource requests (iframe, XHR, Fetch).
This commit is contained in:
Marshall Greenblatt 2020-09-04 15:08:55 -04:00
parent 19391d8ab0
commit 4791109a28
10 changed files with 288 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
libcef/browser/alloy/alloy_browser_context.h
View File

@ -103,7 +103,7 @@ class AlloyBrowserContext : public ChromeProfileAlloy,
return !!settings_.persist_session_cookies;
}
base::Optional<std::vector<std::string>> GetCookieableSchemes() override {
return cookieable_schemes_;
return cookieable_schemes();
}
// visitedlink::VisitedLinkDelegate methods.

8
libcef/browser/browser_context.h
View File

@ -196,10 +196,11 @@ class CefBrowserContext {
CefMediaRouterManager* GetMediaRouterManager();
void set_cookieable_schemes(
base::Optional<std::vector<std::string>> schemes) {
using CookieableSchemes = base::Optional<std::vector<std::string>>;
void set_cookieable_schemes(const CookieableSchemes& schemes) {
cookieable_schemes_ = schemes;
}
CookieableSchemes cookieable_schemes() const { return cookieable_schemes_; }
// These accessors are safe to call from any thread because the values don't
// change during this object's lifespan.
@ -227,10 +228,9 @@ class CefBrowserContext {
const CefRequestContextSettings settings_;
base::FilePath cache_path_;
base::Optional<std::vector<std::string>> cookieable_schemes_;
private:
std::unique_ptr<CefIOThreadState> iothread_state_;
CookieableSchemes cookieable_schemes_;
std::unique_ptr<CefMediaRouterManager> media_router_manager_;
// CefRequestContextImpl objects referencing this object.

24
libcef/browser/net_service/cookie_helper.cc
View File

@ -17,6 +17,7 @@
#include "services/network/public/cpp/resource_request.h"
namespace net_service {
namespace cookie_helper {
namespace {
@ -137,6 +138,28 @@ void SaveCookiesOnUIThread(content::BrowserContext* browser_context,
} // namespace
bool IsCookieableScheme(
const GURL& url,
const base::Optional<std::vector<std::string>>& cookieable_schemes) {
if (!url.has_scheme())
return false;
if (cookieable_schemes) {
// The client has explicitly registered the full set of schemes that should
// be supported.
const auto url_scheme = url.scheme_piece();
for (auto scheme : *cookieable_schemes) {
if (url_scheme == scheme)
return true;
}
return false;
}
// Schemes that support cookies by default.
// This should match CookieMonster::kDefaultCookieableSchemes.
return url.SchemeIsHTTPOrHTTPS() || url.SchemeIsWSOrWSS();
}
void LoadCookies(content::BrowserContext* browser_context,
const network::ResourceRequest& request,
const AllowCookieCallback& allow_cookie_callback,
@ -228,4 +251,5 @@ void SaveCookies(content::BrowserContext* browser_context,
}
}
} // namespace cookie_helper
} // namespace net_service

9
libcef/browser/net_service/cookie_helper.h
View File

@ -21,6 +21,14 @@ struct ResourceRequest;
} // namespace network
namespace net_service {
namespace cookie_helper {
// Returns true if the scheme for |url| supports cookies. |cookieable_schemes|
// is the optional list of schemes that the client has explicitly registered as
// cookieable, which may intentionally exclude standard schemes.
bool IsCookieableScheme(
const GURL& url,
const base::Optional<std::vector<std::string>>& cookieable_schemes);
using AllowCookieCallback =
base::Callback<void(const net::CanonicalCookie&, bool* /* allow */)>;
@ -52,6 +60,7 @@ void SaveCookies(content::BrowserContext* browser_context,
const AllowCookieCallback& allow_cookie_callback,
DoneCookieCallback done_callback);
} // namespace cookie_helper
} // namespace net_service
#endif // CEF_LIBCEF_BROWSER_NET_SERVICE_COOKIE_HELPER_H_

28
libcef/browser/net_service/resource_request_handler_wrapper.cc
View File

@ -264,6 +264,7 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler {
CefBrowserContext::FromBrowserContext(browser_context);
iothread_state_ = cef_browser_context->iothread_state();
DCHECK(iothread_state_);
cookieable_schemes_ = cef_browser_context->cookieable_schemes();
// We register to be notified of CEF context or browser destruction so
// that we can stop accepting new requests and cancel pending/in-progress
@ -312,6 +313,7 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler {
CefRefPtr<CefBrowserHostImpl> browser_;
CefRefPtr<CefFrame> frame_;
CefIOThreadState* iothread_state_ = nullptr;
CefBrowserContext::CookieableSchemes cookieable_schemes_;
int render_process_id_ = 0;
int render_frame_id_ = -1;
int frame_tree_node_id_ = -1;
@ -487,6 +489,13 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler {
base::OnceClosure callback) {
CEF_REQUIRE_IOT();
if (!cookie_helper::IsCookieableScheme(request->url,
init_state_->cookieable_schemes_)) {
// The scheme does not support cookies.
std::move(callback).Run();
return;
}
// We need to load/save cookies ourselves for custom-handled requests, or
// if we're using a cookie filter.
auto allow_cookie_callback =
@ -499,9 +508,9 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler {
auto done_cookie_callback = base::BindOnce(
&InterceptedRequestHandlerWrapper::ContinueWithLoadedCookies,
weak_ptr_factory_.GetWeakPtr(), id, request, std::move(callback));
net_service::LoadCookies(init_state_->browser_context_, *request,
allow_cookie_callback,
std::move(done_cookie_callback));
cookie_helper::LoadCookies(init_state_->browser_context_, *request,
allow_cookie_callback,
std::move(done_cookie_callback));
}
static void AllowCookieAlways(const net::CanonicalCookie& cookie,
@ -850,6 +859,13 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler {
return;
}
if (!cookie_helper::IsCookieableScheme(request->url,
init_state_->cookieable_schemes_)) {
// The scheme does not support cookies.
std::move(callback).Run();
return;
}
// We need to load/save cookies ourselves for custom-handled requests, or
// if we're using a cookie filter.
auto allow_cookie_callback =
@ -862,9 +878,9 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler {
auto done_cookie_callback = base::BindOnce(
&InterceptedRequestHandlerWrapper::ContinueWithSavedCookies,
weak_ptr_factory_.GetWeakPtr(), id, std::move(callback));
net_service::SaveCookies(init_state_->browser_context_, *request, headers,
allow_cookie_callback,
std::move(done_cookie_callback));
cookie_helper::SaveCookies(init_state_->browser_context_, *request, headers,
allow_cookie_callback,
std::move(done_cookie_callback));
}
void AllowCookieSave(const RequestId& id,

10
tests/ceftests/cookie_unittest.cc
View File

@ -1569,20 +1569,24 @@ class CookieAccessTestHandler : public RoutingTestHandler,
EXPECT_EQ(0, can_save_cookie1_ct_);
EXPECT_EQ(0, can_send_cookie2_ct_);
} else {
// Get 1 call to CanSaveCookie for the 1st network request due to the
// network cookie.
EXPECT_EQ(1, can_save_cookie1_ct_);
if (test_mode_ == BLOCK_ALL_COOKIES) {
// Never send any cookies.
EXPECT_EQ(0, can_send_cookie2_ct_);
EXPECT_EQ(0, can_save_cookie1_ct_);
} else if (test_mode_ & BLOCK_WRITE) {
// Get 1 calls to CanSendCookie for the 2nd network request due to the
// JS cookie (network cookie is blocked).
EXPECT_EQ(1, can_send_cookie2_ct_);
// Get 1 call to CanSaveCookie for the 1st network request due to the
// network cookie.
EXPECT_EQ(1, can_save_cookie1_ct_);
} else {
// Get 2 calls to CanSendCookie for the 2nd network request due to the
// network cookie + JS cookie.
EXPECT_EQ(2, can_send_cookie2_ct_);
// Get 1 call to CanSaveCookie for the 1st network request due to the
// network cookie.
EXPECT_EQ(1, can_save_cookie1_ct_);
}
}

177
tests/ceftests/cors_unittest.cc
View File

@ -22,6 +22,7 @@ const char kMimeTypeText[] = "text/plain";
const char kDefaultHtml[] = "<html><body>TEST</body></html>";
const char kDefaultText[] = "TEST";
const char kDefaultCookie[] = "testCookie=testVal";
const char kSuccessMsg[] = "CorsTestHandler.Success";
const char kFailureMsg[] = "CorsTestHandler.Failure";
@ -144,6 +145,9 @@ struct TestSetup {
// Used for testing received console messages.
std::vector<std::string> console_messages;
// If true cookies will be cleared after every test run.
bool clear_cookies = false;
void AddResource(Resource* resource) {
DCHECK(resource);
resource->Validate();
@ -206,6 +210,12 @@ struct TestSetup {
(*it)->AssertDone();
}
}
// Optionally override to verify cleared cookie contents.
virtual bool VerifyClearedCookies(
const test_request::CookieVector& cookies) const {
return true;
}
};
class TestServerObserver : public test_server::ObserverHelper {
@ -284,6 +294,18 @@ class CorsTestHandler : public RoutingTestHandler {
void DestroyTest() override {
EXPECT_TRUE(shutting_down_);
if (setup_->NeedsServer()) {
EXPECT_TRUE(got_stopped_server_);
} else {
EXPECT_FALSE(got_stopped_server_);
}
if (setup_->clear_cookies) {
EXPECT_TRUE(got_cleared_cookies_);
} else {
EXPECT_FALSE(got_cleared_cookies_);
}
setup_->AssertDone();
EXPECT_TRUE(setup_->console_messages.empty())
<< "Did not receive expected console message: "
@ -292,17 +314,6 @@ class CorsTestHandler : public RoutingTestHandler {
RoutingTestHandler::DestroyTest();
}
void DestroyTestIfDone() {
CEF_REQUIRE_UI_THREAD();
if (shutting_down_)
return;
if (setup_->IsDone()) {
shutting_down_ = true;
StopServer();
}
}
CefRefPtr<CefResourceHandler> GetResourceHandler(
CefRefPtr<CefBrowser> browser,
CefRefPtr<CefFrame> frame,
@ -412,6 +423,17 @@ class CorsTestHandler : public RoutingTestHandler {
CefPostTask(TID_UI, base::Bind(&CorsTestHandler::DestroyTestIfDone, this));
}
void DestroyTestIfDone() {
CEF_REQUIRE_UI_THREAD();
if (shutting_down_)
return;
if (setup_->IsDone()) {
shutting_down_ = true;
StopServer();
}
}
void StartServer(const base::Closure& next_step) {
if (!CefCurrentlyOn(TID_UI)) {
CefPostTask(TID_UI,
@ -433,7 +455,7 @@ class CorsTestHandler : public RoutingTestHandler {
CEF_REQUIRE_UI_THREAD();
if (!server_) {
DCHECK(!setup_->NeedsServer());
DestroyTest();
AfterStoppedServer();
return;
}
@ -443,7 +465,32 @@ class CorsTestHandler : public RoutingTestHandler {
void StoppedServer() {
CEF_REQUIRE_UI_THREAD();
got_stopped_server_.yes();
server_ = nullptr;
AfterStoppedServer();
}
void AfterStoppedServer() {
CEF_REQUIRE_UI_THREAD();
if (setup_->clear_cookies) {
ClearCookies();
} else {
DestroyTest();
}
}
void ClearCookies() {
CEF_REQUIRE_UI_THREAD();
DCHECK(setup_->clear_cookies);
test_request::GetAllCookies(
CefCookieManager::GetGlobalManager(nullptr), /*delete_cookies=*/true,
base::Bind(&CorsTestHandler::ClearedCookies, this));
}
void ClearedCookies(const test_request::CookieVector& cookies) {
CEF_REQUIRE_UI_THREAD();
got_cleared_cookies_.yes();
EXPECT_TRUE(setup_->VerifyClearedCookies(cookies));
DestroyTest();
}
@ -458,6 +505,9 @@ class CorsTestHandler : public RoutingTestHandler {
TestServerObserver* server_ = nullptr;
bool shutting_down_ = false;
TrackCallback got_stopped_server_;
TrackCallback got_cleared_cookies_;
IMPLEMENT_REFCOUNTING(CorsTestHandler);
DISALLOW_COPY_AND_ASSIGN(CorsTestHandler);
};
@ -558,6 +608,68 @@ TEST(CorsTest, BasicCustomStandardSchemeWithQuery) {
namespace {
struct CookieTestSetup : TestSetup {
CookieTestSetup() {}
bool expect_cookie = false;
bool VerifyClearedCookies(
const test_request::CookieVector& cookies) const override {
if (!expect_cookie) {
EXPECT_TRUE(cookies.empty());
return cookies.empty();
}
EXPECT_EQ(1U, cookies.size());
const std::string& cookie = CefString(&cookies[0].name).ToString() + "=" +
CefString(&cookies[0].value).ToString();
EXPECT_STREQ(kDefaultCookie, cookie.c_str());
return cookie == kDefaultCookie;
}
};
struct CookieResource : Resource {
CookieResource() {}
bool expect_cookie = false;
void InitSetCookie() {
response->SetHeaderByName("Set-Cookie", kDefaultCookie,
/*override=*/true);
}
bool VerifyRequest(CefRefPtr<CefRequest> request) const override {
const std::string& cookie = request->GetHeaderByName("Cookie");
const std::string& expected_cookie =
expect_cookie ? kDefaultCookie : std::string();
EXPECT_STREQ(expected_cookie.c_str(), cookie.c_str()) << GetPathURL();
return expected_cookie == cookie;
}
};
void SetupCookieExpectations(CookieTestSetup* setup,
CookieResource* main_resource,
CookieResource* sub_resource) {
// All schemes except custom non-standard support cookies.
const bool supports_cookies =
main_resource->handler != HandlerType::CUSTOM_NONSTANDARD_SCHEME;
// The main resource may set the cookie (if cookies are supported), but should
// not receive one.
main_resource->InitSetCookie();
main_resource->expect_cookie = false;
// A cookie will be set only for schemes that support cookies.
setup->expect_cookie = supports_cookies;
// Always clear cookies so we can verify that one wasn't set unexpectedly.
setup->clear_cookies = true;
// Expect the sub-resource to receive the cookie for same-origin requests
// only.
sub_resource->expect_cookie =
supports_cookies && main_resource->handler == sub_resource->handler;
}
std::string GetIframeMainHtml(const std::string& iframe_url,
const std::string& sandbox_attribs) {
return "<html><body>TEST<iframe src=\"" + iframe_url + "\" sandbox=\"" +
@ -576,12 +688,12 @@ bool HasSandboxAttrib(const std::string& sandbox_attribs,
return sandbox_attribs.find(attrib) != std::string::npos;
}
void SetupIframeRequest(TestSetup* setup,
void SetupIframeRequest(CookieTestSetup* setup,
const std::string& test_name,
HandlerType main_handler,
Resource* main_resource,
CookieResource* main_resource,
HandlerType iframe_handler,
Resource* iframe_resource,
CookieResource* iframe_resource,
const std::string& sandbox_attribs) {
const std::string& base_path = "/" + test_name;
@ -594,6 +706,8 @@ void SetupIframeRequest(TestSetup* setup,
main_resource->Init(main_handler, base_path, kMimeTypeHtml,
GetIframeMainHtml(iframe_url, sandbox_attribs));
SetupCookieExpectations(setup, main_resource, iframe_resource);
if (HasSandboxAttrib(sandbox_attribs, "allow-scripts")) {
// Expect the iframe to load successfully and send the SuccessMsg.
iframe_resource->expected_success_query_ct = 1;
@ -636,8 +750,8 @@ void SetupIframeRequest(TestSetup* setup,
#define CORS_TEST_IFRAME(test_name, handler_main, handler_iframe, \
sandbox_attribs) \
TEST(CorsTest, Iframe##test_name) { \
TestSetup setup; \
Resource resource_main, resource_iframe; \
CookieTestSetup setup; \
CookieResource resource_main, resource_iframe; \
SetupIframeRequest(&setup, "CorsTest.Iframe" #test_name, \
HandlerType::handler_main, &resource_main, \
HandlerType::handler_iframe, &resource_iframe, \
@ -698,7 +812,7 @@ CORS_TEST_IFRAME_ALL(AllowScriptsAndSameOrigin,
namespace {
struct SubResource : Resource {
struct SubResource : CookieResource {
SubResource() {}
std::string main_origin;
@ -733,14 +847,15 @@ struct SubResource : Resource {
}
bool VerifyRequest(CefRefPtr<CefRequest> request) const override {
if (!CookieResource::VerifyRequest(request))
return false;
// Verify that the "Origin" header contains the expected value.
const std::string& origin = request->GetHeaderByName("Origin");
if (is_cross_origin) {
EXPECT_STREQ(main_origin.c_str(), origin.c_str());
return main_origin == origin;
}
EXPECT_TRUE(origin.empty());
return origin.empty();
const std::string& expected_origin =
is_cross_origin ? main_origin : std::string();
EXPECT_STREQ(expected_origin.c_str(), origin.c_str()) << GetPathURL();
return expected_origin == origin;
}
};
@ -809,10 +924,10 @@ std::string GetExecMainHtml(ExecMode mode, const std::string& sub_url) {
// XHR and fetch requests behave the same, except for console message contents.
void SetupExecRequest(ExecMode mode,
TestSetup* setup,
CookieTestSetup* setup,
const std::string& test_name,
HandlerType main_handler,
Resource* main_resource,
CookieResource* main_resource,
HandlerType sub_handler,
SubResource* sub_resource,
bool add_header) {
@ -828,6 +943,8 @@ void SetupExecRequest(ExecMode mode,
main_resource->Init(main_handler, base_path, kMimeTypeHtml,
GetExecMainHtml(mode, sub_url));
SetupCookieExpectations(setup, main_resource, sub_resource);
if (sub_resource->is_cross_origin &&
(!sub_resource->supports_cors || !add_header)) {
// Expect the cross-origin XHR to be blocked.
@ -877,8 +994,8 @@ void SetupExecRequest(ExecMode mode,
// Test XHR requests with different origin combinations.
#define CORS_TEST_XHR(test_name, handler_main, handler_sub, add_header) \
TEST(CorsTest, Xhr##test_name) { \
TestSetup setup; \
Resource resource_main; \
CookieTestSetup setup; \
CookieResource resource_main; \
SubResource resource_sub; \
SetupExecRequest(ExecMode::XHR, &setup, "CorsTest.Xhr" #test_name, \
HandlerType::handler_main, &resource_main, \
@ -930,8 +1047,8 @@ CORS_TEST_XHR_ALL(WithHeader, true)
// Test fetch requests with different origin combinations.
#define CORS_TEST_FETCH(test_name, handler_main, handler_sub, add_header) \
TEST(CorsTest, Fetch##test_name) { \
TestSetup setup; \
Resource resource_main; \
CookieTestSetup setup; \
CookieResource resource_main; \
SubResource resource_sub; \
SetupExecRequest(ExecMode::FETCH, &setup, "CorsTest.Fetch" #test_name, \
HandlerType::handler_main, &resource_main, \

2
tests/ceftests/scheme_handler_unittest.cc
View File

@ -2605,11 +2605,13 @@ void RegisterSchemeHandlerCustomSchemes(
// Add a custom standard scheme.
registrar->AddCustomScheme(
"customstd", CEF_SCHEME_OPTION_STANDARD | CEF_SCHEME_OPTION_CORS_ENABLED);
cookiable_schemes.push_back("customstd");
// Also used in cors_unittest.cc.
registrar->AddCustomScheme("customstdfetch",
CEF_SCHEME_OPTION_STANDARD |
CEF_SCHEME_OPTION_CORS_ENABLED |
CEF_SCHEME_OPTION_FETCH_ENABLED);
cookiable_schemes.push_back("customstdfetch");
// Add a custom non-standard scheme.
registrar->AddCustomScheme("customnonstd", CEF_SCHEME_OPTION_NONE);
registrar->AddCustomScheme("customnonstdfetch",

54
tests/ceftests/test_request.cc
View File

@ -94,6 +94,40 @@ class RequestClient : public CefURLRequestClient, public State {
const RequestDoneCallback done_callback_;
IMPLEMENT_REFCOUNTING(RequestClient);
DISALLOW_COPY_AND_ASSIGN(RequestClient);
};
// Implementation that collects all cookies, and optionally deletes them.
class CookieVisitor : public CefCookieVisitor {
public:
CookieVisitor(bool deleteCookies, const CookieDoneCallback& callback)
: delete_cookies_(deleteCookies), callback_(callback) {
DCHECK(!callback_.is_null());
}
~CookieVisitor() override {
CEF_REQUIRE_UI_THREAD();
callback_.Run(cookies_);
}
bool Visit(const CefCookie& cookie,
int count,
int total,
bool& deleteCookie) override {
CEF_REQUIRE_UI_THREAD();
cookies_.push_back(cookie);
if (delete_cookies_)
deleteCookie = true;
return true;
}
private:
CookieVector cookies_;
bool delete_cookies_;
CookieDoneCallback callback_;
IMPLEMENT_REFCOUNTING(CookieVisitor);
DISALLOW_COPY_AND_ASSIGN(CookieVisitor);
};
} // namespace
@ -145,4 +179,24 @@ CefRefPtr<CefResourceHandler> CreateResourceHandler(
headerMap, stream);
}
void GetAllCookies(CefRefPtr<CefCookieManager> manager,
bool deleteCookies,
const CookieDoneCallback& callback) {
bool result =
manager->VisitAllCookies(new CookieVisitor(deleteCookies, callback));
DCHECK(result);
}
// Retrieves URL cookies from |manager| and executes |callback| upon completion.
// If |deleteCookies| is true the cookies will also be deleted.
void GetUrlCookies(CefRefPtr<CefCookieManager> manager,
const CefString& url,
bool includeHttpOnly,
bool deleteCookies,
const CookieDoneCallback& callback) {
bool result = manager->VisitUrlCookies(
url, includeHttpOnly, new CookieVisitor(deleteCookies, callback));
DCHECK(result);
}
} // namespace test_request

18
tests/ceftests/test_request.h
View File

@ -9,6 +9,7 @@
#include <string>
#include "include/base/cef_bind.h"
#include "include/cef_cookie.h"
#include "include/cef_frame.h"
#include "include/cef_request.h"
#include "include/cef_request_context.h"
@ -73,6 +74,23 @@ CefRefPtr<CefResourceHandler> CreateResourceHandler(
CefRefPtr<CefResponse> response,
const std::string& response_data);
typedef std::vector<CefCookie> CookieVector;
typedef base::Callback<void(const CookieVector& cookies)> CookieDoneCallback;
// Retrieves all cookies from |manager| and executes |callback| upon completion.
// If |deleteCookies| is true the cookies will also be deleted.
void GetAllCookies(CefRefPtr<CefCookieManager> manager,
bool deleteCookies,
const CookieDoneCallback& callback);
// Retrieves URL cookies from |manager| and executes |callback| upon completion.
// If |deleteCookies| is true the cookies will also be deleted.
void GetUrlCookies(CefRefPtr<CefCookieManager> manager,
const CefString& url,
bool includeHttpOnly,
bool deleteCookies,
const CookieDoneCallback& callback);
} // namespace test_request
#endif // CEF_TESTS_CEFTESTS_TEST_REQUEST_H_