Fix short error responses sniffed as downloads hanging.

chrome/browser/net/errorpage_browsertest.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 <memory>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
@@ skipping 40 matching lines @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "net/base/filename_util.h"
 #include "net/base/net_errors.h"
 #include "net/http/failing_http_transaction_factory.h"
 #include "net/http/http_cache.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
 #include "net/test/url_request/url_request_failed_job.h"
 #include "net/test/url_request/url_request_mock_http_job.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_filter.h"
 #include "net/url_request/url_request_interceptor.h"
 #include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_test_job.h"
 #include "net/url_request/url_request_test_util.h"
 #include "ui/base/l10n/l10n_util.h"
@@ skipping 245 matching lines @@
 
   // Add a mock for the search engine the error page will use.
   base::FilePath root_http;
   PathService::Get(chrome::DIR_TEST_DATA, &root_http);
   net::URLRequestFilter::GetInstance()->AddHostnameInterceptor(
       search_url.scheme(), search_url.host(),
       net::URLRequestMockHTTPJob::CreateInterceptorForSingleFile(
           root_http.AppendASCII("title3.html")));
 }
 
+// When it sees a request for |path|, returns a 500 response with a body that
+// will be sniffed as binary/octet-stream.
+std::unique_ptr<net::test_server::HttpResponse> Return500WithBinaryBody(
+    const std::string& path,
+    const net::test_server::HttpRequest& request) {
+  if (path != request.relative_url)
+    return nullptr;
+  return std::unique_ptr<net::test_server::HttpResponse>(
+      new net::test_server::RawHttpResponse("HTTP/1.1 500 Server Sad :(",

[Charlie Harrison, 2017/07/06 18:09:37]

Can you use base::MakeUnique here to avoid raw new?

[mmenke, 2017/07/06 18:17:19]

No - MakeUnique doesn't handle upcasting (I'm creating a RawHttpResponse, and
returning it as an HttpResonse).  std::move does handle that, so I could make a
unique_ptr on the stack and then std::move it, but I don't think that's any
better, since std::moving return values isn't the preferred way to do things.

[Charlie Harrison, 2017/07/06 18:23:09]

I was thinking of just returning the base::MakeUnique<RawHttpResponse>(...),
since I thought the type conversion would be implicit.

+                                            "\x01"));
+}
+
 class ErrorPageTest : public InProcessBrowserTest {
  public:
   enum HistoryNavigationDirection {
     HISTORY_NAVIGATE_BACK,
     HISTORY_NAVIGATE_FORWARD,
   };
 
   ErrorPageTest() : link_doctor_interceptor_(NULL) {}
   ~ErrorPageTest() override {}
 
@@ skipping 1214 matching lines @@
 IN_PROC_BROWSER_TEST_F(ErrorPageWithHttp09OnNonDefaultPortsTest,
                        Http09WeirdPortEnabled) {
   const char kHttp09Response[] = "JumboShrimp";
   ASSERT_TRUE(embedded_test_server()->Start());
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(std::string("/echo-raw?") +
                                                 kHttp09Response));
   EXPECT_TRUE(IsDisplayingText(browser(), kHttp09Response));
 }
 
+// Checks that when an HTTP error page is sniffed as a download, an error page
+// is displayed. This tests the particular case in which the response body
+// is small enough that the entire response must be read before its MIME type
+// can be determined.
+IN_PROC_BROWSER_TEST_F(ErrorPageTest, SniffSmallHttpErrorResponseAsDownload) {
+  const char kErrorPath[] = "/foo";
+  embedded_test_server()->RegisterRequestHandler(
+      base::Bind(&Return500WithBinaryBody, kErrorPath));
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL(kErrorPath));
+
+  ExpectDisplayingLocalErrorPage(browser(), net::ERR_INVALID_RESPONSE);
+}
+
 }  // namespace