diff options
author | pauljensen@chromium.org <pauljensen@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-05-08 16:04:34 +0000 |
---|---|---|
committer | pauljensen@chromium.org <pauljensen@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-05-08 16:04:34 +0000 |
commit | e0f35c91db833cf91df215ad3baf1471f6d5f962 (patch) | |
tree | a552beed80f52a83d58248873977f79916f9ef7b /net | |
parent | 5be833caea8b86647772bc9829c4dacc1fc3d7b9 (diff) | |
download | chromium_src-e0f35c91db833cf91df215ad3baf1471f6d5f962.zip chromium_src-e0f35c91db833cf91df215ad3baf1471f6d5f962.tar.gz chromium_src-e0f35c91db833cf91df215ad3baf1471f6d5f962.tar.bz2 |
Removed static factories for data, ftp, file, and about jobs.
Instead add corresponding ProtocolHandlers as needed.
Remove URLRequestContext members used by these static
factories. Bake FtpAuthCache into FtpProtocolHandler as it
was already unique per FtpProtocolHandler.
This is a revived version of http://crrev.com/10836206
BUG=142945
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188912
Review URL: https://chromiumcodereview.appspot.com/11931024
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@198915 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
31 files changed, 199 insertions, 199 deletions
diff --git a/net/ftp/ftp_network_session.h b/net/ftp/ftp_network_session.h index c9b3388..e92bb06 100644 --- a/net/ftp/ftp_network_session.h +++ b/net/ftp/ftp_network_session.h @@ -7,7 +7,6 @@ #include "base/memory/ref_counted.h" #include "net/base/net_export.h" -#include "net/ftp/ftp_auth_cache.h" namespace net { @@ -20,7 +19,6 @@ class NET_EXPORT_PRIVATE FtpNetworkSession explicit FtpNetworkSession(HostResolver* host_resolver); HostResolver* host_resolver() { return host_resolver_; } - FtpAuthCache* auth_cache() { return &auth_cache_; } private: friend class base::RefCounted<FtpNetworkSession>; @@ -28,7 +26,6 @@ class NET_EXPORT_PRIVATE FtpNetworkSession virtual ~FtpNetworkSession(); HostResolver* const host_resolver_; - FtpAuthCache auth_cache_; }; } // namespace net diff --git a/net/proxy/proxy_script_fetcher_impl_unittest.cc b/net/proxy/proxy_script_fetcher_impl_unittest.cc index ab04aba..d58c5bc 100644 --- a/net/proxy/proxy_script_fetcher_impl_unittest.cc +++ b/net/proxy/proxy_script_fetcher_impl_unittest.cc @@ -21,6 +21,7 @@ #include "net/http/http_server_properties_impl.h" #include "net/ssl/ssl_config_service_defaults.h" #include "net/test/spawned_test_server/spawned_test_server.h" +#include "net/url_request/file_protocol_handler.h" #include "net/url_request/url_request_context_storage.h" #include "net/url_request/url_request_file_job.h" #include "net/url_request/url_request_job_factory_impl.h" @@ -66,7 +67,9 @@ class RequestContext : public URLRequestContext { storage_.set_http_transaction_factory(new HttpCache( network_session, HttpCache::DefaultBackend::InMemory(0))); - storage_.set_job_factory(new URLRequestJobFactoryImpl()); + URLRequestJobFactoryImpl* job_factory = new URLRequestJobFactoryImpl(); + job_factory->SetProtocolHandler("file", new FileProtocolHandler()); + storage_.set_job_factory(job_factory); } virtual ~RequestContext() { diff --git a/net/url_request/file_protocol_handler.cc b/net/url_request/file_protocol_handler.cc index 0483a99..dc5b16f 100644 --- a/net/url_request/file_protocol_handler.cc +++ b/net/url_request/file_protocol_handler.cc @@ -44,4 +44,8 @@ URLRequestJob* FileProtocolHandler::MaybeCreateJob( return new URLRequestFileJob(request, network_delegate, file_path); } +bool FileProtocolHandler::IsSafeRedirectTarget(const GURL& location) const { + return false; +} + } // namespace net diff --git a/net/url_request/file_protocol_handler.h b/net/url_request/file_protocol_handler.h index 619227b..8087a6e 100644 --- a/net/url_request/file_protocol_handler.h +++ b/net/url_request/file_protocol_handler.h @@ -9,6 +9,8 @@ #include "base/compiler_specific.h" #include "net/url_request/url_request_job_factory.h" +class GURL; + namespace net { class NetworkDelegate; @@ -22,6 +24,7 @@ class NET_EXPORT FileProtocolHandler : FileProtocolHandler(); virtual URLRequestJob* MaybeCreateJob( URLRequest* request, NetworkDelegate* network_delegate) const OVERRIDE; + virtual bool IsSafeRedirectTarget(const GURL& location) const OVERRIDE; private: DISALLOW_COPY_AND_ASSIGN(FileProtocolHandler); diff --git a/net/url_request/ftp_protocol_handler.cc b/net/url_request/ftp_protocol_handler.cc index 7d9ba88..d873295 100644 --- a/net/url_request/ftp_protocol_handler.cc +++ b/net/url_request/ftp_protocol_handler.cc @@ -7,6 +7,7 @@ #include "base/logging.h" #include "net/base/net_errors.h" #include "net/base/net_util.h" +#include "net/ftp/ftp_auth_cache.h" #include "net/url_request/url_request.h" #include "net/url_request/url_request_error_job.h" #include "net/url_request/url_request_ftp_job.h" @@ -15,12 +16,13 @@ namespace net { FtpProtocolHandler::FtpProtocolHandler( - FtpTransactionFactory* ftp_transaction_factory, - FtpAuthCache* ftp_auth_cache) + FtpTransactionFactory* ftp_transaction_factory) : ftp_transaction_factory_(ftp_transaction_factory), - ftp_auth_cache_(ftp_auth_cache) { + ftp_auth_cache_(new FtpAuthCache) { DCHECK(ftp_transaction_factory_); - DCHECK(ftp_auth_cache_); +} + +FtpProtocolHandler::~FtpProtocolHandler() { } URLRequestJob* FtpProtocolHandler::MaybeCreateJob( @@ -34,7 +36,7 @@ URLRequestJob* FtpProtocolHandler::MaybeCreateJob( return new URLRequestFtpJob(request, network_delegate, ftp_transaction_factory_, - ftp_auth_cache_); + ftp_auth_cache_.get()); } } // namespace net diff --git a/net/url_request/ftp_protocol_handler.h b/net/url_request/ftp_protocol_handler.h index 871f422..7c2278a 100644 --- a/net/url_request/ftp_protocol_handler.h +++ b/net/url_request/ftp_protocol_handler.h @@ -7,6 +7,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" +#include "base/memory/scoped_ptr.h" #include "net/url_request/url_request_job_factory.h" namespace net { @@ -20,14 +21,16 @@ class URLRequestJob; class NET_EXPORT FtpProtocolHandler : public URLRequestJobFactory::ProtocolHandler { public: - FtpProtocolHandler(FtpTransactionFactory* ftp_transaction_factory, - FtpAuthCache* ftp_auth_cache); + explicit FtpProtocolHandler(FtpTransactionFactory* ftp_transaction_factory); + virtual ~FtpProtocolHandler(); virtual URLRequestJob* MaybeCreateJob( URLRequest* request, NetworkDelegate* network_delegate) const OVERRIDE; private: + friend class FtpTestURLRequestContext; + FtpTransactionFactory* ftp_transaction_factory_; - FtpAuthCache* ftp_auth_cache_; + scoped_ptr<FtpAuthCache> ftp_auth_cache_; DISALLOW_COPY_AND_ASSIGN(FtpProtocolHandler); }; diff --git a/net/url_request/protocol_intercept_job_factory.cc b/net/url_request/protocol_intercept_job_factory.cc index f72fbf9..d0f92f4 100644 --- a/net/url_request/protocol_intercept_job_factory.cc +++ b/net/url_request/protocol_intercept_job_factory.cc @@ -39,4 +39,9 @@ bool ProtocolInterceptJobFactory::IsHandledURL(const GURL& url) const { return job_factory_->IsHandledURL(url); } +bool ProtocolInterceptJobFactory::IsSafeRedirectTarget( + const GURL& location) const { + return job_factory_->IsSafeRedirectTarget(location); +} + } // namespace net diff --git a/net/url_request/protocol_intercept_job_factory.h b/net/url_request/protocol_intercept_job_factory.h index 7deeb30..abc2fb8 100644 --- a/net/url_request/protocol_intercept_job_factory.h +++ b/net/url_request/protocol_intercept_job_factory.h @@ -22,6 +22,8 @@ class URLRequestJob; // given the option of creating a URLRequestJob for each potential URLRequest. // If |protocol_handler_| does not create a job (i.e. MaybeCreateJob() returns // NULL) the URLRequest is forwarded to the |job_factory_| to be handled there. +// Only the MaybeCreateJob() member of |protocol_handler_| is called; the +// IsSafeRedirectTarget() member is not used. class NET_EXPORT ProtocolInterceptJobFactory : public URLRequestJobFactory { public: ProtocolInterceptJobFactory(scoped_ptr<URLRequestJobFactory> job_factory, @@ -35,6 +37,7 @@ class NET_EXPORT ProtocolInterceptJobFactory : public URLRequestJobFactory { NetworkDelegate* network_delegate) const OVERRIDE; virtual bool IsHandledProtocol(const std::string& scheme) const OVERRIDE; virtual bool IsHandledURL(const GURL& url) const OVERRIDE; + virtual bool IsSafeRedirectTarget(const GURL& location) const OVERRIDE; private: scoped_ptr<URLRequestJobFactory> job_factory_; diff --git a/net/url_request/url_request_about_job.cc b/net/url_request/url_request_about_job.cc index d234a51..7c7251d 100644 --- a/net/url_request/url_request_about_job.cc +++ b/net/url_request/url_request_about_job.cc @@ -20,13 +20,6 @@ URLRequestAboutJob::URLRequestAboutJob(URLRequest* request, weak_factory_(this) { } -// static -URLRequestJob* URLRequestAboutJob::Factory(URLRequest* request, - NetworkDelegate* network_delegate, - const std::string& scheme) { - return new URLRequestAboutJob(request, network_delegate); -} - void URLRequestAboutJob::Start() { // Start reading asynchronously so that all error reporting and data // callbacks happen as they would for network requests. diff --git a/net/url_request/url_request_about_job.h b/net/url_request/url_request_about_job.h index ea2b83f..66a888a 100644 --- a/net/url_request/url_request_about_job.h +++ b/net/url_request/url_request_about_job.h @@ -17,8 +17,6 @@ class NET_EXPORT URLRequestAboutJob : public URLRequestJob { public: URLRequestAboutJob(URLRequest* request, NetworkDelegate* network_delegate); - static URLRequest::ProtocolFactory Factory; - // URLRequestJob: virtual void Start() OVERRIDE; virtual bool GetMimeType(std::string* mime_type) const OVERRIDE; diff --git a/net/url_request/url_request_context.cc b/net/url_request/url_request_context.cc index d69d3e1..318d5f9 100644 --- a/net/url_request/url_request_context.cc +++ b/net/url_request/url_request_context.cc @@ -10,7 +10,6 @@ #include "base/string_util.h" #include "net/cookies/cookie_store.h" #include "net/dns/host_resolver.h" -#include "net/ftp/ftp_transaction_factory.h" #include "net/http/http_transaction_factory.h" #include "net/url_request/http_user_agent_settings.h" #include "net/url_request/url_request.h" @@ -29,11 +28,7 @@ URLRequestContext::URLRequestContext() http_server_properties_(NULL), http_user_agent_settings_(NULL), transport_security_state_(NULL), -#if !defined(DISABLE_FTP_SUPPORT) - ftp_auth_cache_(new FtpAuthCache), -#endif http_transaction_factory_(NULL), - ftp_transaction_factory_(NULL), job_factory_(NULL), throttler_manager_(NULL), url_requests_(new std::set<const URLRequest*>) { @@ -57,9 +52,7 @@ void URLRequestContext::CopyFrom(const URLRequestContext* other) { set_http_server_properties(other->http_server_properties_); set_cookie_store(other->cookie_store_); set_transport_security_state(other->transport_security_state_); - // FTPAuthCache is unique per context. set_http_transaction_factory(other->http_transaction_factory_); - set_ftp_transaction_factory(other->ftp_transaction_factory_); set_job_factory(other->job_factory_); set_throttler_manager(other->throttler_manager_); set_http_user_agent_settings(other->http_user_agent_settings_); diff --git a/net/url_request/url_request_context.h b/net/url_request/url_request_context.h index f5dc635..bb26897 100644 --- a/net/url_request/url_request_context.h +++ b/net/url_request/url_request_context.h @@ -19,7 +19,6 @@ #include "base/threading/non_thread_safe.h" #include "net/base/net_export.h" #include "net/base/net_log.h" -#include "net/ftp/ftp_auth_cache.h" #include "net/http/http_network_session.h" #include "net/http/http_server_properties.h" #include "net/http/transport_security_state.h" @@ -30,7 +29,6 @@ namespace net { class CertVerifier; class CookieStore; class FraudulentCertificateReporter; -class FtpTransactionFactory; class HostResolver; class HttpAuthHandlerFactory; class HttpTransactionFactory; @@ -131,14 +129,6 @@ class NET_EXPORT URLRequestContext http_transaction_factory_ = factory; } - // Gets the ftp transaction factory for this context. - FtpTransactionFactory* ftp_transaction_factory() const { - return ftp_transaction_factory_; - } - void set_ftp_transaction_factory(FtpTransactionFactory* factory) { - ftp_transaction_factory_ = factory; - } - void set_network_delegate(NetworkDelegate* network_delegate) { network_delegate_ = network_delegate; } @@ -165,15 +155,6 @@ class NET_EXPORT URLRequestContext transport_security_state_ = state; } - // Gets the FTP authentication cache for this context. - FtpAuthCache* ftp_auth_cache() const { -#if !defined(DISABLE_FTP_SUPPORT) - return ftp_auth_cache_.get(); -#else - return NULL; -#endif - } - // --------------------------------------------------------------------------- // Legacy accessors that delegate to http_user_agent_settings_. // TODO(pauljensen): Remove after all clients are updated to directly access @@ -237,11 +218,7 @@ class NET_EXPORT URLRequestContext HttpUserAgentSettings* http_user_agent_settings_; scoped_refptr<CookieStore> cookie_store_; TransportSecurityState* transport_security_state_; -#if !defined(DISABLE_FTP_SUPPORT) - scoped_ptr<FtpAuthCache> ftp_auth_cache_; -#endif HttpTransactionFactory* http_transaction_factory_; - FtpTransactionFactory* ftp_transaction_factory_; const URLRequestJobFactory* job_factory_; URLRequestThrottlerManager* throttler_manager_; diff --git a/net/url_request/url_request_context_builder.cc b/net/url_request/url_request_context_builder.cc index f391ebf..893ada6 100644 --- a/net/url_request/url_request_context_builder.cc +++ b/net/url_request/url_request_context_builder.cc @@ -26,9 +26,13 @@ #include "net/http/transport_security_state.h" #include "net/proxy/proxy_service.h" #include "net/ssl/ssl_config_service_defaults.h" +#include "net/url_request/data_protocol_handler.h" +#include "net/url_request/file_protocol_handler.h" +#include "net/url_request/ftp_protocol_handler.h" #include "net/url_request/static_http_user_agent_settings.h" #include "net/url_request/url_request_context.h" #include "net/url_request/url_request_context_storage.h" +#include "net/url_request/url_request_job_factory_impl.h" namespace net { @@ -180,7 +184,11 @@ URLRequestContextBuilder::HttpNetworkSessionParams::~HttpNetworkSessionParams() {} URLRequestContextBuilder::URLRequestContextBuilder() - : ftp_enabled_(false), + : data_enabled_(false), + file_enabled_(false), +#if !defined(DISABLE_FTP_SUPPORT) + ftp_enabled_(false), +#endif http_cache_enabled_(true) {} URLRequestContextBuilder::~URLRequestContextBuilder() {} @@ -205,11 +213,6 @@ URLRequestContext* URLRequestContextBuilder::Build() { storage->set_host_resolver(net::HostResolver::CreateDefaultResolver(NULL)); - if (ftp_enabled_) { - storage->set_ftp_transaction_factory( - new FtpNetworkLayer(context->host_resolver())); - } - context->StartFileThread(); // TODO(willchan): Switch to using this code when @@ -290,6 +293,21 @@ URLRequestContext* URLRequestContextBuilder::Build() { } storage->set_http_transaction_factory(http_transaction_factory); + URLRequestJobFactoryImpl* job_factory = new URLRequestJobFactoryImpl; + if (data_enabled_) + job_factory->SetProtocolHandler("data", new DataProtocolHandler); + if (file_enabled_) + job_factory->SetProtocolHandler("file", new FileProtocolHandler); +#if !defined(DISABLE_FTP_SUPPORT) + if (ftp_enabled_) { + ftp_transaction_factory_.reset( + new FtpNetworkLayer(context->host_resolver())); + job_factory->SetProtocolHandler("ftp", + new FtpProtocolHandler(ftp_transaction_factory_.get())); + } +#endif + storage->set_job_factory(job_factory); + // TODO(willchan): Support sdch. return context; diff --git a/net/url_request/url_request_context_builder.h b/net/url_request/url_request_context_builder.h index 2163ed5..20fc30e 100644 --- a/net/url_request/url_request_context_builder.h +++ b/net/url_request/url_request_context_builder.h @@ -25,6 +25,7 @@ namespace net { +class FtpTransactionFactory; class HostMappingRules; class ProxyConfigService; class URLRequestContext; @@ -82,10 +83,22 @@ class NET_EXPORT URLRequestContextBuilder { user_agent_ = user_agent; } - // By default it's disabled. + // Control support for data:// requests. By default it's disabled. + void set_data_enabled(bool enable) { + data_enabled_ = enable; + } + + // Control support for file:// requests. By default it's disabled. + void set_file_enabled(bool enable) { + file_enabled_ = enable; + } + +#if !defined(DISABLE_FTP_SUPPORT) + // Control support for ftp:// requests. By default it's disabled. void set_ftp_enabled(bool enable) { ftp_enabled_ = enable; } +#endif // Uses BasicNetworkDelegate by default. Note that calling Build will unset // any custom delegate in builder, so this must be called each time before @@ -114,7 +127,14 @@ class NET_EXPORT URLRequestContextBuilder { private: std::string accept_language_; std::string user_agent_; + // Include support for data:// requests. + bool data_enabled_; + // Include support for file:// requests. + bool file_enabled_; +#if !defined(DISABLE_FTP_SUPPORT) + // Include support for ftp:// requests. bool ftp_enabled_; +#endif bool http_cache_enabled_; HttpCacheParams http_cache_params_; HttpNetworkSessionParams http_network_session_params_; @@ -122,6 +142,7 @@ class NET_EXPORT URLRequestContextBuilder { scoped_ptr<ProxyConfigService> proxy_config_service_; #endif // defined(OS_LINUX) || defined(OS_ANDROID) scoped_ptr<NetworkDelegate> network_delegate_; + scoped_ptr<FtpTransactionFactory> ftp_transaction_factory_; DISALLOW_COPY_AND_ASSIGN(URLRequestContextBuilder); }; diff --git a/net/url_request/url_request_context_storage.cc b/net/url_request/url_request_context_storage.cc index 30fefd5..e3d84b47 100644 --- a/net/url_request/url_request_context_storage.cc +++ b/net/url_request/url_request_context_storage.cc @@ -106,12 +106,6 @@ void URLRequestContextStorage::set_http_transaction_factory( http_transaction_factory_.reset(http_transaction_factory); } -void URLRequestContextStorage::set_ftp_transaction_factory( - FtpTransactionFactory* ftp_transaction_factory) { - context_->set_ftp_transaction_factory(ftp_transaction_factory); - ftp_transaction_factory_.reset(ftp_transaction_factory); -} - void URLRequestContextStorage::set_job_factory( URLRequestJobFactory* job_factory) { context_->set_job_factory(job_factory); diff --git a/net/url_request/url_request_context_storage.h b/net/url_request/url_request_context_storage.h index e08b3c8..0a9d066 100644 --- a/net/url_request/url_request_context_storage.h +++ b/net/url_request/url_request_context_storage.h @@ -62,8 +62,6 @@ class NET_EXPORT URLRequestContextStorage { TransportSecurityState* transport_security_state); void set_http_transaction_factory( HttpTransactionFactory* http_transaction_factory); - void set_ftp_transaction_factory( - FtpTransactionFactory* ftp_transaction_factory); void set_job_factory(URLRequestJobFactory* job_factory); void set_throttler_manager(URLRequestThrottlerManager* throttler_manager); void set_http_user_agent_settings( @@ -93,7 +91,6 @@ class NET_EXPORT URLRequestContextStorage { scoped_ptr<TransportSecurityState> transport_security_state_; scoped_ptr<HttpTransactionFactory> http_transaction_factory_; - scoped_ptr<FtpTransactionFactory> ftp_transaction_factory_; scoped_ptr<URLRequestJobFactory> job_factory_; scoped_ptr<URLRequestThrottlerManager> throttler_manager_; diff --git a/net/url_request/url_request_data_job.cc b/net/url_request/url_request_data_job.cc index 5dffe32..fd248c7 100644 --- a/net/url_request/url_request_data_job.cc +++ b/net/url_request/url_request_data_job.cc @@ -16,13 +16,6 @@ URLRequestDataJob::URLRequestDataJob( : URLRequestSimpleJob(request, network_delegate) { } -// static -URLRequestJob* URLRequestDataJob::Factory(URLRequest* request, - NetworkDelegate* network_delegate, - const std::string& scheme) { - return new URLRequestDataJob(request, network_delegate); -} - int URLRequestDataJob::GetData(std::string* mime_type, std::string* charset, std::string* data, diff --git a/net/url_request/url_request_data_job.h b/net/url_request/url_request_data_job.h index 7ab6561..d9b2621 100644 --- a/net/url_request/url_request_data_job.h +++ b/net/url_request/url_request_data_job.h @@ -18,8 +18,6 @@ class URLRequestDataJob : public URLRequestSimpleJob { public: URLRequestDataJob(URLRequest* request, NetworkDelegate* network_delegate); - static URLRequest::ProtocolFactory Factory; - // URLRequestSimpleJob virtual int GetData(std::string* mime_type, std::string* charset, diff --git a/net/url_request/url_request_file_job.cc b/net/url_request/url_request_file_job.cc index 979759c..29cc28d 100644 --- a/net/url_request/url_request_file_job.cc +++ b/net/url_request/url_request_file_job.cc @@ -62,34 +62,6 @@ URLRequestFileJob::URLRequestFileJob(URLRequest* request, weak_ptr_factory_(this) { } -// static -URLRequestJob* URLRequestFileJob::Factory(URLRequest* request, - NetworkDelegate* network_delegate, - const std::string& scheme) { - base::FilePath file_path; - const bool is_file = FileURLToFilePath(request->url(), &file_path); - - // Check file access permissions. - if (!network_delegate || - !network_delegate->CanAccessFile(*request, file_path)) { - return new URLRequestErrorJob(request, network_delegate, ERR_ACCESS_DENIED); - } - // We need to decide whether to create URLRequestFileJob for file access or - // URLRequestFileDirJob for directory access. To avoid accessing the - // filesystem, we only look at the path string here. - // The code in the URLRequestFileJob::Start() method discovers that a path, - // which doesn't end with a slash, should really be treated as a directory, - // and it then redirects to the URLRequestFileDirJob. - if (is_file && - file_path.EndsWithSeparator() && - file_path.IsAbsolute()) - return new URLRequestFileDirJob(request, network_delegate, file_path); - - // Use a regular file request job for all non-directories (including invalid - // file names). - return new URLRequestFileJob(request, network_delegate, file_path); -} - void URLRequestFileJob::Start() { FileMetaInfo* meta_info = new FileMetaInfo(); base::WorkerPool::PostTaskAndReply( diff --git a/net/url_request/url_request_file_job.h b/net/url_request/url_request_file_job.h index 0d9e192..6fc7fb9 100644 --- a/net/url_request/url_request_file_job.h +++ b/net/url_request/url_request_file_job.h @@ -33,8 +33,6 @@ class NET_EXPORT URLRequestFileJob : public URLRequestJob { NetworkDelegate* network_delegate, const base::FilePath& file_path); - static URLRequest::ProtocolFactory Factory; - // URLRequestJob: virtual void Start() OVERRIDE; virtual void Kill() OVERRIDE; diff --git a/net/url_request/url_request_ftp_job.cc b/net/url_request/url_request_ftp_job.cc index 044310e..ac35f7e 100644 --- a/net/url_request/url_request_ftp_job.cc +++ b/net/url_request/url_request_ftp_job.cc @@ -12,6 +12,7 @@ #include "net/base/load_flags.h" #include "net/base/net_errors.h" #include "net/base/net_util.h" +#include "net/ftp/ftp_auth_cache.h" #include "net/ftp/ftp_response_info.h" #include "net/ftp/ftp_transaction_factory.h" #include "net/http/http_response_headers.h" @@ -46,26 +47,6 @@ URLRequestFtpJob::~URLRequestFtpJob() { proxy_service_->CancelPacRequest(pac_request_); } -// static -URLRequestJob* URLRequestFtpJob::Factory(URLRequest* request, - NetworkDelegate* network_delegate, - const std::string& scheme) { - DCHECK_EQ(scheme, "ftp"); - - int port = request->url().IntPort(); - if (request->url().has_port() && - !IsPortAllowedByFtp(port) && !IsPortAllowedByOverride(port)) { - return new URLRequestErrorJob(request, - network_delegate, - ERR_UNSAFE_PORT); - } - - return new URLRequestFtpJob(request, - network_delegate, - request->context()->ftp_transaction_factory(), - request->context()->ftp_auth_cache()); -} - bool URLRequestFtpJob::IsSafeRedirect(const GURL& location) { // Disallow all redirects. return false; diff --git a/net/url_request/url_request_ftp_job.h b/net/url_request/url_request_ftp_job.h index b1f61ad..4a064a01 100644 --- a/net/url_request/url_request_ftp_job.h +++ b/net/url_request/url_request_ftp_job.h @@ -33,11 +33,6 @@ class NET_EXPORT_PRIVATE URLRequestFtpJob : public URLRequestJob { FtpTransactionFactory* ftp_transaction_factory, FtpAuthCache* ftp_auth_cache); - // TODO(shalev): get rid of this function in favor of FtpProtocolHandler. - static URLRequestJob* Factory(URLRequest* request, - NetworkDelegate* network_delegate, - const std::string& scheme); - protected: virtual ~URLRequestFtpJob(); diff --git a/net/url_request/url_request_ftp_job_unittest.cc b/net/url_request/url_request_ftp_job_unittest.cc index 040f896..189451d 100644 --- a/net/url_request/url_request_ftp_job_unittest.cc +++ b/net/url_request/url_request_ftp_job_unittest.cc @@ -8,19 +8,47 @@ #include "base/memory/scoped_vector.h" #include "base/run_loop.h" #include "googleurl/src/gurl.h" +#include "net/ftp/ftp_auth_cache.h" #include "net/http/http_transaction_unittest.h" #include "net/proxy/mock_proxy_resolver.h" #include "net/proxy/proxy_config_service.h" #include "net/proxy/proxy_config_service_fixed.h" #include "net/socket/socket_test_util.h" +#include "net/url_request/ftp_protocol_handler.h" #include "net/url_request/url_request.h" #include "net/url_request/url_request_context.h" +#include "net/url_request/url_request_job_factory_impl.h" #include "net/url_request/url_request_status.h" #include "net/url_request/url_request_test_util.h" #include "testing/gtest/include/gtest/gtest.h" namespace net { +class FtpTestURLRequestContext : public TestURLRequestContext { + public: + FtpTestURLRequestContext(ClientSocketFactory* socket_factory, + ProxyService* proxy_service, + NetworkDelegate* network_delegate, + FtpTransactionFactory* ftp_transaction_factory) + : TestURLRequestContext(true), + ftp_protocol_handler_(new FtpProtocolHandler(ftp_transaction_factory)) { + set_client_socket_factory(socket_factory); + context_storage_.set_proxy_service(proxy_service); + set_network_delegate(network_delegate); + URLRequestJobFactoryImpl* job_factory = new URLRequestJobFactoryImpl; + job_factory->SetProtocolHandler("ftp", ftp_protocol_handler_); + context_storage_.set_job_factory(job_factory); + Init(); + } + + FtpAuthCache* GetFtpAuthCache() { + return ftp_protocol_handler_->ftp_auth_cache_.get(); + } + + private: + FtpProtocolHandler* ftp_protocol_handler_; +}; + namespace { class SimpleProxyConfigService : public ProxyConfigService { @@ -60,10 +88,10 @@ class SimpleProxyConfigService : public ProxyConfigService { // other hidden functions. class TestURLRequestFtpJob : public URLRequestFtpJob { public: - explicit TestURLRequestFtpJob(URLRequest* request) - : URLRequestFtpJob(request, NULL, - request->context()->ftp_transaction_factory(), - request->context()->ftp_auth_cache()) {} + TestURLRequestFtpJob(URLRequest* request, + FtpTransactionFactory* ftp_factory, + FtpAuthCache* ftp_auth_cache) + : URLRequestFtpJob(request, NULL, ftp_factory, ftp_auth_cache) {} using URLRequestFtpJob::SetPriority; using URLRequestFtpJob::Start; @@ -74,6 +102,15 @@ class TestURLRequestFtpJob : public URLRequestFtpJob { virtual ~TestURLRequestFtpJob() {} }; +class MockFtpTransactionFactory : public FtpTransactionFactory { + public: + virtual FtpTransaction* CreateTransaction() OVERRIDE { + return NULL; + } + + virtual void Suspend(bool suspend) OVERRIDE {} +}; + // Fixture for priority-related tests. Priority matters when there is // an HTTP proxy. class URLRequestFtpJobPriorityTest : public testing::Test { @@ -87,6 +124,8 @@ class URLRequestFtpJobPriorityTest : public testing::Test { ProxyService proxy_service_; MockNetworkLayer network_layer_; + MockFtpTransactionFactory ftp_factory_; + FtpAuthCache ftp_auth_cache_; TestURLRequestContext context_; TestDelegate delegate_; TestURLRequest req_; @@ -95,7 +134,8 @@ class URLRequestFtpJobPriorityTest : public testing::Test { // Make sure that SetPriority actually sets the URLRequestFtpJob's // priority, both before and after start. TEST_F(URLRequestFtpJobPriorityTest, SetPriorityBasic) { - scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob(&req_)); + scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob( + &req_, &ftp_factory_, &ftp_auth_cache_)); EXPECT_EQ(DEFAULT_PRIORITY, job->priority()); job->SetPriority(LOWEST); @@ -114,7 +154,8 @@ TEST_F(URLRequestFtpJobPriorityTest, SetPriorityBasic) { // Make sure that URLRequestFtpJob passes on its priority to its // transaction on start. TEST_F(URLRequestFtpJobPriorityTest, SetTransactionPriorityOnStart) { - scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob(&req_)); + scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob( + &req_, &ftp_factory_, &ftp_auth_cache_)); job->SetPriority(LOW); EXPECT_FALSE(network_layer_.last_transaction()); @@ -128,7 +169,8 @@ TEST_F(URLRequestFtpJobPriorityTest, SetTransactionPriorityOnStart) { // Make sure that URLRequestFtpJob passes on its priority updates to // its transaction. TEST_F(URLRequestFtpJobPriorityTest, SetTransactionPriority) { - scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob(&req_)); + scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob( + &req_, &ftp_factory_, &ftp_auth_cache_)); job->SetPriority(LOW); job->Start(); ASSERT_TRUE(network_layer_.last_transaction()); @@ -141,7 +183,8 @@ TEST_F(URLRequestFtpJobPriorityTest, SetTransactionPriority) { // Make sure that URLRequestFtpJob passes on its priority updates to // newly-created transactions after the first one. TEST_F(URLRequestFtpJobPriorityTest, SetSubsequentTransactionPriority) { - scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob(&req_)); + scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob( + &req_, &ftp_factory_, &ftp_auth_cache_)); job->Start(); job->SetPriority(LOW); @@ -157,19 +200,6 @@ TEST_F(URLRequestFtpJobPriorityTest, SetSubsequentTransactionPriority) { EXPECT_EQ(LOW, network_layer_.last_transaction()->priority()); } -class FtpTestURLRequestContext : public TestURLRequestContext { - public: - FtpTestURLRequestContext(ClientSocketFactory* socket_factory, - ProxyService* proxy_service, - NetworkDelegate* network_delegate) - : TestURLRequestContext(true) { - set_client_socket_factory(socket_factory); - context_storage_.set_proxy_service(proxy_service); - set_network_delegate(network_delegate); - Init(); - } -}; - class URLRequestFtpJobTest : public testing::Test { public: URLRequestFtpJobTest() @@ -177,7 +207,8 @@ class URLRequestFtpJobTest : public testing::Test { new SimpleProxyConfigService, NULL, NULL)), request_context_(&socket_factory_, proxy_service_, - &network_delegate_) { + &network_delegate_, + &ftp_transaction_factory_) { } virtual ~URLRequestFtpJobTest() { @@ -197,7 +228,7 @@ class URLRequestFtpJobTest : public testing::Test { socket_data_.push_back(socket_data); } - URLRequestContext* request_context() { return &request_context_; } + FtpTestURLRequestContext* request_context() { return &request_context_; } TestNetworkDelegate* network_delegate() { return &network_delegate_; } DeterministicSocketData* socket_data(size_t index) { return socket_data_[index]; @@ -207,6 +238,7 @@ class URLRequestFtpJobTest : public testing::Test { ScopedVector<DeterministicSocketData> socket_data_; DeterministicMockClientSocketFactory socket_factory_; TestNetworkDelegate network_delegate_; + MockFtpTransactionFactory ftp_transaction_factory_; // Owned by |request_context_|: ProxyService* proxy_service_; @@ -461,7 +493,7 @@ TEST_F(URLRequestFtpJobTest, FtpProxyRequestNeedProxyAndServerAuth) { GURL url("ftp://ftp.example.com"); // Make sure cached FTP credentials are not used for proxy authentication. - request_context()->ftp_auth_cache()->Add( + request_context()->GetFtpAuthCache()->Add( url.GetOrigin(), AuthCredentials(ASCIIToUTF16("userdonotuse"), ASCIIToUTF16("passworddonotuse"))); diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc index ade6c15..71904a5 100644 --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -43,6 +43,7 @@ #include "net/url_request/url_request.h" #include "net/url_request/url_request_context.h" #include "net/url_request/url_request_error_job.h" +#include "net/url_request/url_request_job_factory.h" #include "net/url_request/url_request_redirect_job.h" #include "net/url_request/url_request_throttler_header_adapter.h" #include "net/url_request/url_request_throttler_manager.h" @@ -1049,25 +1050,16 @@ Filter* URLRequestHttpJob::SetupFilter() const { } bool URLRequestHttpJob::IsSafeRedirect(const GURL& location) { - // We only allow redirects to certain "safe" protocols. This does not - // restrict redirects to externally handled protocols. Our consumer would - // need to take care of those. - - if (!URLRequest::IsHandledURL(location)) + // HTTP is always safe. + // TODO(pauljensen): Remove once crbug.com/146591 is fixed. + if (location.is_valid() && + (location.scheme() == "http" || location.scheme() == "https")) { return true; - - static const char* kSafeSchemes[] = { - "http", - "https", - "ftp" - }; - - for (size_t i = 0; i < arraysize(kSafeSchemes); ++i) { - if (location.SchemeIs(kSafeSchemes[i])) - return true; } - - return false; + // Query URLRequestJobFactory as to whether |location| would be safe to + // redirect to. + return request_->context()->job_factory() && + request_->context()->job_factory()->IsSafeRedirectTarget(location); } bool URLRequestHttpJob::NeedsAuth() { diff --git a/net/url_request/url_request_job_factory.cc b/net/url_request/url_request_job_factory.cc index c1f070b..e203cb1 100644 --- a/net/url_request/url_request_job_factory.cc +++ b/net/url_request/url_request_job_factory.cc @@ -8,6 +8,11 @@ namespace net { URLRequestJobFactory::ProtocolHandler::~ProtocolHandler() {} +bool URLRequestJobFactory::ProtocolHandler::IsSafeRedirectTarget( + const GURL& location) const { + return true; +} + URLRequestJobFactory::URLRequestJobFactory() {} URLRequestJobFactory::~URLRequestJobFactory() {} diff --git a/net/url_request/url_request_job_factory.h b/net/url_request/url_request_job_factory.h index bbd1d09..d4cc49e 100644 --- a/net/url_request/url_request_job_factory.h +++ b/net/url_request/url_request_job_factory.h @@ -30,6 +30,13 @@ class NET_EXPORT URLRequestJobFactory virtual URLRequestJob* MaybeCreateJob( URLRequest* request, NetworkDelegate* network_delegate) const = 0; + + // Indicates if it should be safe to redirect to |location|. Should handle + // protocols handled by MaybeCreateJob(). Only called when registered with + // URLRequestJobFactoryImpl::SetProtocolHandler() not called when used with + // ProtocolInterceptJobFactory. + // NOTE(pauljensen): Default implementation returns true. + virtual bool IsSafeRedirectTarget(const GURL& location) const; }; URLRequestJobFactory(); @@ -44,6 +51,8 @@ class NET_EXPORT URLRequestJobFactory virtual bool IsHandledURL(const GURL& url) const = 0; + virtual bool IsSafeRedirectTarget(const GURL& location) const = 0; + private: DISALLOW_COPY_AND_ASSIGN(URLRequestJobFactory); }; diff --git a/net/url_request/url_request_job_factory_impl.cc b/net/url_request/url_request_job_factory_impl.cc index f4c1d01..84fb951 100644 --- a/net/url_request/url_request_job_factory_impl.cc +++ b/net/url_request/url_request_job_factory_impl.cc @@ -64,4 +64,20 @@ bool URLRequestJobFactoryImpl::IsHandledURL(const GURL& url) const { return IsHandledProtocol(url.scheme()); } +bool URLRequestJobFactoryImpl::IsSafeRedirectTarget( + const GURL& location) const { + DCHECK(CalledOnValidThread()); + if (!location.is_valid()) { + // Error cases are safely handled. + return true; + } + ProtocolHandlerMap::const_iterator it = protocol_handler_map_.find( + location.scheme()); + if (it == protocol_handler_map_.end()) { + // Unhandled cases are safely handled. + return true; + } + return it->second->IsSafeRedirectTarget(location); +} + } // namespace net diff --git a/net/url_request/url_request_job_factory_impl.h b/net/url_request/url_request_job_factory_impl.h index 8f394e2..4f03fb0 100644 --- a/net/url_request/url_request_job_factory_impl.h +++ b/net/url_request/url_request_job_factory_impl.h @@ -33,6 +33,7 @@ class NET_EXPORT URLRequestJobFactoryImpl : public URLRequestJobFactory { NetworkDelegate* network_delegate) const OVERRIDE; virtual bool IsHandledProtocol(const std::string& scheme) const OVERRIDE; virtual bool IsHandledURL(const GURL& url) const OVERRIDE; + virtual bool IsSafeRedirectTarget(const GURL& location) const OVERRIDE; private: typedef std::map<std::string, ProtocolHandler*> ProtocolHandlerMap; diff --git a/net/url_request/url_request_job_manager.cc b/net/url_request/url_request_job_manager.cc index 283ee0d..5a64b41 100644 --- a/net/url_request/url_request_job_manager.cc +++ b/net/url_request/url_request_job_manager.cc @@ -12,12 +12,8 @@ #include "net/base/load_flags.h" #include "net/base/net_errors.h" #include "net/base/network_delegate.h" -#include "net/url_request/url_request_about_job.h" #include "net/url_request/url_request_context.h" -#include "net/url_request/url_request_data_job.h" #include "net/url_request/url_request_error_job.h" -#include "net/url_request/url_request_file_job.h" -#include "net/url_request/url_request_ftp_job.h" #include "net/url_request/url_request_http_job.h" #include "net/url_request/url_request_job_factory.h" @@ -36,12 +32,6 @@ struct SchemeToFactory { static const SchemeToFactory kBuiltinFactories[] = { { "http", URLRequestHttpJob::Factory }, { "https", URLRequestHttpJob::Factory }, - { "file", URLRequestFileJob::Factory }, -#if !defined(DISABLE_FTP_SUPPORT) - { "ftp", URLRequestFtpJob::Factory }, -#endif - { "about", URLRequestAboutJob::Factory }, - { "data", URLRequestDataJob::Factory }, }; // static diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc index 48ab8d9..b1dab4b 100644 --- a/net/url_request/url_request_test_util.cc +++ b/net/url_request/url_request_test_util.cc @@ -72,14 +72,6 @@ void TestURLRequestContext::Init() { context_storage_.set_proxy_service(ProxyService::CreateDirect()); if (!cert_verifier()) context_storage_.set_cert_verifier(CertVerifier::CreateDefault()); - if (!ftp_transaction_factory()) { -#if !defined(DISABLE_FTP_SUPPORT) - context_storage_.set_ftp_transaction_factory( - new FtpNetworkLayer(host_resolver())); -#else - context_storage_.set_ftp_transaction_factory(NULL); -#endif // !defined(DISABLE_FTP_SUPPORT) - } if (!ssl_config_service()) context_storage_.set_ssl_config_service(new SSLConfigServiceDefaults); if (!http_auth_handler_factory()) { diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc index dec3ba2..8f7864f 100644 --- a/net/url_request/url_request_unittest.cc +++ b/net/url_request/url_request_unittest.cc @@ -58,6 +58,8 @@ #include "net/ssl/ssl_connection_status_flags.h" #include "net/test/cert_test_util.h" #include "net/test/spawned_test_server/spawned_test_server.h" +#include "net/url_request/data_protocol_handler.h" +#include "net/url_request/file_protocol_handler.h" #include "net/url_request/ftp_protocol_handler.h" #include "net/url_request/static_http_user_agent_settings.h" #include "net/url_request/url_request.h" @@ -558,6 +560,9 @@ class URLRequestTest : public PlatformTest { URLRequestTest() : default_context_(true) { default_context_.set_network_delegate(&default_network_delegate_); default_context_.set_net_log(&net_log_); + job_factory_.SetProtocolHandler("data", new DataProtocolHandler); + job_factory_.SetProtocolHandler("file", new FileProtocolHandler); + default_context_.set_job_factory(&job_factory_); default_context_.Init(); } virtual ~URLRequestTest() {} @@ -565,16 +570,15 @@ class URLRequestTest : public PlatformTest { // Adds the TestJobInterceptor to the default context. TestJobInterceptor* AddTestInterceptor() { TestJobInterceptor* protocol_handler_ = new TestJobInterceptor(); - job_factory_.reset(new URLRequestJobFactoryImpl); - job_factory_->SetProtocolHandler("http", protocol_handler_); - default_context_.set_job_factory(job_factory_.get()); + job_factory_.SetProtocolHandler("http", NULL); + job_factory_.SetProtocolHandler("http", protocol_handler_); return protocol_handler_; } protected: CapturingNetLog net_log_; TestNetworkDelegate default_network_delegate_; // Must outlive URLRequest. - scoped_ptr<URLRequestJobFactoryImpl> job_factory_; + URLRequestJobFactoryImpl job_factory_; TestURLRequestContext default_context_; }; @@ -3862,6 +3866,20 @@ TEST_F(URLRequestTestHTTP, ContentTypeNormalizationTest) { req.Cancel(); } +TEST_F(URLRequestTestHTTP, ProtocolHandlerAndFactoryRestrictRedirects) { + // Test URLRequestJobFactory::ProtocolHandler::IsSafeRedirectTarget(). + GURL file_url("file:///foo.txt"); + GURL data_url("data:,foo"); + FileProtocolHandler file_protocol_handler; + EXPECT_FALSE(file_protocol_handler.IsSafeRedirectTarget(file_url)); + DataProtocolHandler data_protocol_handler; + EXPECT_TRUE(data_protocol_handler.IsSafeRedirectTarget(data_url)); + + // Test URLRequestJobFactoryImpl::IsSafeRedirectTarget(). + EXPECT_FALSE(job_factory_.IsSafeRedirectTarget(file_url)); + EXPECT_TRUE(job_factory_.IsSafeRedirectTarget(data_url)); +} + TEST_F(URLRequestTestHTTP, RestrictRedirects) { ASSERT_TRUE(test_server_.Start()); @@ -5452,15 +5470,12 @@ TEST_F(URLRequestTestFTP, UnsafePort) { ASSERT_TRUE(test_server_.Start()); URLRequestJobFactoryImpl job_factory; + FtpNetworkLayer ftp_transaction_factory(default_context_.host_resolver()); GURL url("ftp://127.0.0.1:7"); - FtpProtocolHandler ftp_protocol_handler( - default_context_.ftp_transaction_factory(), - default_context_.ftp_auth_cache()); job_factory.SetProtocolHandler( "ftp", - new FtpProtocolHandler(default_context_.ftp_transaction_factory(), - default_context_.ftp_auth_cache())); + new FtpProtocolHandler(&ftp_transaction_factory)); default_context_.set_job_factory(&job_factory); TestDelegate d; |