Index: net/url_request/url_request_unittest.cc |
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc |
index 63e7c757244ddf6c31f28adcd369cd86c2a0ef48..6610ec4b604102973452c32a527ec50b6e92e409 100644 |
--- a/net/url_request/url_request_unittest.cc |
+++ b/net/url_request/url_request_unittest.cc |
@@ -338,6 +338,9 @@ class BlockingNetworkDelegate : public TestNetworkDelegate { |
void set_redirect_url(const GURL& url) { |
redirect_url_ = url; |
} |
+ void set_redirect_url_on_headers_received(const GURL& url) { |
+ redirect_url_on_headers_received_ = url; |
+ } |
void set_block_on(int block_on) { |
block_on_ = block_on; |
@@ -390,7 +393,8 @@ class BlockingNetworkDelegate : public TestNetworkDelegate { |
int retval_; // To be returned in non-auth stages. |
AuthRequiredResponse auth_retval_; |
- GURL redirect_url_; // Used if non-empty. |
+ GURL redirect_url_; // Used if non-empty during OnBeforeURLRequest. |
+ GURL redirect_url_on_headers_received_; // Used if non-empty. |
int block_on_; // Bit mask: in which stages to block. |
// |auth_credentials_| will be copied to |*target_auth_credential_| on |
@@ -487,6 +491,18 @@ int BlockingNetworkDelegate::OnHeadersReceived( |
request, callback, original_response_headers, |
override_response_headers); |
+ // Redirects to the same URL are allowed, but for simplicity, assume that |
+ // the tests only redirect to a different URL. |
+ if (!redirect_url_on_headers_received_.is_empty() && |
+ redirect_url_on_headers_received_ != request->url()) { |
mmenke
2014/03/20 15:22:25
Suggest resetting redirect_url_on_headers_received
|
+ if (override_response_headers->get() == NULL) { |
+ *override_response_headers = new net::HttpResponseHeaders( |
+ original_response_headers->raw_headers()); |
+ } |
+ (*override_response_headers)->SetSafeRedirect( |
+ redirect_url_on_headers_received_); |
+ } |
+ |
return MaybeBlockStage(ON_HEADERS_RECEIVED, callback); |
} |
@@ -2696,6 +2712,126 @@ class URLRequestTestHTTP : public URLRequestTest { |
LocalHttpTestServer test_server_; |
}; |
+// Unsafe URL that has been marked as safe in OverrideRedirectNetworkDelegate. |
mmenke
2014/03/20 15:22:25
This stuff (class and globals) should be in an ano
|
+const char kUnsafeUrlMarkedAsSafe[] = "data:text/html,something"; |
mmenke
2014/03/20 15:22:25
nit: Blank line before class definition.
optiona
|
+// OverrideRedirectNetworkDelegate first redirects to |redirect_url1|, then to |
+// |redirect_url2| (if set). |
mmenke
2014/03/20 15:22:25
Fix description.
|
+class OverrideRedirectNetworkDelegate : public TestNetworkDelegate { |
mmenke
2014/03/20 15:22:25
Suggest a clearer name. Maybe "RedirectOnHeadersR
|
+ public: |
+ explicit OverrideRedirectNetworkDelegate(const GURL& redirect_url) |
+ : redirect_url_(redirect_url) {} |
+ virtual ~OverrideRedirectNetworkDelegate() {} |
+ |
+ // net::NetworkDelegate implementation |
+ virtual int OnHeadersReceived( |
+ net::URLRequest* request, |
+ const net::CompletionCallback& callback, |
+ const net::HttpResponseHeaders* original_response_headers, |
+ scoped_refptr<net::HttpResponseHeaders>* override_response_headers) |
+ OVERRIDE; |
+ |
+ private: |
+ GURL redirect_url_; |
+ |
+ DISALLOW_COPY_AND_ASSIGN(OverrideRedirectNetworkDelegate); |
+}; |
+ |
+int OverrideRedirectNetworkDelegate::OnHeadersReceived( |
+ net::URLRequest* request, |
+ const net::CompletionCallback& callback, |
+ const net::HttpResponseHeaders* original_response_headers, |
+ scoped_refptr<net::HttpResponseHeaders>* override_response_headers) { |
+ |
+ if (redirect_url_ != request->url()) { |
+ net::HttpResponseHeaders* new_response_headers = |
+ new net::HttpResponseHeaders(original_response_headers->raw_headers()); |
+ |
+ new_response_headers->SetSafeRedirect(GURL(kUnsafeUrlMarkedAsSafe)); |
+ new_response_headers->RemoveHeader("Location"); |
+ new_response_headers->AddHeader("Location: " + redirect_url_.spec()); |
+ |
+ *override_response_headers = new_response_headers; |
+ } |
+ return TestNetworkDelegate::OnHeadersReceived(request, |
+ callback, |
+ original_response_headers, |
+ override_response_headers); |
+} |
+ |
+// Tests that replacing the allowed redirect URL is preserved when the |
+// Location header has been overwritten. |
+TEST_F(URLRequestTestHTTP, UnsafeRedirectToWhitelistedUnsafeURL) { |
+ ASSERT_TRUE(test_server_.Start()); |
+ |
+ GURL unsafe_redirect_url(kUnsafeUrlMarkedAsSafe); |
+ OverrideRedirectNetworkDelegate network_delegate(unsafe_redirect_url); |
+ default_context_.set_network_delegate(&network_delegate); |
+ TestDelegate d; |
+ { |
+ URLRequest r(test_server_.GetURL("empty.html"), |
+ DEFAULT_PRIORITY, |
+ &d, |
+ &default_context_); |
+ |
+ r.Start(); |
+ base::RunLoop().Run(); |
+ |
+ EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status()); |
+ |
+ EXPECT_EQ(2U, r.url_chain().size()); |
+ EXPECT_EQ(0, r.status().error()); |
+ EXPECT_EQ(unsafe_redirect_url, r.url()); |
+ } |
+} |
+ |
+// Tests that a redirect to a different unsafe URL is blocked, even after adding |
+// some other URL to the whitelist. |
+TEST_F(URLRequestTestHTTP, UnsafeRedirectToDifferentUnsafeURL) { |
+ ASSERT_TRUE(test_server_.Start()); |
+ |
+ GURL unsafe_redirect_url("data:text/html,something-else"); |
+ OverrideRedirectNetworkDelegate network_delegate(unsafe_redirect_url); |
+ default_context_.set_network_delegate(&network_delegate); |
+ TestDelegate d; |
+ { |
+ URLRequest r(test_server_.GetURL("empty.html"), |
+ DEFAULT_PRIORITY, |
+ &d, |
+ &default_context_); |
+ |
+ r.Start(); |
+ base::RunLoop().Run(); |
+ |
+ EXPECT_EQ(URLRequestStatus::FAILED, r.status().status()); |
+ EXPECT_EQ(ERR_UNSAFE_REDIRECT, r.status().error()); |
+ } |
+} |
+ |
+// Tests that a redirect to a safe URL is allowed, regardless of whether an |
+// unsafe URL was whitelisted. |
+TEST_F(URLRequestTestHTTP, UnsafeRedirectToSafeURL) { |
+ ASSERT_TRUE(test_server_.Start()); |
+ |
+ GURL safe_url(test_server_.GetURL("simple.html")); |
+ OverrideRedirectNetworkDelegate network_delegate(safe_url); |
+ default_context_.set_network_delegate(&network_delegate); |
+ TestDelegate d; |
+ { |
+ URLRequest r(test_server_.GetURL("empty.html"), |
+ DEFAULT_PRIORITY, |
+ &d, |
+ &default_context_); |
+ |
+ r.Start(); |
+ base::RunLoop().Run(); |
+ |
+ EXPECT_EQ(2U, r.url_chain().size()); |
+ EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status()); |
+ EXPECT_EQ(0, r.status().error()); |
+ EXPECT_EQ(safe_url, r.url()); |
mmenke
2014/03/20 15:22:25
Suggest testing the response body, too, for comple
|
+ } |
+} |
mmenke
2014/03/20 15:22:25
Hmm...Can we have a double invalid redirect? Redi
robwu
2014/03/20 16:21:11
Only HTTP responses can be redirected at onHeaders
mmenke
2014/03/20 16:40:28
And we always allow redirects to HTTP/HTTPS, of co
|
+ |
// In this unit test, we're using the HTTPTestServer as a proxy server and |
// issuing a CONNECT request with the magic host name "www.redirect.com". |
// The HTTPTestServer will return a 302 response, which we should not |
@@ -3007,6 +3143,83 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequestPost) { |
EXPECT_EQ(1, network_delegate.destroyed_requests()); |
} |
+// Tests that the network delegate can block and redirect a request to a new |
+// URL during OnHeadersReceived. |
+TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequestOnHeadersReceived) { |
+ ASSERT_TRUE(test_server_.Start()); |
+ |
+ TestDelegate d; |
+ BlockingNetworkDelegate network_delegate( |
+ BlockingNetworkDelegate::AUTO_CALLBACK); |
+ network_delegate.set_block_on(BlockingNetworkDelegate::ON_HEADERS_RECEIVED); |
+ GURL redirect_url(test_server_.GetURL("simple.html")); |
+ network_delegate.set_redirect_url_on_headers_received(redirect_url); |
+ |
+ TestURLRequestContextWithProxy context( |
+ test_server_.host_port_pair().ToString(), &network_delegate); |
+ |
+ { |
+ GURL original_url(test_server_.GetURL("empty.html")); |
+ URLRequest r(original_url, DEFAULT_PRIORITY, &d, &context); |
+ |
+ r.Start(); |
+ base::RunLoop().Run(); |
+ |
+ EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status()); |
+ EXPECT_EQ(0, r.status().error()); |
+ EXPECT_EQ(redirect_url, r.url()); |
+ EXPECT_EQ(original_url, r.original_url()); |
+ EXPECT_EQ(2U, r.url_chain().size()); |
+ EXPECT_EQ(2, network_delegate.created_requests()); |
+ EXPECT_EQ(0, network_delegate.destroyed_requests()); |
+ } |
+ EXPECT_EQ(1, network_delegate.destroyed_requests()); |
+} |
+ |
+// Tests that redirects caused by the network delegate during OnHeadersReceived |
+// preserve POST data. |
+TEST_F(URLRequestTestHTTP, |
+ NetworkDelegateRedirectRequestOnHeadersReceivedPost) { |
+ ASSERT_TRUE(test_server_.Start()); |
+ |
+ const char kData[] = "hello world"; |
+ |
+ TestDelegate d; |
+ BlockingNetworkDelegate network_delegate( |
+ BlockingNetworkDelegate::AUTO_CALLBACK); |
+ network_delegate.set_block_on(BlockingNetworkDelegate::ON_HEADERS_RECEIVED); |
+ GURL redirect_url(test_server_.GetURL("echo")); |
+ network_delegate.set_redirect_url_on_headers_received(redirect_url); |
+ |
+ TestURLRequestContext context(true); |
+ context.set_network_delegate(&network_delegate); |
+ context.Init(); |
+ |
+ { |
+ GURL original_url(test_server_.GetURL("empty.html")); |
+ URLRequest r(original_url, DEFAULT_PRIORITY, &d, &context); |
+ r.set_method("POST"); |
+ r.set_upload(make_scoped_ptr(CreateSimpleUploadData(kData))); |
+ HttpRequestHeaders headers; |
+ headers.SetHeader(HttpRequestHeaders::kContentLength, |
+ base::UintToString(arraysize(kData) - 1)); |
+ r.SetExtraRequestHeaders(headers); |
+ r.Start(); |
+ base::RunLoop().Run(); |
+ |
+ EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status()); |
+ EXPECT_EQ(0, r.status().error()); |
+ EXPECT_EQ(redirect_url, r.url()); |
+ EXPECT_EQ(original_url, r.original_url()); |
+ EXPECT_EQ(2U, r.url_chain().size()); |
+ EXPECT_EQ(2, network_delegate.created_requests()); |
+ EXPECT_EQ(0, network_delegate.destroyed_requests()); |
+ EXPECT_EQ("POST", r.method()); |
+ EXPECT_EQ(kData, d.data_received()); |
+ } |
+ EXPECT_EQ(1, network_delegate.destroyed_requests()); |
+} |
+ |
// Tests that the network delegate can synchronously complete OnAuthRequired |
// by taking no action. This indicates that the NetworkDelegate does not want to |
// handle the challenge, and is passing the buck along to the |