Support redirectUrl at onHeadersReceived in WebRequest / DWR API

net/url_request/url_request_unittest.cc

 // Copyright (c) 2012 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 "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shlobj.h>
 #endif
@@ skipping 320 matching lines @@
     ASSERT_NE(AUTH_REQUIRED_RESPONSE_IO_PENDING, auth_retval);
     auth_retval_ = auth_retval;
   }
   void set_auth_credentials(const AuthCredentials& auth_credentials) {
     auth_credentials_ = auth_credentials;
   }
 
   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;
   }
 
   // Allows the user to check in which state did we block.
   Stage stage_blocked_for_callback() const {
     EXPECT_EQ(USER_CALLBACK, block_mode_);
     return stage_blocked_for_callback_;
   }
@@ skipping 32 matching lines @@
   int MaybeBlockStage(Stage stage, const CompletionCallback& callback);
 
   // Configuration parameters, can be adjusted by public methods:
   const BlockMode block_mode_;
 
   // Values returned on blocking stages when mode is SYNCHRONOUS or
   // AUTO_CALLBACK. For USER_CALLBACK these are set automatically to IO_PENDING.
   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
   // callback.
   AuthCredentials auth_credentials_;
   AuthCredentials* target_auth_credentials_;
 
   // Internal variables, not set by not the user:
   // Last blocked stage waiting for user callback (unused if |block_mode_| !=
   // USER_CALLBACK).
@@ skipping 76 matching lines @@
 
 int BlockingNetworkDelegate::OnHeadersReceived(
     URLRequest* request,
     const CompletionCallback& callback,
     const HttpResponseHeaders* original_response_headers,
     scoped_refptr<HttpResponseHeaders>* override_response_headers) {
   TestNetworkDelegate::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_ after following the redirect
instead.

+    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);
 }
 
 NetworkDelegate::AuthRequiredResponse BlockingNetworkDelegate::OnAuthRequired(
     URLRequest* request,
     const AuthChallengeInfo& auth_info,
     const AuthCallback& callback,
     AuthCredentials* credentials) {
   TestNetworkDelegate::OnAuthRequired(request, auth_info, callback,
                                       credentials);
@@ skipping 2189 matching lines @@
       EXPECT_TRUE(test_server_.Stop());
       EXPECT_TRUE(test_server_.Start());
     }
 
     return is_success;
   }
 
   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 anonymous namespace.

+const char kUnsafeUrlMarkedAsSafe[] = "data:text/html,something";

[mmenke, 2014/03/20 15:22:25]

nit:  Blank line before class definition.

optional:  May be better to make this a public static member of
OverrideRedirectNetworkDelegate.  It's often not done in tests, but I think it
makes clearer that it's the TestNetworkDelegate that marks it is safe, so easier
to tell how things work from looking at the test body.

+// 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 "RedirectOnHeadersReceivedNetworkDelegate".  A
bit of a mouthful, but it makes the tests clearer without having to read this
class's description.

+ 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 completeness.

+  }
+}

[mmenke, 2014/03/20 15:22:25]

Hmm...Can we have a double invalid redirect?  Redirect from http://valid to
data://invalid to data://invalid?  I don't think we can since currently only
http URLs allow redirection.  If I'm mistaken, though, we should have a test for
that case, with the first one allowed and the second not, for the same URL.

[robwu, 2014/03/20 16:21:11]

Only HTTP responses can be redirected at onHeadersReceived, so this test case is
not needed.

[mmenke, 2014/03/20 16:40:28]

And we always allow redirects to HTTP/HTTPS, of course.  Allrighty.

+
 // 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
 // follow.
 TEST_F(URLRequestTestHTTP, ProxyTunnelRedirectTest) {
   ASSERT_TRUE(test_server_.Start());
 
   TestNetworkDelegate network_delegate;  // Must outlive URLRequest.
   TestURLRequestContextWithProxy context(
       test_server_.host_port_pair().ToString(), &network_delegate);
@@ skipping 291 matching lines @@
     EXPECT_EQ(original_url, r.original_url());
     EXPECT_EQ(2U, r.url_chain().size());
     EXPECT_EQ(1, 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 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
 // URLRequest::Delegate.
 TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredSyncNoAction) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate(
       BlockingNetworkDelegate::SYNCHRONOUS);
@@ skipping 4573 matching lines @@
 
     EXPECT_FALSE(r.is_pending());
     EXPECT_EQ(1, d->response_started_count());
     EXPECT_FALSE(d->received_data_before_response());
     EXPECT_EQ(d->bytes_received(), static_cast<int>(file_size));
   }
 }
 #endif  // !defined(DISABLE_FTP_SUPPORT)
 
 }  // namespace net