Disallow fetching of file:// URLs discovered by DHCP-based WPAD.
Bug: 839647
TBR: stevenjb@chromium.org
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I9223238b5f187d632d68d54fd2ee4fd50981fc56
Reviewed-on: https://chromium-review.googlesource.com/1045610
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556644}diff --git a/chromeos/network/dhcp_pac_file_fetcher_chromeos.cc b/chromeos/network/dhcp_pac_file_fetcher_chromeos.cc
index c4d4348..4605b22 100644
--- a/chromeos/network/dhcp_pac_file_fetcher_chromeos.cc
+++ b/chromeos/network/dhcp_pac_file_fetcher_chromeos.cc
@@ -35,7 +35,7 @@
net::URLRequestContext* url_request_context)
: weak_ptr_factory_(this) {
DCHECK(url_request_context);
- pac_file_fetcher_.reset(new net::PacFileFetcherImpl(url_request_context));
+ pac_file_fetcher_ = net::PacFileFetcherImpl::Create(url_request_context);
if (NetworkHandler::IsInitialized())
network_handler_task_runner_ = NetworkHandler::Get()->task_runner();
}
diff --git a/net/data/pac_file_fetcher_unittest/redirect-to-file b/net/data/pac_file_fetcher_unittest/redirect-to-file
new file mode 100644
index 0000000..f21ca1e
--- /dev/null
+++ b/net/data/pac_file_fetcher_unittest/redirect-to-file
@@ -0,0 +1 @@
+-redirected-
diff --git a/net/data/pac_file_fetcher_unittest/redirect-to-file.mock-http-headers b/net/data/pac_file_fetcher_unittest/redirect-to-file.mock-http-headers
new file mode 100644
index 0000000..20a45ae
--- /dev/null
+++ b/net/data/pac_file_fetcher_unittest/redirect-to-file.mock-http-headers
@@ -0,0 +1,2 @@
+HTTP/1.1 302 Found
+Location: file://bin/cat
diff --git a/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win.cc b/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win.cc
index b81f44d..d6515969 100644
--- a/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win.cc
+++ b/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win.cc
@@ -153,7 +153,7 @@
TransitionToFinish();
} else {
state_ = STATE_WAIT_URL;
- script_fetcher_.reset(ImplCreateScriptFetcher());
+ script_fetcher_ = ImplCreateScriptFetcher();
script_fetcher_->Fetch(pac_url_, &pac_script_,
base::Bind(&DhcpPacFileAdapterFetcher::OnFetcherDone,
base::Unretained(this)),
@@ -194,8 +194,9 @@
return state_;
}
-PacFileFetcher* DhcpPacFileAdapterFetcher::ImplCreateScriptFetcher() {
- return new PacFileFetcherImpl(url_request_context_);
+std::unique_ptr<PacFileFetcher>
+DhcpPacFileAdapterFetcher::ImplCreateScriptFetcher() {
+ return PacFileFetcherImpl::Create(url_request_context_);
}
DhcpPacFileAdapterFetcher::DhcpQuery*
diff --git a/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win.h b/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win.h
index 6e07d2b..4d84aef 100644
--- a/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win.h
+++ b/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win.h
@@ -148,7 +148,7 @@
};
// Virtual methods introduced to allow unit testing.
- virtual PacFileFetcher* ImplCreateScriptFetcher();
+ virtual std::unique_ptr<PacFileFetcher> ImplCreateScriptFetcher();
virtual DhcpQuery* ImplCreateDhcpQuery();
virtual base::TimeDelta ImplGetTimeout() const;
diff --git a/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win_unittest.cc b/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win_unittest.cc
index ac97700..7fbffc8 100644
--- a/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win_unittest.cc
+++ b/net/proxy_resolution/dhcp_pac_file_adapter_fetcher_win_unittest.cc
@@ -56,7 +56,7 @@
fetcher_ = NULL;
}
- PacFileFetcher* ImplCreateScriptFetcher() override {
+ std::unique_ptr<PacFileFetcher> ImplCreateScriptFetcher() override {
// We don't maintain ownership of the fetcher, it is transferred to
// the caller.
fetcher_ = new MockPacFileFetcher();
@@ -65,7 +65,7 @@
FROM_HERE, base::TimeDelta::FromMilliseconds(fetcher_delay_ms_), this,
&MockDhcpPacFileAdapterFetcher::OnFetcherTimer);
}
- return fetcher_;
+ return base::WrapUnique(fetcher_);
}
class DelayingDhcpQuery : public DhcpQuery {
@@ -276,9 +276,8 @@
url_request_context_(context) {}
// Returns a real PAC file fetcher.
- PacFileFetcher* ImplCreateScriptFetcher() override {
- PacFileFetcher* fetcher = new PacFileFetcherImpl(url_request_context_);
- return fetcher;
+ std::unique_ptr<PacFileFetcher> ImplCreateScriptFetcher() override {
+ return PacFileFetcherImpl::Create(url_request_context_);
}
URLRequestContext* url_request_context_;
diff --git a/net/proxy_resolution/pac_file_fetcher_impl.cc b/net/proxy_resolution/pac_file_fetcher_impl.cc
index 6a02e00..688f32f 100644
--- a/net/proxy_resolution/pac_file_fetcher_impl.cc
+++ b/net/proxy_resolution/pac_file_fetcher_impl.cc
@@ -19,6 +19,7 @@
#include "net/base/request_priority.h"
#include "net/cert/cert_status_flags.h"
#include "net/http/http_response_headers.h"
+#include "net/url_request/redirect_info.h"
#include "net/url_request/url_request_context.h"
// TODO(eroman):
@@ -81,17 +82,15 @@
} // namespace
-PacFileFetcherImpl::PacFileFetcherImpl(URLRequestContext* url_request_context)
- : url_request_context_(url_request_context),
- buf_(new IOBuffer(kBufSize)),
- next_id_(0),
- cur_request_id_(0),
- result_code_(OK),
- result_text_(NULL),
- max_response_bytes_(kDefaultMaxResponseBytes),
- max_duration_(kDefaultMaxDuration),
- weak_factory_(this) {
- DCHECK(url_request_context);
+std::unique_ptr<PacFileFetcherImpl> PacFileFetcherImpl::Create(
+ URLRequestContext* url_request_context) {
+ return base::WrapUnique(new PacFileFetcherImpl(url_request_context, false));
+}
+
+std::unique_ptr<PacFileFetcherImpl>
+PacFileFetcherImpl::CreateWithFileUrlSupport(
+ URLRequestContext* url_request_context) {
+ return base::WrapUnique(new PacFileFetcherImpl(url_request_context, true));
}
PacFileFetcherImpl::~PacFileFetcherImpl() {
@@ -137,6 +136,9 @@
if (!url_request_context_)
return ERR_CONTEXT_SHUT_DOWN;
+ if (!IsUrlSchemeAllowed(url))
+ return ERR_DISALLOWED_URL_SCHEME;
+
// Handle base-64 encoded data-urls that contain custom PAC scripts.
if (url.SchemeIs("data")) {
std::string mime_type;
@@ -211,6 +213,27 @@
}
}
+void PacFileFetcherImpl::OnReceivedRedirect(URLRequest* request,
+ const RedirectInfo& redirect_info,
+ bool* defer_redirect) {
+ int error = OK;
+
+ // Redirection to file:// is never OK. Ordinarily this is handled lower in the
+ // stack (|FileProtocolHandler::IsSafeRedirectTarget|), but this is reachable
+ // when built without file:// suppport. Return the same error for consistency.
+ if (redirect_info.new_url.SchemeIsFile()) {
+ error = ERR_UNSAFE_REDIRECT;
+ } else if (!IsUrlSchemeAllowed(redirect_info.new_url)) {
+ error = ERR_DISALLOWED_URL_SCHEME;
+ }
+
+ if (error != OK) {
+ // Fail the redirect.
+ request->CancelWithError(error);
+ OnResponseCompleted(request, error);
+ }
+}
+
void PacFileFetcherImpl::OnAuthRequired(URLRequest* request,
AuthChallengeInfo* auth_info) {
DCHECK_EQ(request, cur_request_.get());
@@ -280,6 +303,36 @@
}
}
+PacFileFetcherImpl::PacFileFetcherImpl(URLRequestContext* url_request_context,
+ bool allow_file_url)
+ : url_request_context_(url_request_context),
+ buf_(new IOBuffer(kBufSize)),
+ next_id_(0),
+ cur_request_id_(0),
+ result_code_(OK),
+ result_text_(NULL),
+ max_response_bytes_(kDefaultMaxResponseBytes),
+ max_duration_(kDefaultMaxDuration),
+ allow_file_url_(allow_file_url),
+ weak_factory_(this) {
+ DCHECK(url_request_context);
+}
+
+bool PacFileFetcherImpl::IsUrlSchemeAllowed(const GURL& url) const {
+ // Always allow http://, https://, data:, and ftp://.
+ if (url.SchemeIsHTTPOrHTTPS() || url.SchemeIs("ftp") || url.SchemeIs("data"))
+ return true;
+
+ // Only permit file:// if |allow_file_url_| was set. file:// should not be
+ // allowed for URLs that were auto-detected, or as the result of a server-side
+ // redirect.
+ if (url.SchemeIsFile())
+ return allow_file_url_;
+
+ // Disallow any other URL scheme.
+ return false;
+}
+
void PacFileFetcherImpl::ReadBody(URLRequest* request) {
// Read as many bytes as are available synchronously.
while (true) {
diff --git a/net/proxy_resolution/pac_file_fetcher_impl.h b/net/proxy_resolution/pac_file_fetcher_impl.h
index dac5a82..92d95993 100644
--- a/net/proxy_resolution/pac_file_fetcher_impl.h
+++ b/net/proxy_resolution/pac_file_fetcher_impl.h
@@ -38,7 +38,23 @@
// Note that while a request is in progress, we will be holding a reference
// to |url_request_context|. Be careful not to create cycles between the
// fetcher and the context; you can break such cycles by calling Cancel().
- explicit PacFileFetcherImpl(URLRequestContext* url_request_context);
+ //
+ // Fetch() supports the following URL schemes, provided the underlying
+ // |url_request_context| also supports them:
+ //
+ // * http://
+ // * https://
+ // * ftp://
+ // * data:
+ static std::unique_ptr<PacFileFetcherImpl> Create(
+ URLRequestContext* url_request_context);
+
+ // Same as Create(), but additionally allows fetching PAC URLs from file://
+ // URLs (provided the URLRequestContext supports it).
+ //
+ // This should not be used in new code (see https://crbug.com/839566).
+ static std::unique_ptr<PacFileFetcherImpl> CreateWithFileUrlSupport(
+ URLRequestContext* url_request_context);
~PacFileFetcherImpl() override;
@@ -58,6 +74,9 @@
void OnShutdown() override;
// URLRequest::Delegate methods:
+ void OnReceivedRedirect(URLRequest* request,
+ const RedirectInfo& redirect_info,
+ bool* defer_redirect) override;
void OnAuthRequired(URLRequest* request,
AuthChallengeInfo* auth_info) override;
void OnSSLCertificateError(URLRequest* request,
@@ -69,6 +88,13 @@
private:
enum { kBufSize = 4096 };
+ PacFileFetcherImpl(URLRequestContext* url_request_context,
+ bool allow_file_url);
+
+ // Returns true if |url| has an acceptable URL scheme (i.e. http://, https://,
+ // etc).
+ bool IsUrlSchemeAllowed(const GURL& url) const;
+
// Read more bytes from the response.
void ReadBody(URLRequest* request);
@@ -129,6 +155,8 @@
// The time that the first byte was received.
base::TimeTicks fetch_time_to_first_byte_;
+ const bool allow_file_url_;
+
// Factory for creating the time-out task. This takes care of revoking
// outstanding tasks when |this| is deleted.
base::WeakPtrFactory<PacFileFetcherImpl> weak_factory_;
diff --git a/net/proxy_resolution/pac_file_fetcher_impl_unittest.cc b/net/proxy_resolution/pac_file_fetcher_impl_unittest.cc
index 635178a..9fa21ce 100644
--- a/net/proxy_resolution/pac_file_fetcher_impl_unittest.cc
+++ b/net/proxy_resolution/pac_file_fetcher_impl_unittest.cc
@@ -123,9 +123,12 @@
URLRequestContextStorage storage_;
};
-#if !BUILDFLAG(DISABLE_FILE_SUPPORT)
// Get a file:// url relative to net/data/proxy/pac_file_fetcher_unittest.
GURL GetTestFileUrl(const std::string& relpath) {
+#if BUILDFLAG(DISABLE_FILE_SUPPORT)
+ LOG(WARNING)
+ << "Built a file:// URL to test data, however file support is disabled.";
+#endif // BUILDFLAG(DISABLE_FILE_SUPPORT)
base::FilePath path;
base::PathService::Get(base::DIR_SOURCE_ROOT, &path);
path = path.AppendASCII("net");
@@ -134,7 +137,6 @@
GURL base_url = FilePathToFileURL(path);
return GURL(base_url.spec() + "/" + relpath);
}
-#endif // !BUILDFLAG(DISABLE_FILE_SUPPORT)
// Really simple NetworkDelegate so we can allow local file access on ChromeOS
// without introducing layering violations. Also causes a test failure if a
@@ -224,46 +226,91 @@
RequestContext context_;
};
-#if !BUILDFLAG(DISABLE_FILE_SUPPORT)
-TEST_F(PacFileFetcherImplTest, FileUrl) {
- PacFileFetcherImpl pac_fetcher(&context_);
+// Try fetching a non-existent PAC file via file://.
+TEST_F(PacFileFetcherImplTest, FileUrlDoesNotExist) {
+ auto pac_fetcher = PacFileFetcherImpl::CreateWithFileUrlSupport(&context_);
- { // Fetch a non-existent file.
- base::string16 text;
- TestCompletionCallback callback;
- int result =
- pac_fetcher.Fetch(GetTestFileUrl("does-not-exist"), &text,
- callback.callback(), TRAFFIC_ANNOTATION_FOR_TESTS);
- EXPECT_THAT(result, IsError(ERR_IO_PENDING));
- EXPECT_THAT(callback.WaitForResult(), IsError(ERR_FILE_NOT_FOUND));
- EXPECT_TRUE(text.empty());
- }
- { // Fetch a file that exists.
- base::string16 text;
- TestCompletionCallback callback;
- int result =
- pac_fetcher.Fetch(GetTestFileUrl("pac.txt"), &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
- EXPECT_THAT(result, IsError(ERR_IO_PENDING));
- EXPECT_THAT(callback.WaitForResult(), IsOk());
- EXPECT_EQ(ASCIIToUTF16("-pac.txt-\n"), text);
- }
-}
+ base::string16 text;
+ TestCompletionCallback callback;
+ int result =
+ pac_fetcher->Fetch(GetTestFileUrl("does-not-exist"), &text,
+ callback.callback(), TRAFFIC_ANNOTATION_FOR_TESTS);
+
+ EXPECT_THAT(result, IsError(ERR_IO_PENDING));
+ EXPECT_TRUE(text.empty());
+
+#if !BUILDFLAG(DISABLE_FILE_SUPPORT)
+ EXPECT_THAT(callback.WaitForResult(), IsError(ERR_FILE_NOT_FOUND));
+#else
+ EXPECT_THAT(callback.WaitForResult(), IsError(ERR_UNKNOWN_URL_SCHEME));
#endif // !BUILDFLAG(DISABLE_FILE_SUPPORT)
+}
+
+TEST_F(PacFileFetcherImplTest, FileUrlExists) {
+ auto pac_fetcher = PacFileFetcherImpl::CreateWithFileUrlSupport(&context_);
+
+ base::string16 text;
+ TestCompletionCallback callback;
+ int result =
+ pac_fetcher->Fetch(GetTestFileUrl("pac.txt"), &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
+
+ EXPECT_THAT(result, IsError(ERR_IO_PENDING));
+
+#if !BUILDFLAG(DISABLE_FILE_SUPPORT)
+ EXPECT_THAT(callback.WaitForResult(), IsOk());
+ EXPECT_EQ(ASCIIToUTF16("-pac.txt-\n"), text);
+#else
+ EXPECT_THAT(callback.WaitForResult(), IsError(ERR_UNKNOWN_URL_SCHEME));
+#endif // !BUILDFLAG(DISABLE_FILE_SUPPORT)
+}
+
+TEST_F(PacFileFetcherImplTest, FileUrlNotAllowed) {
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
+
+ // Fetch a file that exists, however the PacFileFetcherImpl does not allow use
+ // of file://.
+ base::string16 text;
+ TestCompletionCallback callback;
+ int result =
+ pac_fetcher->Fetch(GetTestFileUrl("pac.txt"), &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
+ EXPECT_THAT(result, IsError(ERR_DISALLOWED_URL_SCHEME));
+}
+
+// Even if the fetcher allows file://, a server-side redirect from http -->
+// file:// should not work.
+//
+// It is currently disallowed lower in the stack and results in an
+// ERR_UNSAFE_REDIRECT.
+TEST_F(PacFileFetcherImplTest, RedirectToFileUrl) {
+ ASSERT_TRUE(test_server_.Start());
+
+ auto pac_fetcher = PacFileFetcherImpl::CreateWithFileUrlSupport(&context_);
+
+ GURL url(test_server_.GetURL("/redirect-to-file"));
+
+ base::string16 text;
+ TestCompletionCallback callback;
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
+ EXPECT_THAT(result, IsError(ERR_IO_PENDING));
+ EXPECT_THAT(callback.WaitForResult(), IsError(ERR_UNSAFE_REDIRECT));
+}
// Note that all mime types are allowed for PAC file, to be consistent
// with other browsers.
TEST_F(PacFileFetcherImplTest, HttpMimeType) {
ASSERT_TRUE(test_server_.Start());
- PacFileFetcherImpl pac_fetcher(&context_);
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
{ // Fetch a PAC with mime type "text/plain"
GURL url(test_server_.GetURL("/pac.txt"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_EQ(ASCIIToUTF16("-pac.txt-\n"), text);
@@ -272,8 +319,8 @@
GURL url(test_server_.GetURL("/pac.html"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_EQ(ASCIIToUTF16("-pac.html-\n"), text);
@@ -282,8 +329,8 @@
GURL url(test_server_.GetURL("/pac.nsproxy"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_EQ(ASCIIToUTF16("-pac.nsproxy-\n"), text);
@@ -293,14 +340,14 @@
TEST_F(PacFileFetcherImplTest, HttpStatusCode) {
ASSERT_TRUE(test_server_.Start());
- PacFileFetcherImpl pac_fetcher(&context_);
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
{ // Fetch a PAC which gives a 500 -- FAIL
GURL url(test_server_.GetURL("/500.pac"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsError(ERR_PAC_STATUS_NOT_OK));
EXPECT_TRUE(text.empty());
@@ -309,8 +356,8 @@
GURL url(test_server_.GetURL("/404.pac"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsError(ERR_PAC_STATUS_NOT_OK));
EXPECT_TRUE(text.empty());
@@ -320,15 +367,15 @@
TEST_F(PacFileFetcherImplTest, ContentDisposition) {
ASSERT_TRUE(test_server_.Start());
- PacFileFetcherImpl pac_fetcher(&context_);
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
// Fetch PAC scripts via HTTP with a Content-Disposition header -- should
// have no effect.
GURL url(test_server_.GetURL("/downloadable.pac"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_EQ(ASCIIToUTF16("-downloadable.pac-\n"), text);
@@ -338,15 +385,15 @@
TEST_F(PacFileFetcherImplTest, NoCache) {
ASSERT_TRUE(test_server_.Start());
- PacFileFetcherImpl pac_fetcher(&context_);
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
// Fetch a PAC script whose HTTP headers make it cacheable for 1 hour.
GURL url(test_server_.GetURL("/cacheable_1hr.pac"));
{
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_EQ(ASCIIToUTF16("-cacheable_1hr.pac-\n"), text);
@@ -361,8 +408,8 @@
{
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
// Expect any error. The exact error varies by platform.
@@ -373,10 +420,10 @@
TEST_F(PacFileFetcherImplTest, TooLarge) {
ASSERT_TRUE(test_server_.Start());
- PacFileFetcherImpl pac_fetcher(&context_);
+ auto pac_fetcher = PacFileFetcherImpl::CreateWithFileUrlSupport(&context_);
// Set the maximum response size to 50 bytes.
- int prev_size = pac_fetcher.SetSizeConstraint(50);
+ int prev_size = pac_fetcher->SetSizeConstraint(50);
// These two URLs are the same file, but are http:// vs file://
GURL urls[] = {
@@ -392,22 +439,22 @@
const GURL& url = urls[i];
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsError(ERR_FILE_TOO_BIG));
EXPECT_TRUE(text.empty());
}
// Restore the original size bound.
- pac_fetcher.SetSizeConstraint(prev_size);
+ pac_fetcher->SetSizeConstraint(prev_size);
{ // Make sure we can still fetch regular URLs.
GURL url(test_server_.GetURL("/pac.nsproxy"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_EQ(ASCIIToUTF16("-pac.nsproxy-\n"), text);
@@ -418,13 +465,13 @@
TEST_F(PacFileFetcherImplTest, Empty) {
ASSERT_TRUE(test_server_.Start());
- PacFileFetcherImpl pac_fetcher(&context_);
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
GURL url(test_server_.GetURL("/empty"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_EQ(0u, text.size());
@@ -433,11 +480,11 @@
TEST_F(PacFileFetcherImplTest, Hang) {
ASSERT_TRUE(test_server_.Start());
- PacFileFetcherImpl pac_fetcher(&context_);
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
// Set the timeout period to 0.5 seconds.
base::TimeDelta prev_timeout =
- pac_fetcher.SetTimeoutConstraint(base::TimeDelta::FromMilliseconds(500));
+ pac_fetcher->SetTimeoutConstraint(base::TimeDelta::FromMilliseconds(500));
// Try fetching a URL which takes 1.2 seconds. We should abort the request
// after 500 ms, and fail with a timeout error.
@@ -445,22 +492,22 @@
GURL url(test_server_.GetURL("/slow/proxy.pac?1.2"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsError(ERR_TIMED_OUT));
EXPECT_TRUE(text.empty());
}
// Restore the original timeout period.
- pac_fetcher.SetTimeoutConstraint(prev_timeout);
+ pac_fetcher->SetTimeoutConstraint(prev_timeout);
{ // Make sure we can still fetch regular URLs.
GURL url(test_server_.GetURL("/pac.nsproxy"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_EQ(ASCIIToUTF16("-pac.nsproxy-\n"), text);
@@ -473,15 +520,15 @@
TEST_F(PacFileFetcherImplTest, Encodings) {
ASSERT_TRUE(test_server_.Start());
- PacFileFetcherImpl pac_fetcher(&context_);
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
// Test a response that is gzip-encoded -- should get inflated.
{
GURL url(test_server_.GetURL("/gzipped_pac"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_EQ(ASCIIToUTF16("This data was gzipped.\n"), text);
@@ -493,8 +540,8 @@
GURL url(test_server_.GetURL("/utf16be_pac"));
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_EQ(ASCIIToUTF16("This was encoded as UTF-16BE.\n"), text);
@@ -502,7 +549,7 @@
}
TEST_F(PacFileFetcherImplTest, DataURLs) {
- PacFileFetcherImpl pac_fetcher(&context_);
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
const char kEncodedUrl[] =
"data:application/x-ns-proxy-autoconfig;base64,ZnVuY3Rpb24gRmluZFByb3h5R"
@@ -520,8 +567,8 @@
GURL url(kEncodedUrl);
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsOk());
EXPECT_EQ(ASCIIToUTF16(kPacScript), text);
}
@@ -534,8 +581,8 @@
GURL url(kEncodedUrlBroken);
base::string16 text;
TestCompletionCallback callback;
- int result = pac_fetcher.Fetch(url, &text, callback.callback(),
- TRAFFIC_ANNOTATION_FOR_TESTS);
+ int result = pac_fetcher->Fetch(url, &text, callback.callback(),
+ TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_FAILED));
}
}
@@ -559,8 +606,7 @@
TestCompletionCallback callback;
base::string16 text;
for (int i = 0; i < num_requests; i++) {
- std::unique_ptr<PacFileFetcherImpl> pac_fetcher =
- std::make_unique<PacFileFetcherImpl>(&context_);
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
GURL url(test_server_.GetURL("/hung"));
// Fine to use the same string and callback for all of these, as they should
// all hang.
@@ -582,16 +628,16 @@
TEST_F(PacFileFetcherImplTest, OnShutdown) {
ASSERT_TRUE(test_server_.Start());
- PacFileFetcherImpl pac_fetcher(&context_);
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
base::string16 text;
TestCompletionCallback callback;
int result =
- pac_fetcher.Fetch(test_server_.GetURL("/hung"), &text,
- callback.callback(), TRAFFIC_ANNOTATION_FOR_TESTS);
+ pac_fetcher->Fetch(test_server_.GetURL("/hung"), &text,
+ callback.callback(), TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_IO_PENDING));
EXPECT_EQ(1u, context_.url_requests()->size());
- pac_fetcher.OnShutdown();
+ pac_fetcher->OnShutdown();
EXPECT_EQ(0u, context_.url_requests()->size());
EXPECT_THAT(callback.WaitForResult(), IsError(ERR_CONTEXT_SHUT_DOWN));
@@ -600,22 +646,23 @@
EXPECT_EQ(0u, context_.url_requests()->size());
EXPECT_FALSE(callback.have_result());
- result = pac_fetcher.Fetch(test_server_.GetURL("/hung"), &text,
- callback.callback(), TRAFFIC_ANNOTATION_FOR_TESTS);
+ result =
+ pac_fetcher->Fetch(test_server_.GetURL("/hung"), &text,
+ callback.callback(), TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_CONTEXT_SHUT_DOWN));
}
TEST_F(PacFileFetcherImplTest, OnShutdownWithNoLiveRequest) {
ASSERT_TRUE(test_server_.Start());
- PacFileFetcherImpl pac_fetcher(&context_);
- pac_fetcher.OnShutdown();
+ auto pac_fetcher = PacFileFetcherImpl::Create(&context_);
+ pac_fetcher->OnShutdown();
base::string16 text;
TestCompletionCallback callback;
int result =
- pac_fetcher.Fetch(test_server_.GetURL("/hung"), &text,
- callback.callback(), TRAFFIC_ANNOTATION_FOR_TESTS);
+ pac_fetcher->Fetch(test_server_.GetURL("/hung"), &text,
+ callback.callback(), TRAFFIC_ANNOTATION_FOR_TESTS);
EXPECT_THAT(result, IsError(ERR_CONTEXT_SHUT_DOWN));
EXPECT_EQ(0u, context_.url_requests()->size());
}
diff --git a/services/network/url_request_context_builder_mojo.cc b/services/network/url_request_context_builder_mojo.cc
index d1f27e39..122176e 100644
--- a/services/network/url_request_context_builder_mojo.cc
+++ b/services/network/url_request_context_builder_mojo.cc
@@ -56,8 +56,8 @@
if (mojo_proxy_resolver_factory_) {
std::unique_ptr<net::DhcpPacFileFetcher> dhcp_pac_file_fetcher =
dhcp_fetcher_factory_->Create(url_request_context);
- std::unique_ptr<net::PacFileFetcher> pac_file_fetcher =
- std::make_unique<net::PacFileFetcherImpl>(url_request_context);
+ auto pac_file_fetcher =
+ net::PacFileFetcherImpl::CreateWithFileUrlSupport(url_request_context);
return CreateProxyResolutionServiceUsingMojoFactory(
std::move(mojo_proxy_resolver_factory_),
std::move(proxy_config_service), std::move(pac_file_fetcher),
