From 52fa0750eb73ff4df176b1db0fd206226eaa930f Mon Sep 17 00:00:00 2001 From: "jochen@chromium.org" Date: Tue, 7 Dec 2010 08:54:42 +0000 Subject: Also register read cookies in the content settings delegate. BUG=63663 TEST=unit tests Review URL: http://codereview.chromium.org/5318002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@68458 0039d316-1c4b-4281-b951-d872f2087c98 --- .../content_setting_image_model_unittest.cc | 2 +- .../renderer_host/render_view_host_delegate.h | 20 +++++-- .../renderer_host/resource_dispatcher_host.cc | 26 +++++++- .../renderer_host/resource_dispatcher_host.h | 2 + .../renderer_host/resource_message_filter.cc | 17 +++++- .../renderer_host/resource_message_filter.h | 4 +- .../tab_contents/tab_specific_content_settings.cc | 26 +++++++- .../tab_contents/tab_specific_content_settings.h | 11 ++-- .../tab_specific_content_settings_unittest.cc | 10 ++-- net/base/cookie_monster.cc | 14 +++-- net/base/cookie_monster.h | 9 ++- net/base/cookie_monster_unittest.cc | 11 ++++ net/url_request/url_request_http_job.cc | 69 +++++++++++----------- 13 files changed, 160 insertions(+), 61 deletions(-) diff --git a/chrome/browser/content_setting_image_model_unittest.cc b/chrome/browser/content_setting_image_model_unittest.cc index 77bcdee..fc7989d 100644 --- a/chrome/browser/content_setting_image_model_unittest.cc +++ b/chrome/browser/content_setting_image_model_unittest.cc @@ -59,7 +59,7 @@ TEST_F(ContentSettingImageModelTest, CookieAccessed) { EXPECT_EQ(std::string(), content_setting_image_model->get_tooltip()); net::CookieOptions options; - content_settings->OnCookieAccessed( + content_settings->OnCookieChanged( GURL("http://google.com"), "A=B", options, false); content_setting_image_model->UpdateFromTabContents(&tab_contents); EXPECT_TRUE(content_setting_image_model->is_visible()); diff --git a/chrome/browser/renderer_host/render_view_host_delegate.h b/chrome/browser/renderer_host/render_view_host_delegate.h index e7f68e0..02ca7ca 100644 --- a/chrome/browser/renderer_host/render_view_host_delegate.h +++ b/chrome/browser/renderer_host/render_view_host_delegate.h @@ -71,6 +71,7 @@ class Message; } namespace net { +class CookieList; class CookieOptions; } @@ -395,14 +396,23 @@ class RenderViewHostDelegate { virtual void OnContentBlocked(ContentSettingsType type, const std::string& resource_identifier) = 0; - // Called when a specific cookie in the current page was accessed. + // Called when cookies for the given URL were read either from within the + // current page or while loading it. |blocked_by_policy| should be true, if + // reading cookies was blocked due to the user's content settings. In that + // case, this function should invoke OnContentBlocked. + virtual void OnCookiesRead( + const GURL& url, + const net::CookieList& cookie_list, + bool blocked_by_policy) = 0; + + // Called when a specific cookie in the current page was changed. // |blocked_by_policy| should be true, if the cookie was blocked due to the // user's content settings. In that case, this function should invoke // OnContentBlocked. - virtual void OnCookieAccessed(const GURL& url, - const std::string& cookie_line, - const net::CookieOptions& options, - bool blocked_by_policy) = 0; + virtual void OnCookieChanged(const GURL& url, + const std::string& cookie_line, + const net::CookieOptions& options, + bool blocked_by_policy) = 0; // Called when a specific indexed db factory in the current page was // accessed. If access was blocked due to the user's content settings, diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.cc b/chrome/browser/renderer_host/resource_dispatcher_host.cc index 14493fa..9555ebc 100644 --- a/chrome/browser/renderer_host/resource_dispatcher_host.cc +++ b/chrome/browser/renderer_host/resource_dispatcher_host.cc @@ -1081,6 +1081,30 @@ void ResourceDispatcherHost::OnSSLCertificateError( SSLManager::OnSSLCertificateError(this, request, cert_error, cert); } +void ResourceDispatcherHost::OnGetCookies( + net::URLRequest* request, + bool blocked_by_policy) { + VLOG(1) << "OnGetCookies: " << request->url().spec(); + + int render_process_id, render_view_id; + if (!RenderViewForRequest(request, &render_process_id, &render_view_id)) + return; + + ChromeURLRequestContext* context = + static_cast(request->context()); + if (context->IsExternal()) + return; + + net::CookieMonster* cookie_monster = + context->cookie_store()->GetCookieMonster(); + net::CookieList cookie_list = + cookie_monster->GetAllCookiesForURL(request->url()); + CallRenderViewHostContentSettingsDelegate( + render_process_id, render_view_id, + &RenderViewHostDelegate::ContentSettings::OnCookiesRead, + request->url(), cookie_list, blocked_by_policy); +} + void ResourceDispatcherHost::OnSetCookie(net::URLRequest* request, const std::string& cookie_line, const net::CookieOptions& options, @@ -1093,7 +1117,7 @@ void ResourceDispatcherHost::OnSetCookie(net::URLRequest* request, CallRenderViewHostContentSettingsDelegate( render_process_id, render_view_id, - &RenderViewHostDelegate::ContentSettings::OnCookieAccessed, + &RenderViewHostDelegate::ContentSettings::OnCookieChanged, request->url(), cookie_line, options, blocked_by_policy); } diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.h b/chrome/browser/renderer_host/resource_dispatcher_host.h index 840d2db..448fbde 100644 --- a/chrome/browser/renderer_host/resource_dispatcher_host.h +++ b/chrome/browser/renderer_host/resource_dispatcher_host.h @@ -214,6 +214,8 @@ class ResourceDispatcherHost : public net::URLRequest::Delegate { virtual void OnSSLCertificateError(net::URLRequest* request, int cert_error, net::X509Certificate* cert); + virtual void OnGetCookies(net::URLRequest* request, + bool blocked_by_policy); virtual void OnSetCookie(net::URLRequest* request, const std::string& cookie_line, const net::CookieOptions& options, diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc index f82e398..c056f41 100644 --- a/chrome/browser/renderer_host/resource_message_filter.cc +++ b/chrome/browser/renderer_host/resource_message_filter.cc @@ -669,7 +669,7 @@ void ResourceMessageFilter::OnSetCookie(const IPC::Message& message, void ResourceMessageFilter::OnGetCookies(const GURL& url, const GURL& first_party_for_cookies, IPC::Message* reply_msg) { - URLRequestContext* context = GetRequestContextForURL(url); + ChromeURLRequestContext* context = GetRequestContextForURL(url); GetCookiesCompletion* callback = new GetCookiesCompletion(id(), reply_msg->routing_id(), url, reply_msg, @@ -1748,7 +1748,7 @@ void SetCookieCompletion::RunWithParams(const Tuple1& params) { if (!context_->IsExternal()) { CallRenderViewHostContentSettingsDelegate( render_process_id_, render_view_id_, - &RenderViewHostDelegate::ContentSettings::OnCookieAccessed, + &RenderViewHostDelegate::ContentSettings::OnCookieChanged, url_, cookie_line_, options, blocked_by_policy); } delete this; @@ -1759,7 +1759,7 @@ GetCookiesCompletion::GetCookiesCompletion(int render_process_id, const GURL& url, IPC::Message* reply_msg, ResourceMessageFilter* filter, - URLRequestContext* context, + ChromeURLRequestContext* context, bool raw_cookies) : url_(url), reply_msg_(reply_msg), @@ -1781,6 +1781,17 @@ void GetCookiesCompletion::RunWithParams(const Tuple1& params) { cookies = cookie_store()->GetCookies(url_); ViewHostMsg_GetCookies::WriteReplyParams(reply_msg_, cookies); filter_->Send(reply_msg_); + if (!context_->IsExternal()) { + net::CookieMonster* cookie_monster = + context_->cookie_store()->GetCookieMonster(); + net::CookieList cookie_list = + cookie_monster->GetAllCookiesForURLWithOptions( + url_, net::CookieOptions()); + CallRenderViewHostContentSettingsDelegate( + render_process_id_, render_view_id_, + &RenderViewHostDelegate::ContentSettings::OnCookiesRead, + url_, cookie_list, result != net::OK); + } delete this; } else { // Ignore the policy result. We only waited on the policy result so that diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h index 5aeaa7d..ce23137 100644 --- a/chrome/browser/renderer_host/resource_message_filter.h +++ b/chrome/browser/renderer_host/resource_message_filter.h @@ -538,7 +538,7 @@ class GetCookiesCompletion : public net::CompletionCallback { int render_view_id, const GURL& url, IPC::Message* reply_msg, ResourceMessageFilter* filter, - URLRequestContext* context, + ChromeURLRequestContext* context, bool raw_cookies); virtual ~GetCookiesCompletion(); @@ -562,7 +562,7 @@ class GetCookiesCompletion : public net::CompletionCallback { GURL url_; IPC::Message* reply_msg_; scoped_refptr filter_; - scoped_refptr context_; + scoped_refptr context_; int render_process_id_; int render_view_id_; bool raw_cookies_; diff --git a/chrome/browser/tab_contents/tab_specific_content_settings.cc b/chrome/browser/tab_contents/tab_specific_content_settings.cc index 22550ea..e5e6f42 100644 --- a/chrome/browser/tab_contents/tab_specific_content_settings.cc +++ b/chrome/browser/tab_contents/tab_specific_content_settings.cc @@ -103,7 +103,31 @@ void TabSpecificContentSettings::OnContentAccessed(ContentSettingsType type) { } } -void TabSpecificContentSettings::OnCookieAccessed( +void TabSpecificContentSettings::OnCookiesRead( + const GURL& url, + const net::CookieList& cookie_list, + bool blocked_by_policy) { + LocalSharedObjectsContainer& container = blocked_by_policy ? + blocked_local_shared_objects_ : allowed_local_shared_objects_; + typedef net::CookieList::const_iterator cookie_iterator; + for (cookie_iterator cookie = cookie_list.begin(); + cookie != cookie_list.end(); ++cookie) { + container.cookies()->SetCookieWithDetails(url, + cookie->Name(), + cookie->Value(), + cookie->Domain(), + cookie->Path(), + cookie->ExpiryDate(), + cookie->IsSecure(), + cookie->IsHttpOnly()); + } + if (blocked_by_policy) + OnContentBlocked(CONTENT_SETTINGS_TYPE_COOKIES, std::string()); + else + OnContentAccessed(CONTENT_SETTINGS_TYPE_COOKIES); +} + +void TabSpecificContentSettings::OnCookieChanged( const GURL& url, const std::string& cookie_line, const net::CookieOptions& options, diff --git a/chrome/browser/tab_contents/tab_specific_content_settings.h b/chrome/browser/tab_contents/tab_specific_content_settings.h index 26685d1..a046a86 100644 --- a/chrome/browser/tab_contents/tab_specific_content_settings.h +++ b/chrome/browser/tab_contents/tab_specific_content_settings.h @@ -96,10 +96,13 @@ class TabSpecificContentSettings // RenderViewHostDelegate::ContentSettings implementation. virtual void OnContentBlocked(ContentSettingsType type, const std::string& resource_identifier); - virtual void OnCookieAccessed(const GURL& url, - const std::string& cookie_line, - const net::CookieOptions& options, - bool blocked_by_policy); + virtual void OnCookiesRead(const GURL& url, + const net::CookieList& cookie_list, + bool blocked_by_policy); + virtual void OnCookieChanged(const GURL& url, + const std::string& cookie_line, + const net::CookieOptions& options, + bool blocked_by_policy); virtual void OnIndexedDBAccessed(const GURL& url, const string16& description, bool blocked_by_policy); diff --git a/chrome/browser/tab_contents/tab_specific_content_settings_unittest.cc b/chrome/browser/tab_contents/tab_specific_content_settings_unittest.cc index 5be081c..d23d877 100644 --- a/chrome/browser/tab_contents/tab_specific_content_settings_unittest.cc +++ b/chrome/browser/tab_contents/tab_specific_content_settings_unittest.cc @@ -53,7 +53,7 @@ TEST(TabSpecificContentSettingsTest, BlockedContent) { EXPECT_FALSE(content_settings.IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS)); // Set a cookie, block access to images, block a popup. - content_settings.OnCookieAccessed( + content_settings.OnCookieChanged( GURL("http://google.com"), "A=B", options, false); EXPECT_TRUE(test_delegate.SettingsChanged()); EXPECT_FALSE(test_delegate.ContentBlocked()); @@ -77,11 +77,11 @@ TEST(TabSpecificContentSettingsTest, BlockedContent) { EXPECT_FALSE( content_settings.IsContentBlocked(CONTENT_SETTINGS_TYPE_COOKIES)); EXPECT_TRUE(content_settings.IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS)); - content_settings.OnCookieAccessed( + content_settings.OnCookieChanged( GURL("http://google.com"), "A=B", options, false); // Block a cookie. - content_settings.OnCookieAccessed( + content_settings.OnCookieChanged( GURL("http://google.com"), "C=D", options, true); EXPECT_TRUE( content_settings.IsContentBlocked(CONTENT_SETTINGS_TYPE_COOKIES)); @@ -124,13 +124,13 @@ TEST(TabSpecificContentSettingsTest, AllowedContent) { content_settings.IsContentAccessed(CONTENT_SETTINGS_TYPE_COOKIES)); ASSERT_FALSE( content_settings.IsContentBlocked(CONTENT_SETTINGS_TYPE_COOKIES)); - content_settings.OnCookieAccessed( + content_settings.OnCookieChanged( GURL("http://google.com"), "A=B", options, false); ASSERT_TRUE( content_settings.IsContentAccessed(CONTENT_SETTINGS_TYPE_COOKIES)); ASSERT_FALSE( content_settings.IsContentBlocked(CONTENT_SETTINGS_TYPE_COOKIES)); - content_settings.OnCookieAccessed( + content_settings.OnCookieChanged( GURL("http://google.com"), "C=D", options, true); ASSERT_TRUE( content_settings.IsContentAccessed(CONTENT_SETTINGS_TYPE_COOKIES)); diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc index 1a987fe..04de3ca 100644 --- a/net/base/cookie_monster.cc +++ b/net/base/cookie_monster.cc @@ -1356,13 +1356,12 @@ CookieList CookieMonster::GetAllCookies() { return cookie_list; } -CookieList CookieMonster::GetAllCookiesForURL(const GURL& url) { +CookieList CookieMonster::GetAllCookiesForURLWithOptions( + const GURL& url, + const CookieOptions& options) { AutoLock autolock(lock_); InitIfNecessary(); - CookieOptions options; - options.set_include_httponly(); - std::vector cookie_ptrs; FindCookiesForHostAndDomain(url, options, false, &cookie_ptrs); std::sort(cookie_ptrs.begin(), cookie_ptrs.end(), CookieSorter); @@ -1375,6 +1374,13 @@ CookieList CookieMonster::GetAllCookiesForURL(const GURL& url) { return cookies; } +CookieList CookieMonster::GetAllCookiesForURL(const GURL& url) { + CookieOptions options; + options.set_include_httponly(); + + return GetAllCookiesForURLWithOptions(url, options); +} + void CookieMonster::FindCookiesForHostAndDomain( const GURL& url, const CookieOptions& options, diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h index 0b11130..2e9614e 100644 --- a/net/base/cookie_monster.h +++ b/net/base/cookie_monster.h @@ -169,10 +169,15 @@ class CookieMonster : public CookieStore { CookieList GetAllCookies(); // Returns all the cookies, for use in management UI, etc. Filters results - // using given url scheme, host / domain and path. This does not mark the - // cookies as having been accessed. + // using given url scheme, host / domain and path and options. This does not + // mark the cookies as having been accessed. // The returned cookies are ordered by longest path, then earliest // creation date. + CookieList GetAllCookiesForURLWithOptions(const GURL& url, + const CookieOptions& options); + + // Invokes GetAllCookiesForURLWithOptions with options set to include HTTP + // only cookies. CookieList GetAllCookiesForURL(const GURL& url); // Deletes all of the cookies. diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc index e807ca1..a4bc58f 100644 --- a/net/base/cookie_monster_unittest.cc +++ b/net/base/cookie_monster_unittest.cc @@ -1217,6 +1217,17 @@ TEST(CookieMonsterTest, GetAllCookiesForURL) { ASSERT_TRUE(++it == cookies.end()); + // Check cookies for url excluding http-only cookies. + cookies = + cm->GetAllCookiesForURLWithOptions(url_google, net::CookieOptions()); + it = cookies.begin(); + + ASSERT_TRUE(it != cookies.end()); + EXPECT_EQ(".google.izzle", it->Domain()); + EXPECT_EQ("C", it->Name()); + + ASSERT_TRUE(++it == cookies.end()); + // Test secure cookies. cookies = cm->GetAllCookiesForURL(url_google_secure); it = cookies.begin(); diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc index 888e94c..c951088 100644 --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -451,18 +451,20 @@ void URLRequestHttpJob::StopCaching() { void URLRequestHttpJob::OnCanGetCookiesCompleted(int policy) { // If the request was destroyed, then there is no more work to do. if (request_ && request_->delegate()) { - if (policy == net::ERR_ACCESS_DENIED) { - request_->delegate()->OnGetCookies(request_, true); - } else if (policy == net::OK && request_->context()->cookie_store()) { - request_->delegate()->OnGetCookies(request_, false); - net::CookieOptions options; - options.set_include_httponly(); - std::string cookies = - request_->context()->cookie_store()->GetCookiesWithOptions( - request_->url(), options); - if (!cookies.empty()) { - request_info_.extra_headers.SetHeader( - net::HttpRequestHeaders::kCookie, cookies); + if (request_->context()->cookie_store()) { + if (policy == net::ERR_ACCESS_DENIED) { + request_->delegate()->OnGetCookies(request_, true); + } else if (policy == net::OK) { + request_->delegate()->OnGetCookies(request_, false); + net::CookieOptions options; + options.set_include_httponly(); + std::string cookies = + request_->context()->cookie_store()->GetCookiesWithOptions( + request_->url(), options); + if (!cookies.empty()) { + request_info_.extra_headers.SetHeader( + net::HttpRequestHeaders::kCookie, cookies); + } } } // We may have been canceled within OnGetCookies. @@ -478,27 +480,28 @@ void URLRequestHttpJob::OnCanGetCookiesCompleted(int policy) { void URLRequestHttpJob::OnCanSetCookieCompleted(int policy) { // If the request was destroyed, then there is no more work to do. if (request_ && request_->delegate()) { - if (policy == net::ERR_ACCESS_DENIED) { - request_->delegate()->OnSetCookie( - request_, - response_cookies_[response_cookies_save_index_], - net::CookieOptions(), - true); - } else if ((policy == net::OK || policy == net::OK_FOR_SESSION_ONLY) && - request_->context()->cookie_store()) { - // OK to save the current response cookie now. - net::CookieOptions options; - options.set_include_httponly(); - if (policy == net::OK_FOR_SESSION_ONLY) - options.set_force_session(); - request_->context()->cookie_store()->SetCookieWithOptions( - request_->url(), response_cookies_[response_cookies_save_index_], - options); - request_->delegate()->OnSetCookie( - request_, - response_cookies_[response_cookies_save_index_], - options, - false); + if (request_->context()->cookie_store()) { + if (policy == net::ERR_ACCESS_DENIED) { + request_->delegate()->OnSetCookie( + request_, + response_cookies_[response_cookies_save_index_], + net::CookieOptions(), + true); + } else if (policy == net::OK || policy == net::OK_FOR_SESSION_ONLY) { + // OK to save the current response cookie now. + net::CookieOptions options; + options.set_include_httponly(); + if (policy == net::OK_FOR_SESSION_ONLY) + options.set_force_session(); + request_->context()->cookie_store()->SetCookieWithOptions( + request_->url(), response_cookies_[response_cookies_save_index_], + options); + request_->delegate()->OnSetCookie( + request_, + response_cookies_[response_cookies_save_index_], + options, + false); + } } response_cookies_save_index_++; // We may have been canceled within OnSetCookie. -- cgit v1.1