Range request support for parallel download in DownloadRequestCore.

content/browser/download/download_request_core.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 "content/browser/download/download_request_core.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ skipping 19 matching lines @@
 #include "content/public/browser/download_manager_delegate.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/web_contents.h"
 #include "device/power_save_blocker/power_save_blocker.h"
 #include "net/base/elements_upload_data_stream.h"
 #include "net/base/io_buffer.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/base/upload_bytes_element_reader.h"
+#include "net/http/http_request_headers.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_request_context.h"
 
 namespace content {
 
 namespace {
 
 // This is a UserData::Data that will be attached to a URLRequest as a
 // side-channel for passing download parameters.
@@ skipping 101 matching lines @@
     else
       load_flags |= net::LOAD_SKIP_CACHE_VALIDATION;
   } else {
     load_flags |= net::LOAD_DISABLE_CACHE;
   }
   request->SetLoadFlags(load_flags);
 
   bool has_last_modified = !params->last_modified().empty();
   bool has_etag = !params->etag().empty();
 
-  // If we've asked for a range, we want to make sure that we only get that
-  // range if our current copy of the information is good.  We shouldn't be
-  // asked to continue if we don't have a verifier.
-  DCHECK(params->offset() == 0 || has_etag || has_last_modified);
+  // Strong validator(i.e. etag or last modified) is required in range requests
+  // for download resumption and parallel download.
+  DCHECK((params->offset() == 0 &&
+          params->length() == DownloadSaveInfo::kLengthFullContent) ||
+         has_etag || has_last_modified);
 
-  // If we're not at the beginning of the file, retrieve only the remaining
-  // portion.
-  if (params->offset() > 0 && (has_etag || has_last_modified)) {
+  // Add "Range" and "If-Range" request header fields if the range request is to
+  // retrieve bytes from {offset} to the end of the file.
+  // E.g. "Range:bytes=50-".
+  if (params->offset() > 0 &&
+      params->length() == DownloadSaveInfo::kLengthFullContent &&
+      (has_etag || has_last_modified)) {
     request->SetExtraRequestHeaderByName(
-        "Range", base::StringPrintf("bytes=%" PRId64 "-", params->offset()),
-        true);
+        net::HttpRequestHeaders::kRange,
+        base::StringPrintf("bytes=%" PRId64 "-", params->offset()), true);
 
-    // In accordance with RFC 2616 Section 14.27, use If-Range to specify that
+    // In accordance with RFC 7233 Section 3.2, use If-Range to specify that
     // the server return the entire entity if the validator doesn't match.
     // Last-Modified can be used in the absence of ETag as a validator if the
     // response headers satisfied the HttpUtil::HasStrongValidators() predicate.
     //
     // This function assumes that HasStrongValidators() was true and that the
     // ETag and Last-Modified header values supplied are valid.
     request->SetExtraRequestHeaderByName(
         "If-Range", has_etag ? params->etag() : params->last_modified(), true);
   }
 
+  // Add "Range", "If-Match", and "If-Unmodified-Since" for range requests with
+  // last byte position specified. e.g. "Range:bytes=50-59". If the file is
+  // updated on the server, we should get http 412 response code.
+  if (params->length() != DownloadSaveInfo::kLengthFullContent &&
+      (has_etag || has_last_modified)) {
+    request->SetExtraRequestHeaderByName(
+        net::HttpRequestHeaders::kRange,
+        base::StringPrintf("bytes=%" PRId64 "-%" PRId64, params->offset(),
+                           params->offset() + params->length() - 1),
+        true);
+    if (has_etag)
+      request->SetExtraRequestHeaderByName("If-Match", params->etag(), true);

[asanka, 2017/02/03 22:16:22]

Could you add "If-Match"/"If-Unmodified-Since" to HttpRequestHeaders? I can
stamp that change along with this one.

Then you can use the new constants here and in tests.

[xingliu, 2017/02/06 19:31:39]

Done.

+    // According to RFC 7232 section 3.4, "If-Unmodified-Since" is mainly for
+    // old servers that didn't implement "If-Match" and must be ignored when
+    // "If-Match" presents.
+    if (has_last_modified)
+      request->SetExtraRequestHeaderByName("If-Unmodified-Since",
+                                           params->last_modified(), true);
+  }
+
   // Downloads are treated as top level navigations. Hence the first-party
   // origin for cookies is always based on the target URL and is updated on
   // redirects.
   request->set_first_party_for_cookies(params->url());
   request->set_first_party_url_policy(
       net::URLRequest::UPDATE_FIRST_PARTY_URL_ON_REDIRECT);
   request->set_initiator(params->initiator());
 
   for (const auto& header : params->request_headers())
     request->SetExtraRequestHeaderByName(header.first, header.second,
@@ skipping 103 matching lines @@
 
   if (!override_mime_type.empty())
     create_info->mime_type = override_mime_type;
   else
     request()->GetMimeType(&create_info->mime_type);
 
   // Get the last modified time and etag.
   const net::HttpResponseHeaders* headers = request()->response_headers();
   if (headers) {
     if (headers->HasStrongValidators()) {
-      // If we don't have strong validators as per RFC 2616 section 13.3.3, then
+      // If we don't have strong validators as per RFC 7232 section 2, then
       // we neither store nor use them for range requests.
       if (!headers->EnumerateHeader(nullptr, "Last-Modified",
                                     &create_info->last_modified))
         create_info->last_modified.clear();
       if (!headers->EnumerateHeader(nullptr, "ETag", &create_info->etag))
         create_info->etag.clear();
     }
 
     // Grab the first content-disposition header.  There may be more than one,
     // though as of this writing, the network stack ensures if there are, they
@@ skipping 211 matching lines @@
   DownloadInterruptReason reason = ConvertNetErrorToInterruptReason(
       error_code, DOWNLOAD_INTERRUPT_FROM_NETWORK);
 
   return reason;
 }
 
 // static
 DownloadInterruptReason DownloadRequestCore::HandleSuccessfulServerResponse(
     const net::HttpResponseHeaders& http_headers,
     DownloadSaveInfo* save_info) {
   switch (http_headers.response_code()) {

[asanka, 2017/02/03 22:16:23]

We should now handle HTTP 412 here (net::HTTP_PRECONDITION_FAILED) which could
conceivably map to the obsoleted PRECONDITION_FAILED interrupt reason
(https://cs.chromium.org/chromium/src/content/public/browser/download_interrup...).

[xingliu, 2017/02/06 19:31:39]

Can we do it later in a separate patch? Since it needs to touch extension api,
where extensions::api::downloads is tied to download_interrupt_reasons.h.
 

I'll put a TODO here.

     case -1:  // Non-HTTP request.
     case net::HTTP_OK:
     case net::HTTP_NON_AUTHORITATIVE_INFORMATION:
     case net::HTTP_PARTIAL_CONTENT:
       // Expected successful codes.
       break;
 
     case net::HTTP_CREATED:
     case net::HTTP_ACCEPTED:
-      // Per RFC 2616 the entity being transferred is metadata about the
+      // Per RFC 7231 the entity being transferred is metadata about the
       // resource at the target URL and not the resource at that URL (or the
       // resource that would be at the URL once processing is completed in the
       // case of HTTP_ACCEPTED). However, we currently don't have special
       // handling for these response and they are downloaded the same as a
       // regular response.
       break;
 
     case net::HTTP_NO_CONTENT:
     case net::HTTP_RESET_CONTENT:
-    // These two status codes don't have an entity (or rather RFC 2616
+    // These two status codes don't have an entity (or rather RFC 7231
     // requires that there be no entity). They are treated the same as the
     // resource not being found since there is no entity to download.
 
     case net::HTTP_NOT_FOUND:
       return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
       break;
 
     case net::HTTP_REQUESTED_RANGE_NOT_SATISFIABLE:
       // Retry by downloading from the start automatically:
       // If we haven't received data when we get this error, we won't.
       return DOWNLOAD_INTERRUPT_REASON_SERVER_NO_RANGE;
       break;
     case net::HTTP_UNAUTHORIZED:
     case net::HTTP_PROXY_AUTHENTICATION_REQUIRED:
       // Server didn't authorize this request.
       return DOWNLOAD_INTERRUPT_REASON_SERVER_UNAUTHORIZED;
       break;
     case net::HTTP_FORBIDDEN:
       // Server forbids access to this resource.
       return DOWNLOAD_INTERRUPT_REASON_SERVER_FORBIDDEN;
       break;
     default:  // All other errors.
       // Redirection and informational codes should have been handled earlier
       // in the stack.
       DCHECK_NE(3, http_headers.response_code() / 100);
       DCHECK_NE(1, http_headers.response_code() / 100);
       return DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED;
   }
 
-  if (save_info && save_info->offset > 0) {
-    // The caller is expecting a partial response.
+  // The caller is expecting a partial response.
+  if (save_info && (save_info->offset > 0 || save_info->length > 0)) {
+    if (http_headers.response_code() != net::HTTP_PARTIAL_CONTENT) {
+      // Server should send partial content when we ask for a range with last
+      // byte position specified. e.g. "Range:bytes=50-99".

[asanka, 2017/02/03 22:16:22]

Nit: This is a result of the If-Match/If-Unmodified-Since + Range headers.

[xingliu, 2017/02/06 19:31:40]

Done.

+      if (save_info->length != DownloadSaveInfo::kLengthFullContent)
+        return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
 
-    if (http_headers.response_code() != net::HTTP_PARTIAL_CONTENT) {
-      // Requested a partial range, but received the entire response.
+      // Requested a partial range, but received the entire response, when
+      // the range header is "Range:bytes={offset}-".
       save_info->offset = 0;
       save_info->hash_of_partial_file.clear();
       save_info->hash_state.reset();
       return DOWNLOAD_INTERRUPT_REASON_NONE;
     }
 
     int64_t first_byte = -1;
     int64_t last_byte = -1;
     int64_t length = -1;
     if (!http_headers.GetContentRangeFor206(&first_byte, &last_byte, &length))
       return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
     DCHECK_GE(first_byte, 0);
 
-    if (first_byte != save_info->offset) {
+    if (first_byte != save_info->offset ||
+        (save_info->length > 0 &&
+         last_byte != save_info->offset + save_info->length - 1)) {
       // The server returned a different range than the one we requested. Assume
       // the response is bad.
       //
       // In the future we should consider allowing offsets that are less than
       // the offset we've requested, since in theory we can truncate the partial
       // file at the offset and continue.
       return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
     }
 
     return DOWNLOAD_INTERRUPT_REASON_NONE;
   }
 
   if (http_headers.response_code() == net::HTTP_PARTIAL_CONTENT)
     return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
 
   return DOWNLOAD_INTERRUPT_REASON_NONE;
 }
 
 }  // namespace content