Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(471)

Unified Diff: net/url_request/url_request_unittest.cc

Issue 154473002: Support redirectUrl at onHeadersReceived in WebRequest / DWR API (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Add more tests Created 6 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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
« net/url_request/url_request_http_job.cc ('K') | « net/url_request/url_request_http_job.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698