Download retry: Support redirect in UrlDownloader.

Currently redirect is not supported for download in non network service
code path without a renderer where we handle the network request with
UrlDownloader.

We have two legit use cases for redirect in UrlDownloader.

1. Retry, we won't assume the renderer exists and the download URL is
the final URL.

2. Background download in normal user profile. It should be legit for
all clients to redirect.

Bug: 883387
Change-Id: Ib517b3a1e23fbfac939386c0f2341a15a54b70ab
Reviewed-on: https://chromium-review.googlesource.com/c/1312114
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604798}

components/download/content/internal/download_driver_impl.cc

@@ -164,6 +164,7 @@ void DownloadDriverImpl::Start(
   download_url_params->set_download_source(
       download::DownloadSource::INTERNAL_API);
   download_url_params->set_post_body(post_body);
+  download_url_params->set_follow_cross_origin_redirects(true);
 
   download_manager_->DownloadUrl(std::move(download_url_params),
                                  nullptr /* blob_data_handle */,

components/download/internal/common/download_item_impl.cc

@@ -2313,6 +2313,7 @@ void DownloadItemImpl::ResumeInterruptedDownload(
   // will only be sent to the URL returned by GetURL().
   download_params->set_referrer(GetReferrerUrl());
   download_params->set_referrer_policy(net::URLRequest::NEVER_CLEAR_REFERRER);
+  download_params->set_follow_cross_origin_redirects(false);
 
   TransitionTo(RESUMING_INTERNAL);
   RecordDownloadCountWithSource(source == ResumptionRequestSource::USER

components/download/internal/common/parallel_download_job.cc

@@ -291,6 +291,7 @@ void ParallelDownloadJob::CreateRequest(int64_t offset, int64_t length) {
   // download request.
   download_params->set_referrer(download_item_->GetReferrerUrl());
   download_params->set_referrer_policy(net::URLRequest::NEVER_CLEAR_REFERRER);
+  download_params->set_follow_cross_origin_redirects(false);
 
   // Send the request.
   worker->SendRequest(std::move(download_params), url_loader_factory_getter_,

components/download/public/common/download_url_parameters.h

@@ -204,7 +204,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters {
 
   // If |follow_cross_origin_redirects| is true, we will follow cross origin
   // redirects while downloading, otherwise, we'll attempt to navigate to the
-  // URL.
+  // URL or cancel the download.
   void set_follow_cross_origin_redirects(bool follow_cross_origin_redirects) {
     follow_cross_origin_redirects_ = follow_cross_origin_redirects;
   }

content/browser/download/download_browsertest.cc

@@ -1686,6 +1686,35 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, RedirectWhileResume) {
   EXPECT_GT(parameters.size, requests[2]->transferred_byte_count);
 }
 
+// Verify that DownloadUrl can support URL redirect.
+IN_PROC_BROWSER_TEST_F(DownloadContentTest, RedirectDownload) {
+  // Setup a redirect chain with two URL.
+  GURL first_url = embedded_test_server()->GetURL("example.com", "/first-url");
+  GURL download_url =
+      embedded_test_server()->GetURL("example.com", "/download");
+  TestDownloadHttpResponse::StartServingStaticResponse(
+      base::StringPrintf("HTTP/1.1 302 Redirect\r\n"
+                         "Location: %s\r\n\r\n",
+                         download_url.spec().c_str()),
+      first_url);
+  TestDownloadHttpResponse::StartServing(TestDownloadHttpResponse::Parameters(),
+                                         download_url);
+
+  // Start a download and explicitly specify to support redirect.
+  std::unique_ptr<DownloadTestObserver> observer(CreateWaiter(shell(), 1));
+  auto download_parameters = std::make_unique<download::DownloadUrlParameters>(
+      first_url, TRAFFIC_ANNOTATION_FOR_TESTS);
+  download_parameters->set_follow_cross_origin_redirects(true);
+  DownloadManagerForShell(shell())->DownloadUrl(std::move(download_parameters));
+  observer->WaitForFinished();
+
+  // Verify download is done.
+  std::vector<download::DownloadItem*> downloads;
+  DownloadManagerForShell(shell())->GetAllDownloads(&downloads);
+  EXPECT_EQ(1u, downloads.size());
+  EXPECT_EQ(download::DownloadItem::COMPLETE, downloads[0]->GetState());
+}
+
 // If the server response for the resumption request specifies a bad range (i.e.
 // not the range that was requested or an invalid or missing Content-Range
 // header), then the download should be marked as interrupted again without

content/browser/download/url_downloader.cc

@@ -46,9 +46,10 @@ std::unique_ptr<UrlDownloader> UrlDownloader::BeginDownload(
 
   // From this point forward, the |UrlDownloader| is responsible for
   // |started_callback|.
-  std::unique_ptr<UrlDownloader> downloader(
-      new UrlDownloader(std::move(request), delegate, is_parallel_request,
-                        params->request_origin(), params->download_source()));
+  std::unique_ptr<UrlDownloader> downloader(new UrlDownloader(
+      std::move(request), delegate, is_parallel_request,
+      params->request_origin(), params->follow_cross_origin_redirects(),
+      params->download_source()));
   downloader->Start();
 
   return downloader;
@@ -59,6 +60,7 @@ UrlDownloader::UrlDownloader(
     base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate,
     bool is_parallel_request,
     const std::string& request_origin,
+    bool follow_cross_origin_redirects,
     download::DownloadSource download_source)
     : request_(std::move(request)),
       delegate_(delegate),
@@ -67,6 +69,7 @@ UrlDownloader::UrlDownloader(
             is_parallel_request,
             request_origin,
             download_source),
+      follow_cross_origin_redirects_(follow_cross_origin_redirects),
       weak_ptr_factory_(this) {}
 
 UrlDownloader::~UrlDownloader() {
@@ -82,13 +85,12 @@ void UrlDownloader::Start() {
 void UrlDownloader::OnReceivedRedirect(net::URLRequest* request,
                                        const net::RedirectInfo& redirect_info,
                                        bool* defer_redirect) {
+  if (follow_cross_origin_redirects_)
+    return;
+
   DVLOG(1) << "OnReceivedRedirect: " << request_->url().spec();
-  // We are going to block redirects even if DownloadRequestCore allows it.  No
-  // redirects are expected for download requests that are made without a
-  // renderer, which are currently exclusively resumption requests. Since there
-  // is no security policy being applied here, it's safer to block redirects and
-  // revisit if some previously unknown legitimate use case arises for redirects
-  // while resuming.
+
+  // Block redirects since there is no security policy being applied here.
   core_.OnWillAbort(download::DOWNLOAD_INTERRUPT_REASON_SERVER_UNREACHABLE);
   request_->CancelWithError(net::ERR_UNSAFE_REDIRECT);
 }

content/browser/download/url_downloader.h

@@ -32,6 +32,7 @@ class UrlDownloader : public net::URLRequest::Delegate,
                 base::WeakPtr<UrlDownloadHandler::Delegate> delegate,
                 bool is_parallel_request,
                 const std::string& request_origin,
+                bool follow_cross_origin_redirects,
                 download::DownloadSource download_source);
   ~UrlDownloader() override;
 
@@ -78,6 +79,8 @@ class UrlDownloader : public net::URLRequest::Delegate,
   base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate_;
   DownloadRequestCore core_;
 
+  bool follow_cross_origin_redirects_;
+
   base::WeakPtrFactory<UrlDownloader> weak_ptr_factory_;
 };