Report cases where the connection appears to close too early while transferring an HTTP body.

This should not change behavior at all, but will let us see how commonly these situations happen in the wild.

BUG=52847
TEST=Existing tests.


Review URL: http://codereview.chromium.org/9950023

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133208 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 90 insertions, 31 deletions

diff --git a/chrome/renderer/localized_error.cc b/chrome/renderer/localized_error.cc
index abb44a0..548b77c 100644
--- a/chrome/renderer/localized_error.cc
+++ b/chrome/renderer/localized_error.cc
@@ -211,6 +211,20 @@ const LocalizedErrorMap net_error_options[] = {
    IDS_ERRORPAGES_DETAILS_RESPONSE_HEADERS_MULTIPLE_LOCATION,
    SUGGEST_NONE,
   },
+  {net::ERR_CONTENT_LENGTH_MISMATCH,
+   IDS_ERRORPAGES_TITLE_NOT_AVAILABLE,
+   IDS_ERRORPAGES_HEADING_NOT_AVAILABLE,
+   IDS_ERRORPAGES_SUMMARY_NOT_AVAILABLE,
+   IDS_ERRORPAGES_DETAILS_CONNECTION_CLOSED,
+   SUGGEST_RELOAD,
+  },
+  {net::ERR_INCOMPLETE_CHUNKED_ENCODING,
+   IDS_ERRORPAGES_TITLE_NOT_AVAILABLE,
+   IDS_ERRORPAGES_HEADING_NOT_AVAILABLE,
+   IDS_ERRORPAGES_SUMMARY_NOT_AVAILABLE,
+   IDS_ERRORPAGES_DETAILS_CONNECTION_CLOSED,
+   SUGGEST_RELOAD,
+  },
   {net::ERR_SSL_PROTOCOL_ERROR,
    IDS_ERRORPAGES_TITLE_LOAD_FAILED,
    IDS_ERRORPAGES_HEADING_SSL_PROTOCOL_ERROR,
diff --git a/content/browser/download/download_resource_handler.cc b/content/browser/download/download_resource_handler.cc
index 7dc717b..bdcd1d7 100644
--- a/content/browser/download/download_resource_handler.cc
+++ b/content/browser/download/download_resource_handler.cc
@@ -289,13 +289,14 @@ void DownloadResourceHandler::OnResponseCompletedInternal(
   net::Error error_code = net::OK;
   if (status.status() == net::URLRequestStatus::FAILED)
     error_code = static_cast<net::Error>(status.error());  // Normal case.
-  // ERR_CONNECTION_CLOSED is allowed since a number of servers in the wild
-  // advertise a larger Content-Length than the amount of bytes in the message
-  // body, and then close the connection. Other browsers - IE8, Firefox 4.0.1,
-  // and Safari 5.0.4 - treat the download as complete in this case, so we
-  // follow their lead.
-  if (error_code == net::ERR_CONNECTION_CLOSED)
+  // ERR_CONTENT_LENGTH_MISMATCH and ERR_INCOMPLETE_CHUNKED_ENCODING are
+  // allowed since a number of servers in the wild close the connection too
+  // early by mistake. Other browsers - IE9, Firefox 11.0, and Safari 5.1.4 -
+  // treat downloads as complete in both cases, so we follow their lead.
+  if (error_code == net::ERR_CONTENT_LENGTH_MISMATCH ||
+      error_code == net::ERR_INCOMPLETE_CHUNKED_ENCODING) {
     error_code = net::OK;
+  }
   content::DownloadInterruptReason reason =
       content::ConvertNetErrorToInterruptReason(
         error_code, content::DOWNLOAD_INTERRUPT_FROM_NETWORK);
diff --git a/content/browser/renderer_host/resource_dispatcher_host_impl.cc b/content/browser/renderer_host/resource_dispatcher_host_impl.cc
index fcb5893..0ef88541 100644
--- a/content/browser/renderer_host/resource_dispatcher_host_impl.cc
+++ b/content/browser/renderer_host/resource_dispatcher_host_impl.cc
@@ -1920,21 +1920,28 @@ void ResourceDispatcherHostImpl::ResponseCompleted(net::URLRequest* request) {
   // If the load for a main frame has failed, track it in a histogram,
   // since it will probably cause the user to see an error page.
   if (!request->status().is_success() &&
-      info->GetResourceType() == ResourceType::MAIN_FRAME &&
       request->status().error() != net::ERR_ABORTED) {
-    // This enumeration has "2" appended to its name to distinguish it from
-    // its original version. We changed the buckets at one point (added
-    // guard buckets by using CustomHistogram::ArrayToCustomRanges).
-    UMA_HISTOGRAM_CUSTOM_ENUMERATION(
-        "Net.ErrorCodesForMainFrame2",
-        -request->status().error(),
-        base::CustomHistogram::ArrayToCustomRanges(
-            kAllNetErrorCodes, arraysize(kAllNetErrorCodes)));
-
-    if (request->url().SchemeIsSecure() &&
-        request->url().host() == "www.google.com") {
+    if (info->GetResourceType() == ResourceType::MAIN_FRAME) {
+      // This enumeration has "2" appended to its name to distinguish it from
+      // its original version. We changed the buckets at one point (added
+      // guard buckets by using CustomHistogram::ArrayToCustomRanges).
       UMA_HISTOGRAM_CUSTOM_ENUMERATION(
-          "Net.ErrorCodesForHTTPSGoogleMainFrame",
+          "Net.ErrorCodesForMainFrame2",
+          -request->status().error(),
+          base::CustomHistogram::ArrayToCustomRanges(
+              kAllNetErrorCodes, arraysize(kAllNetErrorCodes)));
+
+      if (request->url().SchemeIsSecure() &&
+          request->url().host() == "www.google.com") {
+        UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+            "Net.ErrorCodesForHTTPSGoogleMainFrame",
+            -request->status().error(),
+            base::CustomHistogram::ArrayToCustomRanges(
+                kAllNetErrorCodes, arraysize(kAllNetErrorCodes)));
+      }
+    } else {
+      UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+          "Net.ErrorCodesForSubresources",
           -request->status().error(),
           base::CustomHistogram::ArrayToCustomRanges(
               kAllNetErrorCodes, arraysize(kAllNetErrorCodes)));
diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h
index d308a6f..1cab980 100644
--- a/net/base/net_error_list.h
+++ b/net/base/net_error_list.h
@@ -519,6 +519,14 @@ NET_ERROR(SPDY_PING_FAILED, -352)
 // The request couldn't be completed on an HTTP pipeline. Client should retry.
 NET_ERROR(PIPELINE_EVICTION, -353)
 
+// The HTTP response body transferred fewer bytes than were advertised by the
+// Content-Length header when the connection is closed.
+NET_ERROR(CONTENT_LENGTH_MISMATCH, -354)
+
+// The HTTP response body is transferred with Chunked-Encoding, but the
+// terminating zero-length chunk was never sent when the connection is closed.
+NET_ERROR(INCOMPLETE_CHUNKED_ENCODING, -355)
+
 // The cache does not have the requested entry.
 NET_ERROR(CACHE_MISS, -400)
 
diff --git a/net/http/http_network_transaction_spdy2_unittest.cc b/net/http/http_network_transaction_spdy2_unittest.cc
index e9ef256..fe0ff78 100644
--- a/net/http/http_network_transaction_spdy2_unittest.cc
+++ b/net/http/http_network_transaction_spdy2_unittest.cc
@@ -6341,7 +6341,7 @@ TEST_F(HttpNetworkTransactionSpdy2Test, LargeContentLengthThenClose) {
 
   std::string response_data;
   rv = ReadTransaction(trans.get(), &response_data);
-  EXPECT_EQ(ERR_CONNECTION_CLOSED, rv);
+  EXPECT_EQ(ERR_CONTENT_LENGTH_MISMATCH, rv);
 }
 
 TEST_F(HttpNetworkTransactionSpdy2Test, UploadFileSmallerThanLength) {
diff --git a/net/http/http_network_transaction_spdy3_unittest.cc b/net/http/http_network_transaction_spdy3_unittest.cc
index 4c73e94..c35def2 100644
--- a/net/http/http_network_transaction_spdy3_unittest.cc
+++ b/net/http/http_network_transaction_spdy3_unittest.cc
@@ -6341,7 +6341,7 @@ TEST_F(HttpNetworkTransactionSpdy3Test, LargeContentLengthThenClose) {
 
   std::string response_data;
   rv = ReadTransaction(trans.get(), &response_data);
-  EXPECT_EQ(ERR_CONNECTION_CLOSED, rv);
+  EXPECT_EQ(ERR_CONTENT_LENGTH_MISMATCH, rv);
 }
 
 TEST_F(HttpNetworkTransactionSpdy3Test, UploadFileSmallerThanLength) {
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index 8098a18..31c7b73 100644
--- a/net/http/http_stream_parser.cc
+++ b/net/http/http_stream_parser.cc
@@ -662,12 +662,40 @@ int HttpStreamParser::DoReadBody() {
 }
 
 int HttpStreamParser::DoReadBodyComplete(int result) {
-  // If we didn't get a Content-Length and aren't using a chunked encoding,
-  // the only way to signal the end of a stream is to close the connection,
-  // so we don't treat that as an error, though in some cases we may not
-  // have completely received the resource.
-  if (result == 0 && !IsResponseBodyComplete() && CanFindEndOfResponse())
-    result = ERR_CONNECTION_CLOSED;
+  // When the connection is closed, there are numerous ways to interpret it.
+  //
+  //  - If a Content-Length header is present and the body contains exactly that
+  //    number of bytes at connection close, the response is successful.
+  //
+  //  - If a Content-Length header is present and the body contains fewer bytes
+  //    than promised by the header at connection close, it may indicate that
+  //    the connection was closed prematurely, or it may indicate that the
+  //    server sent an invalid Content-Length header. Unfortunately, the invalid
+  //    Content-Length header case does occur in practice and other browsers are
+  //    tolerant of it. We choose to treat it as an error for now, but the
+  //    download system treats it as a non-error, and URLRequestHttpJob also
+  //    treats it as OK if the Content-Length is the post-decoded body content
+  //    length.
+  //
+  //  - If chunked encoding is used and the terminating chunk has been processed
+  //    when the connection is closed, the response is successful.
+  //
+  //  - If chunked encoding is used and the terminating chunk has not been
+  //    processed when the connection is closed, it may indicate that the
+  //    connection was closed prematurely or it may indicate that the server
+  //    sent an invalid chunked encoding. We choose to treat it as
+  //    an invalid chunked encoding.
+  //
+  //  - If a Content-Length is not present and chunked encoding is not used,
+  //    connection close is the only way to signal that the response is
+  //    complete. Unfortunately, this also means that there is no way to detect
+  //    early close of a connection. No error is returned.
+  if (result == 0 && !IsResponseBodyComplete() && CanFindEndOfResponse()) {
+    if (chunked_decoder_.get())
+      result = ERR_INCOMPLETE_CHUNKED_ENCODING;
+    else
+      result = ERR_CONTENT_LENGTH_MISMATCH;
+  }
 
   // Filter incoming data if appropriate.  FilterBuf may return an error.
   if (result > 0 && chunked_decoder_.get()) {
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 2b8988b..cb7a64d 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -755,9 +755,9 @@ void URLRequestHttpJob::OnReadCompleted(int result) {
   read_in_progress_ = false;
 
   if (ShouldFixMismatchedContentLength(result))
-    result = 0;
+    result = OK;
 
-  if (result == 0) {
+  if (result == OK) {
     NotifyDone(URLRequestStatus());
   } else if (result < 0) {
     NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, result));
@@ -1098,7 +1098,8 @@ bool URLRequestHttpJob::ShouldFixMismatchedContentLength(int rv) const {
   // the uncompressed size.  Although this violates the HTTP spec we want to
   // support it (as IE and FireFox do), but *only* for an exact match.
   // See http://crbug.com/79694.
-  if (rv == net::ERR_CONNECTION_CLOSED) {
+  if (rv == net::ERR_CONTENT_LENGTH_MISMATCH ||
+      rv == net::ERR_INCOMPLETE_CHUNKED_ENCODING) {
     if (request_ && request_->response_headers()) {
       int64 expected_length = request_->response_headers()->GetContentLength();
       VLOG(1) << __FUNCTION__ << "() "
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 8633a48..12c90b7 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -1243,7 +1243,7 @@ TEST_F(URLRequestTestHTTP, GetZippedTest) {
             << " Parameter = \"" << test_file << "\"";
       } else {
         EXPECT_EQ(URLRequestStatus::FAILED, r.status().status());
-        EXPECT_EQ(-100, r.status().error())
+        EXPECT_EQ(ERR_CONTENT_LENGTH_MISMATCH, r.status().error())
             << " Parameter = \"" << test_file << "\"";
       }
     }