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 140 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)) {
-    request->SetExtraRequestHeaderByName(
-        "Range", base::StringPrintf("bytes=%" PRId64 "-", params->offset()),
-        true);
+  // Add "Range" and "If-Range" request header fields if we ask for a range.
+  if ((params->offset() > 0 ||
+       params->length() > DownloadSaveInfo::kLengthFullContent) &&

[asanka, 2017/02/01 22:51:17]

This expression requires that kLengthFullContent be 0. Any other value would be
nonsensical.

Perhaps just check that params->length() != kLengthFullContent ?

[xingliu, 2017/02/02 05:46:36]

Make sense, changed to != check in this file.

+      (has_etag || has_last_modified)) {
+    std::string range_header =
+        params->length() > DownloadSaveInfo::kLengthFullContent
+            ? base::StringPrintf("bytes=%" PRId64 "-%" PRId64, params->offset(),
+                                 params->offset() + params->length() - 1)
+            : base::StringPrintf("bytes=%" PRId64 "-", params->offset());
 
-    // In accordance with RFC 2616 Section 14.27, use If-Range to specify that
+    request->SetExtraRequestHeaderByName("Range", range_header, true);
+
+    // 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);
   }
 
@@ skipping 113 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 221 matching lines @@
   switch (http_headers.response_code()) {
     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/01 22:51:17]

Since we specified an If-Range header, we specifically asked the server to send
the entire entity if the validators didn't match.

We are trying to do two things here:

  * Assuming we have a prefix of the entity, we can send a request for the
remainder. This is Range: bytes=123- . If there's a mismatch in our validators,
then the server send the entire resource and we discard the prefix and store the
entity.

  * With-or-without a prefix, we want to fetch some random part of the entity.
This is 'Range: bytes=123-345'. If validation fails, then we don't necessarily
want to revert to fetching the entire entity. Instead we fail and allow the
caller to figure out how they want to handle this.

In the case of multiple donwload jobs, you'd now be able to ensure that only the
primary job will fetch the entire resource. We really don't want each job to
fetch full separate copies :-)

So, with apologies, the case of the length-capped fetch should be handled as:

   Range: bytes=123-235
   If-Match: etag

  or

   Range: bytes=123-234
   If-Unmodified-Since: last-modified

This way, the server will fail the request with a HTTP 412 rather than send the
entire response body.

[xingliu, 2017/02/02 05:46:36]

Thanks for looking into RFC details here.

Yeah, I'm thinking of If-Range is a bit weird for a slice. It can work but need
to drop the request somewhere when the server gives 200 response for a slice
like "Range:bytes=x-y".

Changed to send If-Match and If-Unmodified-Since together. In RFC, these two
fields can stack together.

+      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