diff options
22 files changed, 1077 insertions, 430 deletions
diff --git a/chrome/browser/automation/automation_profile_impl.cc b/chrome/browser/automation/automation_profile_impl.cc index 3a1387d..d09b036 100644 --- a/chrome/browser/automation/automation_profile_impl.cc +++ b/chrome/browser/automation/automation_profile_impl.cc @@ -58,8 +58,12 @@ class AutomationCookieStore : public net::CookieStore { } // CookieStore implementation. - virtual bool SetCookie(const GURL& url, const std::string& cookie_line) { - bool cookie_set = original_cookie_store_->SetCookie(url, cookie_line); + virtual bool SetCookieWithOptions(const GURL& url, + const std::string& cookie_line, + const net::CookieOptions& options) { + bool cookie_set = + original_cookie_store_->SetCookieWithOptions(url, cookie_line, + options); if (cookie_set) { // TODO(eroman): Should NOT be accessing the profile from here, as this // is running on the IO thread. @@ -68,48 +72,14 @@ class AutomationCookieStore : public net::CookieStore { } return cookie_set; } - virtual bool SetCookieWithOptions(const GURL& url, - const std::string& cookie_line, - const net::CookieOptions& options) { - return original_cookie_store_->SetCookieWithOptions(url, cookie_line, - options); - } - virtual bool SetCookieWithCreationTime(const GURL& url, - const std::string& cookie_line, - const base::Time& creation_time) { - return original_cookie_store_->SetCookieWithCreationTime(url, cookie_line, - creation_time); - } - virtual bool SetCookieWithCreationTimeWithOptions( - const GURL& url, - const std::string& cookie_line, - const base::Time& creation_time, - const net::CookieOptions& options) { - return original_cookie_store_->SetCookieWithCreationTimeWithOptions(url, - cookie_line, creation_time, options); - } - virtual void SetCookies(const GURL& url, - const std::vector<std::string>& cookies) { - original_cookie_store_->SetCookies(url, cookies); - } - virtual void SetCookiesWithOptions(const GURL& url, - const std::vector<std::string>& cookies, - const net::CookieOptions& options) { - original_cookie_store_->SetCookiesWithOptions(url, cookies, options); - } - virtual std::string GetCookies(const GURL& url) { - return original_cookie_store_->GetCookies(url); - } virtual std::string GetCookiesWithOptions(const GURL& url, const net::CookieOptions& options) { return original_cookie_store_->GetCookiesWithOptions(url, options); } - virtual void DeleteCookie(const GURL& url, const std::string& cookie_name) { return original_cookie_store_->DeleteCookie(url, cookie_name); } - virtual net::CookieMonster* GetCookieMonster() { return original_cookie_store_->GetCookieMonster(); } diff --git a/chrome/browser/automation/url_request_automation_job.cc b/chrome/browser/automation/url_request_automation_job.cc index 319b976..77e927f 100644 --- a/chrome/browser/automation/url_request_automation_job.cc +++ b/chrome/browser/automation/url_request_automation_job.cc @@ -13,7 +13,6 @@ #include "chrome/browser/renderer_host/resource_dispatcher_host_request_info.h" #include "chrome/test/automation/automation_messages.h" #include "net/base/cookie_monster.h" -#include "net/base/cookie_policy.h" #include "net/base/io_buffer.h" #include "net/base/net_errors.h" #include "net/http/http_util.h" @@ -298,6 +297,9 @@ void URLRequestAutomationJob::OnRequestStarted(int tab, int id, URLRequestContext* ctx = request_->context(); std::vector<std::string> response_cookies; + // NOTE: We ignore Chrome's cookie policy to allow the automation to + // decide what cookies should be set. + if (!response.headers.empty()) { headers_ = new net::HttpResponseHeaders( net::HttpUtil::AssembleRawHeaders(response.headers.data(), @@ -312,30 +314,23 @@ void URLRequestAutomationJob::OnRequestStarted(int tab, int id, response_cookies.push_back(value); } - if (response_cookies.size()) { - if (ctx && ctx->cookie_store() && (!ctx->cookie_policy() || - ctx->cookie_policy()->CanSetCookie( - url_for_cookies, request_->first_party_for_cookies()))) { - net::CookieOptions options; - options.set_include_httponly(); - ctx->cookie_store()->SetCookiesWithOptions(url_for_cookies, - response_cookies, - options); - } + if (response_cookies.size() && ctx && ctx->cookie_store()) { + net::CookieOptions options; + options.set_include_httponly(); + ctx->cookie_store()->SetCookiesWithOptions(url_for_cookies, + response_cookies, + options); } } - if (!response.persistent_cookies.empty() && ctx && ctx->cookie_store() && - (!ctx->cookie_policy() || ctx->cookie_policy()->CanSetCookie( - url_for_cookies, request_->first_party_for_cookies()))) { + if (!response.persistent_cookies.empty() && ctx && ctx->cookie_store()) { StringTokenizer cookie_parser(response.persistent_cookies, ";"); net::CookieMonster::CookieList existing_cookies; net::CookieMonster* monster = ctx->cookie_store()->GetCookieMonster(); DCHECK(monster); - if (monster) { - existing_cookies = monster->GetRawCookies(url_for_cookies); - } + if (monster) + existing_cookies = monster->GetAllCookiesForURL(url_for_cookies); while (cookie_parser.GetNext()) { std::string cookie_string = cookie_parser.token(); diff --git a/chrome/browser/net/chrome_cookie_policy.cc b/chrome/browser/net/chrome_cookie_policy.cc new file mode 100644 index 0000000..3e924d1 --- /dev/null +++ b/chrome/browser/net/chrome_cookie_policy.cc @@ -0,0 +1,162 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/net/chrome_cookie_policy.h" + +#include "base/string_util.h" +#include "chrome/browser/chrome_thread.h" +#include "chrome/browser/host_content_settings_map.h" +#include "googleurl/src/gurl.h" +#include "net/base/net_errors.h" +#include "net/base/static_cookie_policy.h" + +// If we queue up more than this number of completions, then switch from ASK to +// BLOCK. More than this number of requests at once seems like it could be a +// sign of trouble anyways. +static const size_t kMaxCompletionsPerHost = 10000; + +ChromeCookiePolicy::ChromeCookiePolicy(HostContentSettingsMap* map) + : host_content_settings_map_(map) { +} + +ChromeCookiePolicy::~ChromeCookiePolicy() { + DCHECK(host_completions_map_.empty()); +} + +int ChromeCookiePolicy::CanGetCookies(const GURL& url, + const GURL& first_party, + net::CompletionCallback* callback) { + if (host_content_settings_map_->BlockThirdPartyCookies()) { + net::StaticCookiePolicy policy( + net::StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES); + int rv = policy.CanGetCookies(url, first_party, NULL); + if (rv != net::OK) + return rv; + } + + const std::string& host = url.host(); + + ContentSetting setting = host_content_settings_map_->GetContentSetting( + host, CONTENT_SETTINGS_TYPE_COOKIES); + if (setting == CONTENT_SETTING_BLOCK) + return net::ERR_ACCESS_DENIED; + if (setting == CONTENT_SETTING_ALLOW) + return net::OK; + + DCHECK(callback); + + // If we are currently prompting the user for a 'set-cookie' matching this + // host, then we need to defer reading cookies. + + HostCompletionsMap::iterator it = host_completions_map_.find(host); + if (it == host_completions_map_.end()) + return net::OK; + + if (it->second.size() >= kMaxCompletionsPerHost) { + LOG(ERROR) << "Would exceed kMaxCompletionsPerHost"; + return net::ERR_ACCESS_DENIED; + } + + it->second.push_back(Completion::ForGetCookies(callback)); + + return net::ERR_IO_PENDING; +} + +int ChromeCookiePolicy::CanSetCookie(const GURL& url, + const GURL& first_party, + const std::string& cookie_line, + net::CompletionCallback* callback) { + if (host_content_settings_map_->BlockThirdPartyCookies()) { + net::StaticCookiePolicy policy( + net::StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES); + int rv = policy.CanSetCookie(url, first_party, cookie_line, NULL); + if (rv != net::OK) + return rv; + } + + const std::string& host = url.host(); + + ContentSetting setting = host_content_settings_map_->GetContentSetting( + host, CONTENT_SETTINGS_TYPE_COOKIES); + if (setting == CONTENT_SETTING_BLOCK) + return net::ERR_ACCESS_DENIED; + if (setting == CONTENT_SETTING_ALLOW) + return net::OK; + + DCHECK(callback); + + // Else, ask the user... + + Completions& completions = host_completions_map_[host]; + + if (completions.size() >= kMaxCompletionsPerHost) { + LOG(ERROR) << "Would exceed kMaxCompletionsPerHost"; + return net::ERR_ACCESS_DENIED; + } + + completions.push_back(Completion::ForSetCookie(callback)); + + PromptForSetCookie(host, cookie_line); + return net::ERR_IO_PENDING; +} + +void ChromeCookiePolicy::PromptForSetCookie(const std::string &host, + const std::string& cookie_line) { + if (!ChromeThread::CurrentlyOn(ChromeThread::UI)) { + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, + NewRunnableMethod(this, &ChromeCookiePolicy::PromptForSetCookie, host, + cookie_line)); + return; + } + + // TODO(darin): Prompt user! +#if 0 + MessageBox(NULL, + UTF8ToWide(cookie_line).c_str(), + UTF8ToWide(host).c_str(), + MB_OK); +#endif + + DidPromptForSetCookie(host, net::OK); +} + +void ChromeCookiePolicy::DidPromptForSetCookie(const std::string &host, + int result) { + if (!ChromeThread::CurrentlyOn(ChromeThread::IO)) { + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &ChromeCookiePolicy::DidPromptForSetCookie, + host, result)); + return; + } + + // Notify all callbacks, starting with the first until we hit another that + // is for a 'set-cookie'. + HostCompletionsMap::iterator it = host_completions_map_.find(host); + CHECK(it != host_completions_map_.end()); + + Completions& completions = it->second; + CHECK(!completions.empty() && completions[0].is_set_cookie_request()); + + // Gather the list of callbacks to notify, and remove them from the + // completions list before handing control to the callbacks (in case + // they should call back into us to modify host_completions_map_). + + std::vector<net::CompletionCallback*> callbacks; + callbacks.push_back(completions[0].callback()); + size_t i = 1; + for (; i < completions.size(); ++i) { + if (completions[i].is_set_cookie_request()) + break; + callbacks.push_back(completions[i].callback()); + } + completions.erase(completions.begin(), completions.begin() + i); + + if (completions.empty()) + host_completions_map_.erase(it); + + for (size_t j = 0; j < callbacks.size(); ++j) + callbacks[j]->Run(result); +} diff --git a/chrome/browser/net/chrome_cookie_policy.h b/chrome/browser/net/chrome_cookie_policy.h new file mode 100644 index 0000000..0d536f8 --- /dev/null +++ b/chrome/browser/net/chrome_cookie_policy.h @@ -0,0 +1,74 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_NET_CHROME_COOKIE_POLICY_H_ +#define CHROME_BROWSER_NET_CHROME_COOKIE_POLICY_H_ + +#include <map> +#include <vector> + +#include "base/ref_counted.h" +#include "net/base/cookie_policy.h" + +class HostContentSettingsMap; + +// Implements CookiePolicy that may delay queries to ask the user to decide. +// +// We will only prompt the user before setting a cookie. We do not prompt the +// user before getting a cookie. However, if we are in the process of +// prompting the user, then any requests to get cookies will be deferred. +// This is done so that cookie requests remain FIFO. +// +class ChromeCookiePolicy + : public base::RefCountedThreadSafe<ChromeCookiePolicy>, + public net::CookiePolicy { + public: + explicit ChromeCookiePolicy(HostContentSettingsMap* map); + ~ChromeCookiePolicy(); + + // CookiePolicy methods: + virtual int CanGetCookies(const GURL& url, const GURL& first_party, + net::CompletionCallback* callback); + virtual int CanSetCookie(const GURL& url, const GURL& first_party, + const std::string& cookie_line, + net::CompletionCallback* callback); + + private: + class Completion { + public: + static Completion ForSetCookie(net::CompletionCallback* callback) { + return Completion(true, callback); + } + + static Completion ForGetCookies(net::CompletionCallback* callback) { + return Completion(false, callback); + } + + bool is_set_cookie_request() const { return is_set_cookie_request_; } + net::CompletionCallback* callback() const { return callback_; } + + private: + Completion(bool is_set_cookie_request, net::CompletionCallback* callback) + : is_set_cookie_request_(is_set_cookie_request), + callback_(callback) { + } + + bool is_set_cookie_request_; + net::CompletionCallback* callback_; + }; + + typedef std::vector<Completion> Completions; + typedef std::map<std::string, Completions> HostCompletionsMap; + + void PromptForSetCookie(const std::string& host, + const std::string& cookie_line); + void DidPromptForSetCookie(const std::string& host, int result); + + // A map from hostname to callbacks awaiting a cookie policy response. + HostCompletionsMap host_completions_map_; + + scoped_refptr<HostContentSettingsMap> host_content_settings_map_; +}; + +#endif // CHROME_BROWSER_NET_CHROME_COOKIE_POLICY_H_ diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc index 6b97030..8013800 100644 --- a/chrome/browser/net/chrome_url_request_context.cc +++ b/chrome/browser/net/chrome_url_request_context.cc @@ -184,6 +184,9 @@ ChromeURLRequestContext* FactoryForOriginal::Create() { context->set_cookie_store(new net::CookieMonster(cookie_db.get())); } + context->set_cookie_policy( + new ChromeCookiePolicy(host_content_settings_map_)); + // Create a new AppCacheService (issues fetches through the // main URLRequestContext that we just created). context->set_appcache_service( @@ -262,6 +265,8 @@ ChromeURLRequestContext* FactoryForOffTheRecord::Create() { new net::HttpCache(context->host_resolver(), context->proxy_service(), context->ssl_config_service(), 0); context->set_cookie_store(new net::CookieMonster); + context->set_cookie_policy( + new ChromeCookiePolicy(host_content_settings_map_)); context->set_http_transaction_factory(cache); if (CommandLine::ForCurrentProcess()->HasSwitch( @@ -300,6 +305,7 @@ ChromeURLRequestContext* FactoryForOffTheRecordExtensions::Create() { const char* schemes[] = {chrome::kExtensionScheme}; cookie_monster->SetCookieableSchemes(schemes, 1); context->set_cookie_store(cookie_monster); + // No dynamic cookie policy for extensions. return context; } @@ -341,6 +347,8 @@ ChromeURLRequestContext* FactoryForMedia::Create() { context->set_proxy_service(main_context->proxy_service()); // Also share the cookie store of the common profile. context->set_cookie_store(main_context->cookie_store()); + context->set_cookie_policy( + static_cast<ChromeCookiePolicy*>(main_context->cookie_policy())); // Create a media cache with default size. // TODO(hclam): make the maximum size of media cache configurable. @@ -606,8 +614,6 @@ void ChromeURLRequestContextGetter::GetCookieStoreAsyncHelper( ChromeURLRequestContext::ChromeURLRequestContext() { CheckCurrentlyOnIOThread(); - cookie_policy_ = this; // We implement CookiePolicy - url_request_tracker()->SetGraveyardFilter( &ChromeURLRequestContext::ShouldTrackRequest); } @@ -640,6 +646,9 @@ ChromeURLRequestContext::~ChromeURLRequestContext() { delete ftp_transaction_factory_; delete http_transaction_factory_; + // cookie_policy_'s lifetime is auto-managed by chrome_cookie_policy_. We + // null this out here to avoid a dangling reference to chrome_cookie_policy_ + // when ~URLRequestContext runs. cookie_policy_ = NULL; } @@ -756,48 +765,10 @@ bool ChromeURLRequestContext::AreCookiesEnabled() const { return setting != CONTENT_SETTING_BLOCK; } -bool ChromeURLRequestContext::CanGetCookies(const GURL& url, - const GURL& first_party) { - if (host_content_settings_map_->BlockThirdPartyCookies()) { - net::StaticCookiePolicy policy( - net::StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES); - if (!policy.CanGetCookies(url, first_party)) - return false; - } - - ContentSetting setting = host_content_settings_map_->GetContentSetting( - url.host(), CONTENT_SETTINGS_TYPE_COOKIES); - if (setting == CONTENT_SETTING_BLOCK) - return false; - - // TODO(darin): Implement CONTENT_SETTING_ASK - return true; -} - -bool ChromeURLRequestContext::CanSetCookie(const GURL& url, - const GURL& first_party) { - if (host_content_settings_map_->BlockThirdPartyCookies()) { - net::StaticCookiePolicy policy( - net::StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES); - if (!policy.CanSetCookie(url, first_party)) - return false; - } - - ContentSetting setting = host_content_settings_map_->GetContentSetting( - url.host(), CONTENT_SETTINGS_TYPE_COOKIES); - if (setting == CONTENT_SETTING_BLOCK) - return false; - - // TODO(darin): Implement CONTENT_SETTING_ASK - return true; -} - ChromeURLRequestContext::ChromeURLRequestContext( ChromeURLRequestContext* other) { CheckCurrentlyOnIOThread(); - cookie_policy_ = this; // We implement CookiePolicy - // Set URLRequestContext members host_resolver_ = other->host_resolver_; proxy_service_ = other->proxy_service_; @@ -805,6 +776,7 @@ ChromeURLRequestContext::ChromeURLRequestContext( http_transaction_factory_ = other->http_transaction_factory_; ftp_transaction_factory_ = other->ftp_transaction_factory_; cookie_store_ = other->cookie_store_; + cookie_policy_ = other->cookie_policy_; transport_security_state_ = other->transport_security_state_; accept_language_ = other->accept_language_; accept_charset_ = other->accept_charset_; @@ -814,6 +786,7 @@ ChromeURLRequestContext::ChromeURLRequestContext( extension_info_ = other->extension_info_; user_script_dir_path_ = other->user_script_dir_path_; appcache_service_ = other->appcache_service_; + chrome_cookie_policy_ = other->chrome_cookie_policy_; host_content_settings_map_ = other->host_content_settings_map_; host_zoom_map_ = other->host_zoom_map_; privacy_blacklist_ = other->privacy_blacklist_; diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h index 8d4186d..f52029e 100644 --- a/chrome/browser/net/chrome_url_request_context.h +++ b/chrome/browser/net/chrome_url_request_context.h @@ -11,6 +11,7 @@ #include "chrome/browser/host_content_settings_map.h" #include "chrome/browser/host_zoom_map.h" #include "chrome/browser/privacy_blacklist/blacklist.h" +#include "chrome/browser/net/chrome_cookie_policy.h" #include "chrome/browser/net/url_request_context_getter.h" #include "chrome/common/appcache/chrome_appcache_service.h" #include "chrome/common/extensions/extension.h" @@ -34,8 +35,7 @@ class IOThread; // // All methods of this class must be called from the IO thread, // including the constructor and destructor. -class ChromeURLRequestContext : public URLRequestContext, - public net::CookiePolicy { +class ChromeURLRequestContext : public URLRequestContext { public: // Maintains some extension-related state we need on the IO thread. // TODO(aa): It would be cool if the Extension objects in ExtensionsService @@ -121,10 +121,6 @@ class ChromeURLRequestContext : public URLRequestContext, // False only if cookies are globally blocked without exception. bool AreCookiesEnabled() const; - // CookiePolicy methods: - virtual bool CanGetCookies(const GURL& url, const GURL& first_party); - virtual bool CanSetCookie(const GURL& url, const GURL& first_party); - protected: // Copies the dependencies from |other| into |this|. If you use this // constructor, then you should hold a reference to |other|, as we @@ -167,6 +163,10 @@ class ChromeURLRequestContext : public URLRequestContext, void set_cookie_store(net::CookieStore* cookie_store) { cookie_store_ = cookie_store; } + void set_cookie_policy(ChromeCookiePolicy* cookie_policy) { + chrome_cookie_policy_ = cookie_policy; // Take a strong reference. + cookie_policy_ = cookie_policy; + } void set_proxy_service(net::ProxyService* service) { proxy_service_ = service; } @@ -206,6 +206,7 @@ class ChromeURLRequestContext : public URLRequestContext, FilePath user_script_dir_path_; scoped_refptr<ChromeAppCacheService> appcache_service_; + scoped_refptr<ChromeCookiePolicy> chrome_cookie_policy_; scoped_refptr<HostContentSettingsMap> host_content_settings_map_; scoped_refptr<HostZoomMap> host_zoom_map_; scoped_refptr<Blacklist> privacy_blacklist_; diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc index 1bcb7a7..2861718 100644 --- a/chrome/browser/renderer_host/resource_message_filter.cc +++ b/chrome/browser/renderer_host/resource_message_filter.cc @@ -152,6 +152,99 @@ Blacklist::Match* GetPrivacyBlacklistMatchForURL( return blacklist->FindMatch(url); } +class SetCookieCompletion : public net::CompletionCallback { + public: + SetCookieCompletion(const GURL& url, const std::string& cookie_line, + URLRequestContext* context) + : url_(url), + cookie_line_(cookie_line), + context_(context) { + } + + virtual void RunWithParams(const Tuple1<int>& params) { + int result = params.a; + if (result == net::OK) + context_->cookie_store()->SetCookie(url_, cookie_line_); + delete this; + } + + private: + GURL url_; + std::string cookie_line_; + scoped_refptr<URLRequestContext> context_; +}; + +class GetCookiesCompletion : public net::CompletionCallback { + public: + GetCookiesCompletion(const GURL& url, IPC::Message* reply_msg, + ResourceMessageFilter* filter, + URLRequestContext* context) + : url_(url), + reply_msg_(reply_msg), + filter_(filter), + context_(context) { + } + + virtual void RunWithParams(const Tuple1<int>& params) { + int result = params.a; + std::string cookies; + if (result == net::OK) + cookies = context_->cookie_store()->GetCookies(url_); + ViewHostMsg_GetCookies::WriteReplyParams(reply_msg_, cookies); + filter_->Send(reply_msg_); + delete this; + } + + private: + GURL url_; + IPC::Message* reply_msg_; + scoped_refptr<ResourceMessageFilter> filter_; + scoped_refptr<URLRequestContext> context_; +}; + +class GetRawCookiesCompletion : public net::CompletionCallback { + public: + GetRawCookiesCompletion(const GURL& url, IPC::Message* reply_msg, + ResourceMessageFilter* filter, + URLRequestContext* context) + : url_(url), + reply_msg_(reply_msg), + filter_(filter), + context_(context) { + } + + virtual void RunWithParams(const Tuple1<int>& params) { + // Ignore the policy result. We only waited on the policy result so that + // any pending 'set-cookie' requests could be flushed. The intent of + // querying the raw cookies is to reveal the contents of the cookie DB, so + // it important that we don't read the cookie db ahead of pending writes. + + net::CookieMonster* cookie_monster = + context_->cookie_store()->GetCookieMonster(); + net::CookieMonster::CookieList cookie_list = + cookie_monster->GetAllCookiesForURL(url_); + + std::vector<webkit_glue::WebCookie> cookies; + for (size_t i = 0; i < cookie_list.size(); ++i) { + // TODO(darin): url.host() is not necessarily the domain of the cookie. + // We need a different API on CookieMonster to provide the domain info. + // See http://crbug.com/34315. + cookies.push_back( + webkit_glue::WebCookie(cookie_list[i].first, cookie_list[i].second)); + } + + ViewHostMsg_GetRawCookies::WriteReplyParams(reply_msg_, cookies); + filter_->Send(reply_msg_); + delete this; + } + + private: + GURL url_; + IPC::Message* reply_msg_; + scoped_refptr<ResourceMessageFilter> filter_; + scoped_refptr<URLRequestContext> context_; +}; + } // namespace ResourceMessageFilter::ResourceMessageFilter( @@ -302,8 +395,8 @@ bool ResourceMessageFilter::OnMessageReceived(const IPC::Message& msg) { IPC_MESSAGE_HANDLER(ViewHostMsg_CreateWindow, OnMsgCreateWindow) IPC_MESSAGE_HANDLER(ViewHostMsg_CreateWidget, OnMsgCreateWidget) IPC_MESSAGE_HANDLER(ViewHostMsg_SetCookie, OnSetCookie) - IPC_MESSAGE_HANDLER(ViewHostMsg_GetCookies, OnGetCookies) - IPC_MESSAGE_HANDLER(ViewHostMsg_GetRawCookies, OnGetRawCookies) + IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_GetCookies, OnGetCookies) + IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_GetRawCookies, OnGetRawCookies) IPC_MESSAGE_HANDLER(ViewHostMsg_DeleteCookie, OnDeleteCookie) IPC_MESSAGE_HANDLER(ViewHostMsg_GetCookiesEnabled, OnGetCookiesEnabled) #if defined(OS_WIN) // This hack is Windows-specific. @@ -483,58 +576,55 @@ void ResourceMessageFilter::OnSetCookie(const GURL& url, const std::string& cookie) { ChromeURLRequestContext* context = GetRequestContextForURL(url); - DCHECK(context->cookie_policy()); - if (!context->cookie_policy()->CanSetCookie(url, first_party_for_cookies)) - return; scoped_ptr<Blacklist::Match> match( GetPrivacyBlacklistMatchForURL(url, context)); if (match.get() && (match->attributes() & Blacklist::kBlockCookies)) return; - context->cookie_store()->SetCookie(url, cookie); + + SetCookieCompletion* callback = new SetCookieCompletion(url, cookie, context); + + DCHECK(context->cookie_policy()); + int policy = context->cookie_policy()->CanSetCookie( + url, first_party_for_cookies, cookie, callback); + if (policy == net::ERR_IO_PENDING) + return; + callback->Run(policy); } void ResourceMessageFilter::OnGetCookies(const GURL& url, const GURL& first_party_for_cookies, - std::string* cookies) { + IPC::Message* reply_msg) { URLRequestContext* context = GetRequestContextForURL(url); + + GetCookiesCompletion* callback = + new GetCookiesCompletion(url, reply_msg, this, context); + DCHECK(context->cookie_policy()); - if (context->cookie_policy()->CanGetCookies(url, first_party_for_cookies)) - *cookies = context->cookie_store()->GetCookies(url); + int policy = context->cookie_policy()->CanGetCookies( + url, first_party_for_cookies, callback); + if (policy == net::ERR_IO_PENDING) + return; + callback->Run(policy); } void ResourceMessageFilter::OnGetRawCookies( const GURL& url, const GURL& first_party_for_cookies, - std::vector<webkit_glue::WebCookie>* raw_cookies) { - raw_cookies->clear(); - + IPC::Message* reply_msg) { URLRequestContext* context = GetRequestContextForURL(url); - net::CookieMonster* cookie_monster = context->cookie_store()-> - GetCookieMonster(); - if (!cookie_monster) { - NOTREACHED(); - return; - } - if (!context->cookie_policy()->CanGetCookies(url, first_party_for_cookies)) - return; + GetRawCookiesCompletion* callback = + new GetRawCookiesCompletion(url, reply_msg, this, context); - typedef net::CookieMonster::CookieList CookieList; - CookieList cookieList = cookie_monster->GetRawCookies(url); - for (CookieList::iterator it = cookieList.begin(); - it != cookieList.end(); ++it) { - net::CookieMonster::CanonicalCookie& cookie = it->second; - raw_cookies->push_back( - webkit_glue::WebCookie( - cookie.Name(), - cookie.Value(), - it->first, - cookie.Path(), - cookie.ExpiryDate().ToDoubleT() * 1000, - cookie.IsHttpOnly(), - cookie.IsSecure(), - !cookie.IsPersistent())); - } + // We check policy here to avoid sending back cookies that would not normally + // be applied to outbound requests for the given URL. Since this cookie info + // is visible in the developer tools, it is helpful to make it match reality. + DCHECK(context->cookie_policy()); + int policy = context->cookie_policy()->CanGetCookies( + url, first_party_for_cookies, callback); + if (policy == net::ERR_IO_PENDING) + return; + callback->Run(policy); } void ResourceMessageFilter::OnDeleteCookie(const GURL& url, diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h index 3931c83..e813ae1 100644 --- a/chrome/browser/renderer_host/resource_message_filter.h +++ b/chrome/browser/renderer_host/resource_message_filter.h @@ -133,10 +133,10 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter, const std::string& cookie); void OnGetCookies(const GURL& url, const GURL& first_party_for_cookies, - std::string* cookies); + IPC::Message* reply_msg); void OnGetRawCookies(const GURL& url, const GURL& first_party_for_cookies, - std::vector<webkit_glue::WebCookie>* raw_cookies); + IPC::Message* reply_msg); void OnDeleteCookie(const GURL& url, const std::string& cookieName); void OnGetCookiesEnabled(const GURL& url, diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 343db16..8ced6dc 100755 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1253,6 +1253,8 @@ 'browser/modal_html_dialog_delegate.h', 'browser/net/browser_url_util.cc', 'browser/net/browser_url_util.h', + 'browser/net/chrome_cookie_policy.cc', + 'browser/net/chrome_cookie_policy.h', 'browser/net/chrome_url_request_context.cc', 'browser/net/chrome_url_request_context.h', 'browser/net/url_request_context_getter.cc', diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc index 992191c..805bfc1 100644 --- a/net/base/cookie_monster.cc +++ b/net/base/cookie_monster.cc @@ -404,44 +404,11 @@ void CookieMonster::SetCookieableSchemes( schemes, schemes + num_schemes); } -bool CookieMonster::SetCookie(const GURL& url, - const std::string& cookie_line) { - CookieOptions options; - return SetCookieWithOptions(url, cookie_line, options); -} - -bool CookieMonster::SetCookieWithOptions(const GURL& url, - const std::string& cookie_line, - const CookieOptions& options) { - Time creation_date; - { - AutoLock autolock(lock_); - creation_date = CurrentTime(); - last_time_seen_ = creation_date; - } - return SetCookieWithCreationTimeWithOptions(url, - cookie_line, - creation_date, - options); -} - -bool CookieMonster::SetCookieWithCreationTime(const GURL& url, - const std::string& cookie_line, - const Time& creation_time) { - CookieOptions options; - return SetCookieWithCreationTimeWithOptions(url, - cookie_line, - creation_time, - options); -} - -bool CookieMonster::SetCookieWithCreationTimeWithOptions( - const GURL& url, - const std::string& cookie_line, - const Time& creation_time, - const CookieOptions& options) { - DCHECK(!creation_time.is_null()); - +bool CookieMonster::SetCookieWithCreationTimeAndOptions( + const GURL& url, + const std::string& cookie_line, + const Time& creation_time_or_null, + const CookieOptions& options) { if (!HasCookieableScheme(url)) { return false; } @@ -451,6 +418,12 @@ bool CookieMonster::SetCookieWithCreationTimeWithOptions( COOKIE_DLOG(INFO) << "SetCookie() line: " << cookie_line; + Time creation_time = creation_time_or_null; + if (creation_time.is_null()) { + creation_time = CurrentTime(); + last_time_seen_ = creation_time; + } + // Parse the cookie. ParsedCookie pc(cookie_line); @@ -508,21 +481,6 @@ bool CookieMonster::SetCookieWithCreationTimeWithOptions( return true; } -void CookieMonster::SetCookies(const GURL& url, - const std::vector<std::string>& cookies) { - CookieOptions options; - SetCookiesWithOptions(url, cookies, options); -} - -void CookieMonster::SetCookiesWithOptions( - const GURL& url, - const std::vector<std::string>& cookies, - const CookieOptions& options) { - for (std::vector<std::string>::const_iterator iter = cookies.begin(); - iter != cookies.end(); ++iter) - SetCookieWithOptions(url, *iter, options); -} - void CookieMonster::InternalInsertCookie(const std::string& key, CanonicalCookie* cc, bool sync_to_store) { @@ -736,6 +694,12 @@ static bool CookieSorter(CookieMonster::CanonicalCookie* cc1, return cc1->Path().length() > cc2->Path().length(); } +bool CookieMonster::SetCookieWithOptions(const GURL& url, + const std::string& cookie_line, + const CookieOptions& options) { + return SetCookieWithCreationTimeAndOptions(url, cookie_line, Time(), options); +} + // Currently our cookie datastructure is based on Mozilla's approach. We have a // hash keyed on the cookie's domain, and for any query we walk down the domain // components and probe for cookies until we reach the TLD, where we stop. @@ -748,11 +712,6 @@ static bool CookieSorter(CookieMonster::CanonicalCookie* cc1, // search/prefix trie, where we reverse the hostname and query for all // keys that are a prefix of our hostname. I think the hash probing // should be fast and simple enough for now. -std::string CookieMonster::GetCookies(const GURL& url) { - CookieOptions options; - return GetCookiesWithOptions(url, options); -} - std::string CookieMonster::GetCookiesWithOptions(const GURL& url, const CookieOptions& options) { if (!HasCookieableScheme(url)) { @@ -834,7 +793,7 @@ CookieMonster::CookieList CookieMonster::GetAllCookies() { return cookie_list; } -CookieMonster::CookieList CookieMonster::GetRawCookies(const GURL& url) { +CookieMonster::CookieList CookieMonster::GetAllCookiesForURL(const GURL& url) { AutoLock autolock(lock_); InitIfNecessary(); diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h index 6976f7f..7578c46 100644 --- a/net/base/cookie_monster.h +++ b/net/base/cookie_monster.h @@ -73,30 +73,25 @@ class CookieMonster : public CookieStore { static base::Time ParseCookieTime(const std::string& time_string); // CookieStore implementation. - virtual bool SetCookie(const GURL& url, const std::string& cookie_line); virtual bool SetCookieWithOptions(const GURL& url, const std::string& cookie_line, const CookieOptions& options); - virtual bool SetCookieWithCreationTime(const GURL& url, - const std::string& cookie_line, - const base::Time& creation_time); - virtual bool SetCookieWithCreationTimeWithOptions( - const GURL& url, - const std::string& cookie_line, - const base::Time& creation_time, - const CookieOptions& options); - virtual void SetCookies(const GURL& url, - const std::vector<std::string>& cookies); - virtual void SetCookiesWithOptions(const GURL& url, - const std::vector<std::string>& cookies, - const CookieOptions& options); - virtual std::string GetCookies(const GURL& url); virtual std::string GetCookiesWithOptions(const GURL& url, const CookieOptions& options); virtual void DeleteCookie(const GURL& url, const std::string& cookie_name); - - virtual CookieMonster* GetCookieMonster() { - return this; + virtual CookieMonster* GetCookieMonster() { return this; } + + + // Exposed for unit testing. + bool SetCookieWithCreationTimeAndOptions(const GURL& url, + const std::string& cookie_line, + const base::Time& creation_time, + const CookieOptions& options); + bool SetCookieWithCreationTime(const GURL& url, + const std::string& cookie_line, + const base::Time& creation_time) { + return SetCookieWithCreationTimeAndOptions(url, cookie_line, creation_time, + CookieOptions()); } // Returns all the cookies, for use in management UI, etc. This does not mark @@ -106,7 +101,7 @@ class CookieMonster : public CookieStore { // Returns all the cookies, for use in management UI, etc. Filters results // using given url scheme and host / domain. This does not mark the cookies // as having been accessed. - CookieList GetRawCookies(const GURL& url); + CookieList GetAllCookiesForURL(const GURL& url); // Delete all of the cookies. int DeleteAll(bool sync_to_store); diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc index ba645ee..cdbf869 100644 --- a/net/base/cookie_monster_unittest.cc +++ b/net/base/cookie_monster_unittest.cc @@ -961,7 +961,7 @@ TEST(CookieMonsterTest, SetCookieableSchemes) { EXPECT_FALSE(cm_foo->SetCookie(http_url, "x=1")); } -TEST(CookieMonsterTest, GetRawCookies) { +TEST(CookieMonsterTest, GetAllCookiesForURL) { GURL url_google(kUrlGoogle); GURL url_google_secure(kUrlGoogleSecure); @@ -985,7 +985,8 @@ TEST(CookieMonsterTest, GetRawCookies) { PlatformThread::Sleep(kLastAccessThresholdMilliseconds + 20); // Check raw cookies. - net::CookieMonster::CookieList raw_cookies = cm->GetRawCookies(url_google); + net::CookieMonster::CookieList raw_cookies = + cm->GetAllCookiesForURL(url_google); net::CookieMonster::CookieList::iterator it = raw_cookies.begin(); ASSERT_TRUE(it != raw_cookies.end()); @@ -999,7 +1000,7 @@ TEST(CookieMonsterTest, GetRawCookies) { ASSERT_TRUE(++it == raw_cookies.end()); // Test secure cookies. - raw_cookies = cm->GetRawCookies(url_google_secure); + raw_cookies = cm->GetAllCookiesForURL(url_google_secure); it = raw_cookies.begin(); ASSERT_TRUE(it != raw_cookies.end()); diff --git a/net/base/cookie_policy.h b/net/base/cookie_policy.h index 91bbfb9..d2df2f5 100644 --- a/net/base/cookie_policy.h +++ b/net/base/cookie_policy.h @@ -5,19 +5,32 @@ #ifndef NET_BASE_COOKIE_POLICY_H_ #define NET_BASE_COOKIE_POLICY_H_ +#include "net/base/completion_callback.h" + class GURL; namespace net { class CookiePolicy { public: - // Determine if the URL's cookies may be read. - virtual bool CanGetCookies(const GURL& url, - const GURL& first_party_for_cookies) = 0; - - // Determine if the URL's cookies may be written. - virtual bool CanSetCookie(const GURL& url, - const GURL& first_party_for_cookies) = 0; + // Determines if the URL's cookies may be read. Returns OK if allowed to + // read cookies for the given URL. Returns ERR_IO_PENDING to indicate that + // the completion callback will be notified (asynchronously and on the + // current thread) of the final result. Note: The completion callback must + // remain valid until notified. + virtual int CanGetCookies(const GURL& url, + const GURL& first_party_for_cookies, + CompletionCallback* callback) = 0; + + // Determines if the URL's cookies may be written. Returns OK if allowed to + // write a cookie for the given URL. Returns ERR_IO_PENDING to indicate that + // the completion callback will be notified (asynchronously and on the + // current thread) of the final result. Note: The completion callback must + // remain valid until notified. + virtual int CanSetCookie(const GURL& url, + const GURL& first_party_for_cookies, + const std::string& cookie_line, + CompletionCallback* callback) = 0; protected: virtual ~CookiePolicy() {} diff --git a/net/base/cookie_store.h b/net/base/cookie_store.h index 1ea3fa2..2fbe1d8 100644 --- a/net/base/cookie_store.h +++ b/net/base/cookie_store.h @@ -24,45 +24,51 @@ class CookieMonster; // be thread safe as its methods can be accessed from IO as well as UI threads. class CookieStore : public base::RefCountedThreadSafe<CookieStore> { public: - // Set a single cookie. Expects a cookie line, like "a=1; domain=b.com". - virtual bool SetCookie(const GURL& url, const std::string& cookie_line) = 0; + // Sets a single cookie. Expects a cookie line, like "a=1; domain=b.com". virtual bool SetCookieWithOptions(const GURL& url, const std::string& cookie_line, const CookieOptions& options) = 0; - // Sets a single cookie with a specific creation date. To set a cookie with - // a creation date of Now() use SetCookie() instead (it calls this function - // internally). - virtual bool SetCookieWithCreationTime(const GURL& url, - const std::string& cookie_line, - const base::Time& creation_time) = 0; - virtual bool SetCookieWithCreationTimeWithOptions( - const GURL& url, - const std::string& cookie_line, - const base::Time& creation_time, - const CookieOptions& options) = 0; - // Set a vector of response cookie values for the same URL. - virtual void SetCookies(const GURL& url, - const std::vector<std::string>& cookies) = 0; - virtual void SetCookiesWithOptions(const GURL& url, - const std::vector<std::string>& cookies, - const CookieOptions& options) = 0; // TODO what if the total size of all the cookies >4k, can we have a header // that big or do we need multiple Cookie: headers? - // Simple interface, get a cookie string "a=b; c=d" for the given URL. - // It will _not_ return httponly cookies, see CookieOptions. - virtual std::string GetCookies(const GURL& url) = 0; + // Simple interface, gets a cookie string "a=b; c=d" for the given URL. + // Use options to access httponly cookies. virtual std::string GetCookiesWithOptions(const GURL& url, const CookieOptions& options) = 0; - virtual CookieMonster* GetCookieMonster() { - return NULL; - }; - // Deletes the passed in cookie for the specified URL. virtual void DeleteCookie(const GURL& url, const std::string& cookie_name) = 0; + // Returns the underlying CookieMonster. + virtual CookieMonster* GetCookieMonster() = 0; + + + // -------------------------------------------------------------------------- + // Helpers to make the above interface simpler for some cases. + + // Sets a cookie for the given URL using default options. + bool SetCookie(const GURL& url, const std::string& cookie_line) { + return SetCookieWithOptions(url, cookie_line, CookieOptions()); + } + + // Gets cookies for the given URL using default options. + std::string GetCookies(const GURL& url) { + return GetCookiesWithOptions(url, CookieOptions()); + } + + // Sets a vector of response cookie values for the same URL. + void SetCookiesWithOptions(const GURL& url, + const std::vector<std::string>& cookie_lines, + const CookieOptions& options) { + for (size_t i = 0; i < cookie_lines.size(); ++i) + SetCookieWithOptions(url, cookie_lines[i], options); + } + void SetCookies(const GURL& url, + const std::vector<std::string>& cookie_lines) { + SetCookiesWithOptions(url, cookie_lines, CookieOptions()); + } + protected: friend class base::RefCountedThreadSafe<CookieStore>; virtual ~CookieStore() {} diff --git a/net/base/static_cookie_policy.cc b/net/base/static_cookie_policy.cc index 25d58bc..0ff6ead 100644 --- a/net/base/static_cookie_policy.cc +++ b/net/base/static_cookie_policy.cc @@ -6,40 +6,44 @@ #include "base/logging.h" #include "googleurl/src/gurl.h" +#include "net/base/net_errors.h" #include "net/base/registry_controlled_domain.h" namespace net { -bool StaticCookiePolicy::CanGetCookies(const GURL& url, - const GURL& first_party_for_cookies) { +int StaticCookiePolicy::CanGetCookies(const GURL& url, + const GURL& first_party_for_cookies, + CompletionCallback* callback) { switch (type_) { case StaticCookiePolicy::ALLOW_ALL_COOKIES: - return true; + return OK; case StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES: - return true; + return OK; case StaticCookiePolicy::BLOCK_ALL_COOKIES: - return false; + return ERR_ACCESS_DENIED; default: NOTREACHED(); - return false; + return ERR_ACCESS_DENIED; } } -bool StaticCookiePolicy::CanSetCookie(const GURL& url, - const GURL& first_party_for_cookies) { +int StaticCookiePolicy::CanSetCookie(const GURL& url, + const GURL& first_party_for_cookies, + const std::string& cookie_line, + CompletionCallback* callback) { switch (type_) { case StaticCookiePolicy::ALLOW_ALL_COOKIES: - return true; + return OK; case StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES: if (first_party_for_cookies.is_empty()) - return true; // Empty first-party URL indicates a first-party request. - return net::RegistryControlledDomainService::SameDomainOrHost( - url, first_party_for_cookies); + return OK; // Empty first-party URL indicates a first-party request. + return RegistryControlledDomainService::SameDomainOrHost( + url, first_party_for_cookies) ? OK : ERR_ACCESS_DENIED; case StaticCookiePolicy::BLOCK_ALL_COOKIES: - return false; + return ERR_ACCESS_DENIED; default: NOTREACHED(); - return false; + return ERR_ACCESS_DENIED; } } diff --git a/net/base/static_cookie_policy.h b/net/base/static_cookie_policy.h index a16fd0c..4734d33 100644 --- a/net/base/static_cookie_policy.h +++ b/net/base/static_cookie_policy.h @@ -14,30 +14,18 @@ namespace net { // The StaticCookiePolicy class implements a static cookie policy that supports // three modes: allow all, deny all, or block third-party cookies. +// +// NOTE: This CookiePolicy implementation always completes synchronously. The +// callback parameter will be ignored if specified. Just pass NULL. +// class StaticCookiePolicy : public CookiePolicy { public: - // Consult the user's third-party cookie blocking preferences to determine - // whether the URL's cookies can be read. - bool CanGetCookies(const GURL& url, const GURL& first_party_for_cookies); - - // Consult the user's third-party cookie blocking preferences to determine - // whether the URL's cookies can be set. - bool CanSetCookie(const GURL& url, const GURL& first_party_for_cookies); - enum Type { ALLOW_ALL_COOKIES = 0, // Do not perform any cookie blocking. BLOCK_THIRD_PARTY_COOKIES, // Prevent third-party cookies from being set. BLOCK_ALL_COOKIES // Disable cookies. }; - // Sets the current policy to enforce. This should be called when the user's - // preferences change. - void set_type(Type type) { type_ = type; } - - Type type() const { - return type_; - } - StaticCookiePolicy() : type_(StaticCookiePolicy::ALLOW_ALL_COOKIES) { } @@ -46,6 +34,26 @@ class StaticCookiePolicy : public CookiePolicy { : type_(type) { } + // Sets the current policy to enforce. This should be called when the user's + // preferences change. + void set_type(Type type) { type_ = type; } + Type type() const { return type_; } + + // CookiePolicy methods: + + // Consults the user's third-party cookie blocking preferences to determine + // whether the URL's cookies can be read. + virtual int CanGetCookies(const GURL& url, + const GURL& first_party_for_cookies, + CompletionCallback* callback); + + // Consults the user's third-party cookie blocking preferences to determine + // whether the URL's cookies can be set. + virtual int CanSetCookie(const GURL& url, + const GURL& first_party_for_cookies, + const std::string& cookie_line, + CompletionCallback* callback); + private: Type type_; diff --git a/net/base/static_cookie_policy_unittest.cc b/net/base/static_cookie_policy_unittest.cc index 35af0fc..35c1a82 100644 --- a/net/base/static_cookie_policy_unittest.cc +++ b/net/base/static_cookie_policy_unittest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "net/base/net_errors.h" #include "net/base/static_cookie_policy.h" #include "testing/gtest/include/gtest/gtest.h" #include "googleurl/src/gurl.h" @@ -12,8 +13,19 @@ class StaticCookiePolicyTest : public testing::Test { : url_google_("http://www.google.izzle"), url_google_secure_("https://www.google.izzle"), url_google_mail_("http://mail.google.izzle"), - url_google_analytics_("http://www.googleanalytics.izzle") { } + url_google_analytics_("http://www.googleanalytics.izzle") { + } + void SetPolicyType(net::StaticCookiePolicy::Type type) { + policy_.set_type(type); + } + int CanGetCookies(const GURL& url, const GURL& first_party) { + return policy_.CanGetCookies(url, first_party, NULL); + } + int CanSetCookie(const GURL& url, const GURL& first_party) { + return policy_.CanSetCookie(url, first_party, std::string(), NULL); + } protected: + net::StaticCookiePolicy policy_; GURL url_google_; GURL url_google_secure_; GURL url_google_mail_; @@ -21,67 +33,63 @@ class StaticCookiePolicyTest : public testing::Test { }; TEST_F(StaticCookiePolicyTest, DefaultPolicyTest) { - net::StaticCookiePolicy cp; - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_secure_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_mail_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_analytics_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, GURL())); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_secure_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_mail_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_analytics_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, GURL())); - EXPECT_TRUE(cp.CanSetCookie(url_google_, url_google_)); - EXPECT_TRUE(cp.CanSetCookie(url_google_, url_google_secure_)); - EXPECT_TRUE(cp.CanSetCookie(url_google_, url_google_mail_)); - EXPECT_TRUE(cp.CanSetCookie(url_google_, url_google_analytics_)); - EXPECT_TRUE(cp.CanSetCookie(url_google_, GURL())); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, url_google_)); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, url_google_secure_)); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, url_google_mail_)); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, url_google_analytics_)); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, GURL())); } TEST_F(StaticCookiePolicyTest, AllowAllCookiesTest) { - net::StaticCookiePolicy cp; - cp.set_type(net::StaticCookiePolicy::ALLOW_ALL_COOKIES); + SetPolicyType(net::StaticCookiePolicy::ALLOW_ALL_COOKIES); - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_secure_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_mail_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_analytics_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, GURL())); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_secure_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_mail_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_analytics_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, GURL())); - EXPECT_TRUE(cp.CanSetCookie(url_google_, url_google_)); - EXPECT_TRUE(cp.CanSetCookie(url_google_, url_google_secure_)); - EXPECT_TRUE(cp.CanSetCookie(url_google_, url_google_mail_)); - EXPECT_TRUE(cp.CanSetCookie(url_google_, url_google_analytics_)); - EXPECT_TRUE(cp.CanSetCookie(url_google_, GURL())); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, url_google_)); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, url_google_secure_)); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, url_google_mail_)); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, url_google_analytics_)); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, GURL())); } TEST_F(StaticCookiePolicyTest, BlockThirdPartyCookiesTest) { - net::StaticCookiePolicy cp; - cp.set_type(net::StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES); + SetPolicyType(net::StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES); - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_secure_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_mail_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, url_google_analytics_)); - EXPECT_TRUE(cp.CanGetCookies(url_google_, GURL())); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_secure_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_mail_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, url_google_analytics_)); + EXPECT_EQ(net::OK, CanGetCookies(url_google_, GURL())); - EXPECT_TRUE(cp.CanSetCookie(url_google_, url_google_)); - EXPECT_TRUE(cp.CanSetCookie(url_google_, url_google_secure_)); - EXPECT_TRUE(cp.CanSetCookie(url_google_, url_google_mail_)); - EXPECT_FALSE(cp.CanSetCookie(url_google_, url_google_analytics_)); - EXPECT_TRUE(cp.CanSetCookie(url_google_, GURL())); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, url_google_)); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, url_google_secure_)); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, url_google_mail_)); + EXPECT_NE(net::OK, CanSetCookie(url_google_, url_google_analytics_)); + EXPECT_EQ(net::OK, CanSetCookie(url_google_, GURL())); } TEST_F(StaticCookiePolicyTest, BlockAllCookiesTest) { - net::StaticCookiePolicy cp; - cp.set_type(net::StaticCookiePolicy::BLOCK_ALL_COOKIES); + SetPolicyType(net::StaticCookiePolicy::BLOCK_ALL_COOKIES); - EXPECT_FALSE(cp.CanGetCookies(url_google_, url_google_)); - EXPECT_FALSE(cp.CanGetCookies(url_google_, url_google_secure_)); - EXPECT_FALSE(cp.CanGetCookies(url_google_, url_google_mail_)); - EXPECT_FALSE(cp.CanGetCookies(url_google_, url_google_analytics_)); - EXPECT_FALSE(cp.CanGetCookies(url_google_, GURL())); + EXPECT_NE(net::OK, CanGetCookies(url_google_, url_google_)); + EXPECT_NE(net::OK, CanGetCookies(url_google_, url_google_secure_)); + EXPECT_NE(net::OK, CanGetCookies(url_google_, url_google_mail_)); + EXPECT_NE(net::OK, CanGetCookies(url_google_, url_google_analytics_)); + EXPECT_NE(net::OK, CanGetCookies(url_google_, GURL())); - EXPECT_FALSE(cp.CanSetCookie(url_google_, url_google_)); - EXPECT_FALSE(cp.CanSetCookie(url_google_, url_google_secure_)); - EXPECT_FALSE(cp.CanSetCookie(url_google_, url_google_mail_)); - EXPECT_FALSE(cp.CanSetCookie(url_google_, url_google_analytics_)); - EXPECT_FALSE(cp.CanSetCookie(url_google_, GURL())); + EXPECT_NE(net::OK, CanSetCookie(url_google_, url_google_)); + EXPECT_NE(net::OK, CanSetCookie(url_google_, url_google_secure_)); + EXPECT_NE(net::OK, CanSetCookie(url_google_, url_google_mail_)); + EXPECT_NE(net::OK, CanSetCookie(url_google_, url_google_analytics_)); + EXPECT_NE(net::OK, CanSetCookie(url_google_, GURL())); } diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc index 4b08c64..15b3373 100644 --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -75,12 +75,17 @@ URLRequestHttpJob::URLRequestHttpJob(URLRequest* request) : URLRequestJob(request), context_(request->context()), response_info_(NULL), + response_cookies_save_index_(0), proxy_auth_state_(net::AUTH_STATE_DONT_NEED_AUTH), server_auth_state_(net::AUTH_STATE_DONT_NEED_AUTH), - ALLOW_THIS_IN_INITIALIZER_LIST( - start_callback_(this, &URLRequestHttpJob::OnStartCompleted)), - ALLOW_THIS_IN_INITIALIZER_LIST( - read_callback_(this, &URLRequestHttpJob::OnReadCompleted)), + ALLOW_THIS_IN_INITIALIZER_LIST(can_get_cookies_callback_( + this, &URLRequestHttpJob::OnCanGetCookiesCompleted)), + ALLOW_THIS_IN_INITIALIZER_LIST(can_set_cookie_callback_( + this, &URLRequestHttpJob::OnCanSetCookieCompleted)), + ALLOW_THIS_IN_INITIALIZER_LIST(start_callback_( + this, &URLRequestHttpJob::OnStartCompleted)), + ALLOW_THIS_IN_INITIALIZER_LIST(read_callback_( + this, &URLRequestHttpJob::OnReadCompleted)), read_in_progress_(false), transaction_(NULL), sdch_dictionary_advertised_(false), @@ -146,8 +151,7 @@ void URLRequestHttpJob::Start() { } AddExtraHeaders(); - - StartTransaction(); + AddCookieHeaderAndStart(); } void URLRequestHttpJob::Kill() { @@ -200,11 +204,11 @@ bool URLRequestHttpJob::GetResponseCookies( if (!response_info_) return false; - if (response_cookies_.empty()) - FetchResponseCookies(); + // TODO(darin): Why are we extracting response cookies again? Perhaps we + // should just leverage response_cookies_. cookies->clear(); - cookies->swap(response_cookies_); + FetchResponseCookies(response_info_, cookies); return true; } @@ -320,6 +324,8 @@ void URLRequestHttpJob::SetAuth(const std::wstring& username, void URLRequestHttpJob::RestartTransactionWithAuth( const std::wstring& username, const std::wstring& password) { + username_ = username; + password_ = password; // These will be reset in OnStartCompleted. response_info_ = NULL; @@ -327,27 +333,12 @@ void URLRequestHttpJob::RestartTransactionWithAuth( // Update the cookies, since the cookie store may have been updated from the // headers in the 401/407. Since cookies were already appended to - // extra_headers by AddExtraHeaders(), we need to strip them out. + // extra_headers, we need to strip them out before adding them again. static const char* const cookie_name[] = { "cookie" }; request_info_.extra_headers = net::HttpUtil::StripHeaders( request_info_.extra_headers, cookie_name, arraysize(cookie_name)); - // TODO(eroman): this ordering is inconsistent with non-restarted request, - // where cookies header appears second from the bottom. - request_info_.extra_headers += AssembleRequestCookies(); - - // No matter what, we want to report our status as IO pending since we will - // be notifying our consumer asynchronously via OnStartCompleted. - SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0)); - int rv = transaction_->RestartWithAuth(username, password, - &start_callback_); - if (rv == net::ERR_IO_PENDING) - return; - - // The transaction started synchronously, but we need to notify the - // URLRequest delegate via the message loop. - MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod( - this, &URLRequestHttpJob::OnStartCompleted, rv)); + AddCookieHeaderAndStart(); } void URLRequestHttpJob::CancelAuth() { @@ -438,6 +429,41 @@ bool URLRequestHttpJob::ReadRawData(net::IOBuffer* buf, int buf_size, return false; } +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::OK && request_->context()->cookie_store()) { + net::CookieOptions options; + options.set_include_httponly(); + std::string cookies = + request_->context()->cookie_store()->GetCookiesWithOptions( + request_->url(), options); + if (request_->context()->InterceptRequestCookies(request_, cookies) && + !cookies.empty()) + request_info_.extra_headers += "Cookie: " + cookies + "\r\n"; + } + StartTransaction(); + } + Release(); // Balance AddRef taken in AddCookieHeaderAndStart +} + +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::OK && request_->context()->cookie_store()) { + // OK to save the current response cookie now. + net::CookieOptions options; + options.set_include_httponly(); + request_->context()->cookie_store()->SetCookieWithOptions( + request_->url(), response_cookies_[response_cookies_save_index_], + options); + } + response_cookies_save_index_++; + SaveNextCookie(); + } + Release(); // Balance AddRef taken in SaveNextCookie +} + void URLRequestHttpJob::OnStartCompleted(int result) { // If the request was destroyed, then there is no more work to do. if (!request_ || !request_->delegate()) @@ -452,7 +478,7 @@ void URLRequestHttpJob::OnStartCompleted(int result) { SetStatus(URLRequestStatus()); if (result == net::OK) { - NotifyHeadersComplete(); + SaveCookiesAndNotifyHeadersComplete(); } else if (ShouldTreatAsCertificateError(result)) { // We encountered an SSL certificate error. Ask our delegate to decide // what we should do. @@ -509,21 +535,6 @@ void URLRequestHttpJob::NotifyHeadersComplete() { // also need this info. is_cached_content_ = response_info_->was_cached; - // Get the Set-Cookie values, and send them to our cookie database. - if (!(request_info_.load_flags & net::LOAD_DO_NOT_SAVE_COOKIES)) { - URLRequestContext* ctx = request_->context(); - if (ctx && ctx->cookie_store() && (!ctx->cookie_policy() || - ctx->cookie_policy()->CanSetCookie( - request_->url(), request_->first_party_for_cookies()))) { - FetchResponseCookies(); - net::CookieOptions options; - options.set_include_httponly(); - ctx->cookie_store()->SetCookiesWithOptions(request_->url(), - response_cookies_, - options); - } - } - ProcessStrictTransportSecurityHeader(); if (SdchManager::Global() && @@ -568,26 +579,29 @@ void URLRequestHttpJob::DestroyTransaction() { void URLRequestHttpJob::StartTransaction() { // NOTE: This method assumes that request_info_ is already setup properly. - // Create a transaction. - DCHECK(!transaction_.get()); - - DCHECK(request_->context()); - DCHECK(request_->context()->http_transaction_factory()); - - // No matter what, we want to report our status as IO pending since we will - // be notifying our consumer asynchronously via OnStartCompleted. - SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0)); + // If we already have a transaction, then we should restart the transaction + // with auth provided by username_ and password_. - int rv = request_->context()->http_transaction_factory()->CreateTransaction( - &transaction_); - - if (rv == net::OK) { - rv = transaction_->Start( - &request_info_, &start_callback_, request_->load_log()); - if (rv == net::ERR_IO_PENDING) - return; + int rv; + if (transaction_.get()) { + rv = transaction_->RestartWithAuth(username_, password_, &start_callback_); + username_.clear(); + password_.clear(); + } else { + DCHECK(request_->context()); + DCHECK(request_->context()->http_transaction_factory()); + + rv = request_->context()->http_transaction_factory()->CreateTransaction( + &transaction_); + if (rv == net::OK) { + rv = transaction_->Start( + &request_info_, &start_callback_, request_->load_log()); + } } + if (rv == net::ERR_IO_PENDING) + return; + // The transaction started synchronously, but we need to notify the // URLRequest delegate via the message loop. MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod( @@ -648,8 +662,6 @@ void URLRequestHttpJob::AddExtraHeaders() { URLRequestContext* context = request_->context(); if (context) { - request_info_.extra_headers += AssembleRequestCookies(); - // Only add default Accept-Language and Accept-Charset if the request // didn't have them specified. net::HttpUtil::AppendHeaderIfMissing("Accept-Language", @@ -661,39 +673,87 @@ void URLRequestHttpJob::AddExtraHeaders() { } } -std::string URLRequestHttpJob::AssembleRequestCookies() { - if (request_info_.load_flags & net::LOAD_DO_NOT_SEND_COOKIES) - return std::string(); +void URLRequestHttpJob::AddCookieHeaderAndStart() { + // No matter what, we want to report our status as IO pending since we will + // be notifying our consumer asynchronously via OnStartCompleted. + SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0)); - URLRequestContext* context = request_->context(); - if (context) { - // Add in the cookie header. TODO might we need more than one header? - if (context->cookie_store() && (!context->cookie_policy() || - context->cookie_policy()->CanGetCookies( - request_->url(), request_->first_party_for_cookies()))) { - net::CookieOptions options; - options.set_include_httponly(); - std::string cookies = request_->context()->cookie_store()-> - GetCookiesWithOptions(request_->url(), options); - if (context->InterceptRequestCookies(request_, cookies) && - !cookies.empty()) - return "Cookie: " + cookies + "\r\n"; - } + AddRef(); // Balanced in OnCanGetCookiesCompleted + + int policy = net::OK; + + if (request_info_.load_flags & net::LOAD_DO_NOT_SEND_COOKIES) { + policy = net::ERR_ACCESS_DENIED; + } else if (request_->context()->cookie_policy()) { + policy = request_->context()->cookie_policy()->CanGetCookies( + request_->url(), + request_->first_party_for_cookies(), + &can_get_cookies_callback_); + if (policy == net::ERR_IO_PENDING) + return; // Wait for completion callback } - return std::string(); + + OnCanGetCookiesCompleted(policy); } -void URLRequestHttpJob::FetchResponseCookies() { - DCHECK(response_info_); - DCHECK(response_cookies_.empty()); +void URLRequestHttpJob::SaveCookiesAndNotifyHeadersComplete() { + DCHECK(transaction_.get()); + const net::HttpResponseInfo* response_info = transaction_->GetResponseInfo(); + DCHECK(response_info); + + response_cookies_.clear(); + response_cookies_save_index_ = 0; + + FetchResponseCookies(response_info, &response_cookies_); + + // Now, loop over the response cookies, and attempt to persist each. + SaveNextCookie(); +} + +void URLRequestHttpJob::SaveNextCookie() { + if (response_cookies_save_index_ == response_cookies_.size()) { + response_cookies_.clear(); + response_cookies_save_index_ = 0; + SetStatus(URLRequestStatus()); // Clear the IO_PENDING status + NotifyHeadersComplete(); + return; + } + + // No matter what, we want to report our status as IO pending since we will + // be notifying our consumer asynchronously via OnStartCompleted. + SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0)); + + AddRef(); // Balanced in OnCanSetCookieCompleted + + int policy = net::OK; + + if (request_info_.load_flags & net::LOAD_DO_NOT_SAVE_COOKIES) { + policy = net::ERR_ACCESS_DENIED; + } else if (request_->context()->cookie_policy()) { + policy = request_->context()->cookie_policy()->CanSetCookie( + request_->url(), + request_->first_party_for_cookies(), + response_cookies_[response_cookies_save_index_], + &can_set_cookie_callback_); + if (policy == net::ERR_IO_PENDING) + return; // Wait for completion callback + } + + OnCanSetCookieCompleted(policy); +} + +void URLRequestHttpJob::FetchResponseCookies( + const net::HttpResponseInfo* response_info, + std::vector<std::string>* cookies) { std::string name = "Set-Cookie"; std::string value; void* iter = NULL; - while (response_info_->headers->EnumerateHeader(&iter, name, &value)) + while (response_info->headers->EnumerateHeader(&iter, name, &value)) { if (request_->context()->InterceptResponseCookie(request_, value)) - response_cookies_.push_back(value); + cookies->push_back(value); + } } class HTTPSProberDelegate : public net::HTTPSProberDelegate { diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h index 383bae1..e00e3c4 100644 --- a/net/url_request/url_request_http_job.h +++ b/net/url_request/url_request_http_job.h @@ -62,12 +62,17 @@ class URLRequestHttpJob : public URLRequestJob { void DestroyTransaction(); void StartTransaction(); void AddExtraHeaders(); - std::string AssembleRequestCookies(); - void FetchResponseCookies(); + void AddCookieHeaderAndStart(); + void SaveCookiesAndNotifyHeadersComplete(); + void SaveNextCookie(); + void FetchResponseCookies(const net::HttpResponseInfo* response_info, + std::vector<std::string>* cookies); // Process the Strict-Transport-Security header, if one exists. void ProcessStrictTransportSecurityHeader(); + void OnCanGetCookiesCompleted(int result); + void OnCanSetCookieCompleted(int result); void OnStartCompleted(int result); void OnReadCompleted(int result); @@ -82,12 +87,19 @@ class URLRequestHttpJob : public URLRequestJob { net::HttpRequestInfo request_info_; const net::HttpResponseInfo* response_info_; + std::vector<std::string> response_cookies_; + size_t response_cookies_save_index_; // Auth states for proxy and origin server. net::AuthState proxy_auth_state_; net::AuthState server_auth_state_; + std::wstring username_; + std::wstring password_; + + net::CompletionCallbackImpl<URLRequestHttpJob> can_get_cookies_callback_; + net::CompletionCallbackImpl<URLRequestHttpJob> can_set_cookie_callback_; net::CompletionCallbackImpl<URLRequestHttpJob> start_callback_; net::CompletionCallbackImpl<URLRequestHttpJob> read_callback_; diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc index 48cfd35..56901fc 100644 --- a/net/url_request/url_request_unittest.cc +++ b/net/url_request/url_request_unittest.cc @@ -24,6 +24,7 @@ #include "base/string_piece.h" #include "base/string_util.h" #include "net/base/cookie_monster.h" +#include "net/base/cookie_policy.h" #include "net/base/load_flags.h" #include "net/base/load_log.h" #include "net/base/load_log_unittest.h" @@ -1286,7 +1287,6 @@ TEST_F(URLRequestTest, DoNotSaveCookies) { // Try to set-up another cookie and update the previous cookie. { - scoped_refptr<URLRequestContext> context = new URLRequestTestContext(); TestDelegate d; URLRequest req(server->TestServerPage( "set-cookie?CookieToNotSave=1&CookieToNotUpdate=1"), &d); @@ -1312,6 +1312,216 @@ TEST_F(URLRequestTest, DoNotSaveCookies) { } } +TEST_F(URLRequestTest, DoNotSendCookies_ViaPolicy) { + scoped_refptr<HTTPTestServer> server = + HTTPTestServer::CreateServer(L"", NULL); + ASSERT_TRUE(NULL != server.get()); + scoped_refptr<URLRequestTestContext> context = new URLRequestTestContext(); + + // Set up a cookie. + { + TestDelegate d; + URLRequest req(server->TestServerPage("set-cookie?CookieToNotSend=1"), &d); + req.set_context(context); + req.Start(); + MessageLoop::current()->Run(); + } + + // Verify that the cookie is set. + { + TestDelegate d; + TestURLRequest req(server->TestServerPage("echoheader?Cookie"), &d); + req.set_context(context); + req.Start(); + MessageLoop::current()->Run(); + + EXPECT_TRUE(d.data_received().find("CookieToNotSend=1") + != std::string::npos); + } + + // Verify that the cookie isn't sent. + { + TestCookiePolicy cookie_policy(TestCookiePolicy::NO_GET_COOKIES); + context->set_cookie_policy(&cookie_policy); + + TestDelegate d; + TestURLRequest req(server->TestServerPage("echoheader?Cookie"), &d); + req.set_context(context); + req.Start(); + MessageLoop::current()->Run(); + + EXPECT_TRUE(d.data_received().find("Cookie: CookieToNotSend=1") + == std::string::npos); + + context->set_cookie_policy(NULL); + } +} + +TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy) { + scoped_refptr<HTTPTestServer> server = + HTTPTestServer::CreateServer(L"", NULL); + ASSERT_TRUE(NULL != server.get()); + scoped_refptr<URLRequestTestContext> context = new URLRequestTestContext(); + + // Set up a cookie. + { + TestDelegate d; + URLRequest req(server->TestServerPage("set-cookie?CookieToNotUpdate=2"), + &d); + req.set_context(context); + req.Start(); + MessageLoop::current()->Run(); + } + + // Try to set-up another cookie and update the previous cookie. + { + TestCookiePolicy cookie_policy(TestCookiePolicy::NO_SET_COOKIE); + context->set_cookie_policy(&cookie_policy); + + TestDelegate d; + URLRequest req(server->TestServerPage( + "set-cookie?CookieToNotSave=1&CookieToNotUpdate=1"), &d); + req.set_context(context); + req.Start(); + + MessageLoop::current()->Run(); + + context->set_cookie_policy(NULL); + } + + + // Verify the cookies weren't saved or updated. + { + TestDelegate d; + TestURLRequest req(server->TestServerPage("echoheader?Cookie"), &d); + req.set_context(context); + req.Start(); + MessageLoop::current()->Run(); + + EXPECT_TRUE(d.data_received().find("CookieToNotSave=1") + == std::string::npos); + EXPECT_TRUE(d.data_received().find("CookieToNotUpdate=2") + != std::string::npos); + } +} + +TEST_F(URLRequestTest, DoNotSendCookies_ViaPolicy_Async) { + scoped_refptr<HTTPTestServer> server = + HTTPTestServer::CreateServer(L"", NULL); + ASSERT_TRUE(NULL != server.get()); + scoped_refptr<URLRequestTestContext> context = new URLRequestTestContext(); + + // Set up a cookie. + { + TestDelegate d; + URLRequest req(server->TestServerPage("set-cookie?CookieToNotSend=1"), &d); + req.set_context(context); + req.Start(); + MessageLoop::current()->Run(); + } + + // Verify that the cookie is set. + { + TestDelegate d; + TestURLRequest req(server->TestServerPage("echoheader?Cookie"), &d); + req.set_context(context); + req.Start(); + MessageLoop::current()->Run(); + + EXPECT_TRUE(d.data_received().find("CookieToNotSend=1") + != std::string::npos); + } + + // Verify that the cookie isn't sent. + { + TestCookiePolicy cookie_policy(TestCookiePolicy::NO_GET_COOKIES | + TestCookiePolicy::ASYNC); + context->set_cookie_policy(&cookie_policy); + + TestDelegate d; + TestURLRequest req(server->TestServerPage("echoheader?Cookie"), &d); + req.set_context(context); + req.Start(); + MessageLoop::current()->Run(); + + EXPECT_TRUE(d.data_received().find("Cookie: CookieToNotSend=1") + == std::string::npos); + + context->set_cookie_policy(NULL); + } +} + +TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy_Async) { + scoped_refptr<HTTPTestServer> server = + HTTPTestServer::CreateServer(L"", NULL); + ASSERT_TRUE(NULL != server.get()); + scoped_refptr<URLRequestTestContext> context = new URLRequestTestContext(); + + // Set up a cookie. + { + TestDelegate d; + URLRequest req(server->TestServerPage("set-cookie?CookieToNotUpdate=2"), + &d); + req.set_context(context); + req.Start(); + MessageLoop::current()->Run(); + } + + // Try to set-up another cookie and update the previous cookie. + { + TestCookiePolicy cookie_policy(TestCookiePolicy::NO_SET_COOKIE | + TestCookiePolicy::ASYNC); + context->set_cookie_policy(&cookie_policy); + + TestDelegate d; + URLRequest req(server->TestServerPage( + "set-cookie?CookieToNotSave=1&CookieToNotUpdate=1"), &d); + req.set_context(context); + req.Start(); + + MessageLoop::current()->Run(); + + context->set_cookie_policy(NULL); + } + + // Verify the cookies weren't saved or updated. + { + TestDelegate d; + TestURLRequest req(server->TestServerPage("echoheader?Cookie"), &d); + req.set_context(context); + req.Start(); + MessageLoop::current()->Run(); + + EXPECT_TRUE(d.data_received().find("CookieToNotSave=1") + == std::string::npos); + EXPECT_TRUE(d.data_received().find("CookieToNotUpdate=2") + != std::string::npos); + } +} + +TEST_F(URLRequestTest, CancelTest_DuringCookiePolicy) { + scoped_refptr<HTTPTestServer> server = + HTTPTestServer::CreateServer(L"", NULL); + ASSERT_TRUE(NULL != server.get()); + scoped_refptr<URLRequestTestContext> context = new URLRequestTestContext(); + + TestCookiePolicy cookie_policy(TestCookiePolicy::ASYNC); + context->set_cookie_policy(&cookie_policy); + + // Set up a cookie. + { + TestDelegate d; + URLRequest req(server->TestServerPage("set-cookie?A=1&B=2&C=3"), + &d); + req.set_context(context); + req.Start(); // Triggers an asynchronous cookie policy check. + + // But, now we cancel the request. This should not cause a crash. + } + + context->set_cookie_policy(NULL); +} + // In this test, we do a POST which the server will 302 redirect. // The subsequent transaction should use GET, and should not send the // Content-Type header. diff --git a/net/url_request/url_request_unittest.h b/net/url_request/url_request_unittest.h index abd94aa..e912ab9 100644 --- a/net/url_request/url_request_unittest.h +++ b/net/url_request/url_request_unittest.h @@ -22,6 +22,7 @@ #include "base/time.h" #include "base/waitable_event.h" #include "net/base/cookie_monster.h" +#include "net/base/cookie_policy.h" #include "net/base/host_resolver.h" #include "net/base/io_buffer.h" #include "net/base/net_errors.h" @@ -45,6 +46,83 @@ const std::string kDefaultHostName("localhost"); using base::TimeDelta; +//----------------------------------------------------------------------------- + +class TestCookiePolicy : public net::CookiePolicy { + public: + enum Options { + NO_GET_COOKIES = 1 << 0, + NO_SET_COOKIE = 1 << 1, + ASYNC = 1 << 2 + }; + + explicit TestCookiePolicy(int options_bit_mask) + : ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)), + options_(options_bit_mask), + callback_(NULL) { + } + + virtual int CanGetCookies(const GURL& url, const GURL& first_party, + net::CompletionCallback* callback) { + if ((options_ & ASYNC) && callback) { + callback_ = callback; + MessageLoop::current()->PostTask(FROM_HERE, + method_factory_.NewRunnableMethod( + &TestCookiePolicy::DoGetCookiesPolicy, url, first_party)); + return net::ERR_IO_PENDING; + } + + if (options_ & NO_GET_COOKIES) + return net::ERR_ACCESS_DENIED; + + return net::OK; + } + + virtual int CanSetCookie(const GURL& url, const GURL& first_party, + const std::string& cookie_line, + net::CompletionCallback* callback) { + if ((options_ & ASYNC) && callback) { + callback_ = callback; + MessageLoop::current()->PostTask(FROM_HERE, + method_factory_.NewRunnableMethod( + &TestCookiePolicy::DoSetCookiePolicy, url, first_party, + cookie_line)); + return net::ERR_IO_PENDING; + } + + if (options_ & NO_SET_COOKIE) + return net::ERR_ACCESS_DENIED; + + return net::OK; + } + + private: + void DoGetCookiesPolicy(const GURL& url, const GURL& first_party) { + int policy = CanGetCookies(url, first_party, NULL); + + DCHECK(callback_); + net::CompletionCallback* callback = callback_; + callback_ = NULL; + callback->Run(policy); + } + + void DoSetCookiePolicy(const GURL& url, const GURL& first_party, + const std::string& cookie_line) { + int policy = CanSetCookie(url, first_party, cookie_line, NULL); + + DCHECK(callback_); + net::CompletionCallback* callback = callback_; + callback_ = NULL; + callback->Run(policy); + } + + ScopedRunnableMethodFactory<TestCookiePolicy> method_factory_; + int options_; + net::CompletionCallback* callback_; +}; + +//----------------------------------------------------------------------------- + class TestURLRequestContext : public URLRequestContext { public: TestURLRequestContext() { @@ -61,6 +139,10 @@ class TestURLRequestContext : public URLRequestContext { Init(); } + void set_cookie_policy(net::CookiePolicy* policy) { + cookie_policy_ = policy; + } + protected: virtual ~TestURLRequestContext() { delete ftp_transaction_factory_; @@ -86,6 +168,8 @@ class TestURLRequestContext : public URLRequestContext { // TODO(phajdan.jr): Migrate callers to the new name and remove the typedef. typedef TestURLRequestContext URLRequestTestContext; +//----------------------------------------------------------------------------- + class TestURLRequest : public URLRequest { public: TestURLRequest(const GURL& url, Delegate* delegate) @@ -94,6 +178,8 @@ class TestURLRequest : public URLRequest { } }; +//----------------------------------------------------------------------------- + class TestDelegate : public URLRequest::Delegate { public: TestDelegate() @@ -261,6 +347,8 @@ class TestDelegate : public URLRequest::Delegate { scoped_refptr<net::IOBuffer> buf_; }; +//----------------------------------------------------------------------------- + // This object bounds the lifetime of an external python-based HTTP/FTP server // that can provide various responses useful for testing. class BaseTestServer : public base::RefCounted<BaseTestServer> { @@ -372,6 +460,7 @@ class BaseTestServer : public base::RefCounted<BaseTestServer> { std::string port_str_; }; +//----------------------------------------------------------------------------- // HTTP class HTTPTestServer : public BaseTestServer { @@ -522,6 +611,8 @@ class HTTPTestServer : public BaseTestServer { MessageLoop* loop_; }; +//----------------------------------------------------------------------------- + class HTTPSTestServer : public HTTPTestServer { protected: explicit HTTPSTestServer() { @@ -598,6 +689,7 @@ class HTTPSTestServer : public HTTPTestServer { virtual ~HTTPSTestServer() {} }; +//----------------------------------------------------------------------------- class FTPTestServer : public BaseTestServer { public: diff --git a/webkit/glue/webcookie.h b/webkit/glue/webcookie.h index 4966c442..2feb800 100644 --- a/webkit/glue/webcookie.h +++ b/webkit/glue/webcookie.h @@ -9,7 +9,7 @@ #ifndef WEBKIT_GLUE_WEBCOOKIE_H_ #define WEBKIT_GLUE_WEBCOOKIE_H_ -#include <string> +#include "net/base/cookie_monster.h" namespace webkit_glue { @@ -18,22 +18,34 @@ struct WebCookie { WebCookie(const std::string& name, const std::string& value, const std::string& domain, const std::string& path, double expires, bool http_only, bool secure, bool session) - : name(name), - value(value), - domain(domain), - path(path), - expires(expires), - http_only(http_only), - secure(secure), - session(session) { + : name(name), + value(value), + domain(domain), + path(path), + expires(expires), + http_only(http_only), + secure(secure), + session(session) { + } + + WebCookie(const std::string& domain, + const net::CookieMonster::CanonicalCookie& c) + : name(c.Name()), + value(c.Value()), + domain(domain), + path(c.Path()), + expires(c.ExpiryDate().ToDoubleT() * 1000), + http_only(c.IsHttpOnly()), + secure(c.IsSecure()), + session(!c.IsPersistent()) { } // For default constructions. - WebCookie() : - expires(0), - http_only(false), - secure(false), - session(false) { + WebCookie() + : expires(0), + http_only(false), + secure(false), + session(false) { } // Cookie name. |