[Downloads] Do not store error responses during resumption.

content/browser/download/download_browsertest.cc

Index: content/browser/download/download_browsertest.cc
diff --git a/content/browser/download/download_browsertest.cc b/content/browser/download/download_browsertest.cc
index 794e8327bf8dc9fe14eab0a932ba8036bffc26db..bd44f00446b329e6cc477b338b86be26f5e10f06 100644
--- a/content/browser/download/download_browsertest.cc
+++ b/content/browser/download/download_browsertest.cc
@@ -1121,6 +1121,197 @@ IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, StrongValidators) {
             value);
 }
 
+// Resumption should only attempt to contact the final URL if the download has a
+// URL chain.
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, RedirectBeforeResume) {
+  TestDownloadRequestHandler request_handler_1(
+      GURL("http://example.com/first-url"));
+  request_handler_1.StartServingStaticResponse(
+      "HTTP/1.1 302 Redirect\r\n"
+      "Location: http://example.com/second-url\r\n"
+      "\r\n");
+
+  TestDownloadRequestHandler request_handler_2(
+      GURL("http://example.com/second-url"));
+  request_handler_2.StartServingStaticResponse(
+      "HTTP/1.1 302 Redirect\r\n"
+      "Location: http://example.com/third-url\r\n"
+      "\r\n");
+
+  TestDownloadRequestHandler request_handler_3(
+      GURL("http://example.com/third-url"));
+  request_handler_3.StartServingStaticResponse(
+      "HTTP/1.1 302 Redirect\r\n"
+      "Location: http://example.com/download\r\n"
+      "\r\n");
+
+  TestDownloadRequestHandler resumable_request_handler(
+      GURL("http://example.com/download"));
+  TestDownloadRequestHandler::Parameters parameters =
+      TestDownloadRequestHandler::Parameters::WithSingleInterruption();
+  resumable_request_handler.StartServing(parameters);
+
+  DownloadItem* download = StartDownloadAndReturnItem(
+      initiator_shell_for_resumption(), request_handler_1.url());
+  WaitForInterrupt(download);
+
+  EXPECT_EQ(4u, download->GetUrlChain().size());
+  EXPECT_EQ(request_handler_1.url(), download->GetOriginalUrl());
+  EXPECT_EQ(resumable_request_handler.url(), download->GetURL());
+
+  // Now that the download is interrupted, make all intermediate servers return
+  // a 404. The only way a resumption request would succeed if the resumption
+  // request is sent to the final server in the chain.
+  const char k404Response[] = "HTTP/1.1 404 Not found\r\n\r\n";
+  request_handler_1.StartServingStaticResponse(k404Response);
+  request_handler_2.StartServingStaticResponse(k404Response);
+  request_handler_3.StartServingStaticResponse(k404Response);
+
+  PrepareToResume();
+  download->Resume();
+  WaitForCompletion(download);
+
+  ASSERT_NO_FATAL_FAILURE(ReadAndVerifyFileContents(
+      parameters.pattern_generator_seed, parameters.size,
+      download->GetTargetFilePath()));
+}
+
+// If a resumption request results in a redirect, the response should be ignored
+// and the download should be marked as interrupted again.
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, RedirectWhileResume) {
+  TestDownloadRequestHandler request_handler(
+      GURL("http://example.com/first-url"));
+  TestDownloadRequestHandler::Parameters parameters =
+      TestDownloadRequestHandler::Parameters::WithSingleInterruption();
+  ++parameters.pattern_generator_seed;
+  request_handler.StartServing(parameters);
+
+  // We should never send a request to the decoy. If we do, the request will
+  // always succeed, which results in behavior that diverges from what we want,
+  // which is for the download to return to being interrupted.
+  TestDownloadRequestHandler decoy_request_handler(
+      GURL("http://example.com/decoy"));
+  decoy_request_handler.StartServing(TestDownloadRequestHandler::Parameters());
+
+  DownloadItem* download = StartDownloadAndReturnItem(
+      initiator_shell_for_resumption(), request_handler.url());
+  WaitForInterrupt(download);
+
+  // Upon resumption, the server starts responding with a redirect. This
+  // response should not be accepted.
+  request_handler.StartServingStaticResponse(
+      "HTTP/1.1 302 Redirect\r\n"
+      "Location: http://example.com/decoy\r\n"
+      "\r\n");
+  PrepareToResume();
+  download->Resume();
+  WaitForInterrupt(download);
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_SERVER_UNREACHABLE,
+            download->GetLastReason());
+
+  // Back to the original request handler. Resumption should now succeed, and
+  // use the partial data it had prior to the first interruption.
+  request_handler.StartServing(parameters);
+  download->Resume();
+  WaitForCompletion(download);
+
+  ASSERT_EQ(parameters.size, download->GetReceivedBytes());
+  ASSERT_EQ(parameters.size, download->GetTotalBytes());
+  ASSERT_NO_FATAL_FAILURE(ReadAndVerifyFileContents(
+      parameters.pattern_generator_seed, parameters.size,
+      download->GetTargetFilePath()));
+
+  // Characterization risk: The next portion of the test examines the requests
+  // that were sent out while downloading our resource. These requests
+  // correspond to the requests that were generated by the browser and the
+  // downloads system and may change as implementation details change.
+  TestDownloadRequestHandler::CompletedRequests requests;
+  request_handler.GetCompletedRequestInfo(&requests);
+
+  ASSERT_EQ(3u, requests.size());
+
+  // None of the request should have transferred the entire resource. The
+  // redirect response shows up as a response with 0 bytes transferred.
+  EXPECT_GT(parameters.size, requests[0].transferred_byte_count);
+  EXPECT_EQ(0, requests[1].transferred_byte_count);
+  EXPECT_GT(parameters.size, requests[2].transferred_byte_count);
+}
+
+// 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
+// discarding the partial state.
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, BadRangeHeader) {
+  TestDownloadRequestHandler request_handler;
+  TestDownloadRequestHandler::Parameters parameters =
+      TestDownloadRequestHandler::Parameters::WithSingleInterruption();
+  request_handler.StartServing(parameters);
+
+  DownloadItem* download = StartDownloadAndReturnItem(
+      initiator_shell_for_resumption(), request_handler.url());
+  WaitForInterrupt(download);
+
+  // Upon resumption, the server starts responding with a bad range header.
+  request_handler.StartServingStaticResponse(
+      "HTTP/1.1 206 Partial Content\r\n"
+      "Content-Range: bytes 1000000-2000000/3000000\r\n"
+      "\r\n");
+  PrepareToResume();
+  download->Resume();
+  WaitForInterrupt(download);
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT,
+            download->GetLastReason());
+
+  // Or this time, the server sends a response with an invalid Content-Range
+  // header.
+  request_handler.StartServingStaticResponse(
+      "HTTP/1.1 206 Partial Content\r\n"
+      "Content-Range: ooga-booga-booga-booga\r\n"
+      "\r\n");
+  download->Resume();
+  WaitForInterrupt(download);
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT,
+            download->GetLastReason());
+
+  // Or no Content-Range header at all.
+  request_handler.StartServingStaticResponse(
+      "HTTP/1.1 206 Partial Content\r\n"
+      "Some-Headers: ooga-booga-booga-booga\r\n"
+      "\r\n");
+  download->Resume();
+  WaitForInterrupt(download);
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT,
+            download->GetLastReason());
+
+  // Back to the original request handler. Resumption should now succeed, and
+  // use the partial data it had prior to the first interruption.
+  request_handler.StartServing(parameters);
+  download->Resume();
+  WaitForCompletion(download);
+
+  ASSERT_EQ(parameters.size, download->GetReceivedBytes());
+  ASSERT_EQ(parameters.size, download->GetTotalBytes());
+  ASSERT_NO_FATAL_FAILURE(ReadAndVerifyFileContents(
+      parameters.pattern_generator_seed, parameters.size,
+      download->GetTargetFilePath()));
+
+  // Characterization risk: The next portion of the test examines the requests
+  // that were sent out while downloading our resource. These requests
+  // correspond to the requests that were generated by the browser and the
+  // downloads system and may change as implementation details change.
+  TestDownloadRequestHandler::CompletedRequests requests;
+  request_handler.GetCompletedRequestInfo(&requests);
+
+  ASSERT_EQ(5u, requests.size());
+
+  // None of the request should have transferred the entire resource.
+  EXPECT_GT(parameters.size, requests[0].transferred_byte_count);
+  EXPECT_EQ(0, requests[1].transferred_byte_count);
+  EXPECT_EQ(0, requests[2].transferred_byte_count);
+  EXPECT_EQ(0, requests[3].transferred_byte_count);
+  EXPECT_GT(parameters.size, requests[4].transferred_byte_count);
+}
+
 // A partial resumption results in an HTTP 200 response. I.e. the server ignored
 // the range request and sent the entire resource instead. For If-Range requests
 // (as opposed to If-Match), the behavior for a precondition failure is also to