Fix short error responses sniffed as downloads hanging.

chrome/browser/net/errorpage_browsertest.cc

Index: chrome/browser/net/errorpage_browsertest.cc
diff --git a/chrome/browser/net/errorpage_browsertest.cc b/chrome/browser/net/errorpage_browsertest.cc
index 2c0174ea2a2aad6801e098666eec43a660ab68ea..cbddad76bc20126878613c79af67af1d30a4815c 100644
--- a/chrome/browser/net/errorpage_browsertest.cc
+++ b/chrome/browser/net/errorpage_browsertest.cc
@@ -58,6 +58,8 @@
 #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"
@@ -323,6 +325,18 @@ void InstallMockInterceptors(
           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 {
@@ -1557,4 +1571,20 @@ IN_PROC_BROWSER_TEST_F(ErrorPageWithHttp09OnNonDefaultPortsTest,
   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