Don't process signed exchange response with attachment Content-Disposition header

According to RFC 6266, when the disposition type matches "attachment" the
browser should prompt the user to save the response locally.

But when NetworkService is enabled, Chrome doesn't save the signed exchange, but
shows the internal content of the signed exchange even if "attachment" type
Content-Disposition header is set.

When NetworkService is disabled, Chrome downloads the signed exchange as a file
but the loading animation doesn't stop.
This is because:
  - In MimeSniffingResourceHandler::MaybeStartInterception(),
    InterceptingResourceHandler steals the response.
  - So NavigationURLLoaderImpl::URLLoaderRequestController::OnComplete() will
    not be called forever after SignedExchangeRequestHandler intercepts the
    response.

To fix this problem, this CL adds download_utils::MustDownload() check in
ShouldHandleAsSignedHTTPExchange().
So SignedExchangeRequestHandler::MaybeCreateLoaderForResponse() will not
intercept the response when "attachment" type Content-Disposition header is set.

Bug: 896659
Change-Id: Ic1d4756f5823383d55144b0320a06851ca8ecc3e
Reviewed-on: https://chromium-review.googlesource.com/c/1293065Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605639}

content/browser/web_package/signed_exchange_request_handler_browsertest.cc

@@ -14,8 +14,10 @@
 #include "content/browser/frame_host/navigation_handle_impl.h"
 #include "content/browser/web_package/signed_exchange_handler.h"
 #include "content/browser/web_package/signed_exchange_utils.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/download_manager.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
@@ -31,6 +33,7 @@
 #include "content/public/test/test_navigation_throttle.h"
 #include "content/public/test/url_loader_interceptor.h"
 #include "content/shell/browser/shell.h"
+#include "content/shell/browser/shell_download_manager_delegate.h"
 #include "net/cert/cert_verify_result.h"
 #include "net/cert/mock_cert_verifier.h"
 #include "net/dns/mock_host_resolver.h"
@@ -406,6 +409,81 @@ INSTANTIATE_TEST_CASE_P(
         SignedExchangeRequestHandlerBrowserTestPrefetchParam::
             kPrefetchEnabled));
 
+class SignedExchangeRequestHandlerDownloadBrowserTest
+    : public SignedExchangeRequestHandlerBrowserTestBase {
+ public:
+  SignedExchangeRequestHandlerDownloadBrowserTest() = default;
+  ~SignedExchangeRequestHandlerDownloadBrowserTest() override = default;
+
+ protected:
+  class DownloadObserver : public DownloadManager::Observer {
+   public:
+    DownloadObserver(DownloadManager* manager) : manager_(manager) {
+      manager_->AddObserver(this);
+    }
+    ~DownloadObserver() override { manager_->RemoveObserver(this); }
+
+    void WaitUntilDownloadCreated() { run_loop_.Run(); }
+
+    const GURL& observed_url() const { return url_; }
+    const std::string& observed_content_disposition() const {
+      return content_disposition_;
+    }
+
+    // content::DownloadManager::Observer implementation.
+    void OnDownloadCreated(content::DownloadManager* manager,
+                           download::DownloadItem* item) override {
+      url_ = item->GetURL();
+      content_disposition_ = item->GetContentDisposition();
+      run_loop_.Quit();
+    }
+
+   private:
+    DownloadManager* manager_;
+    base::RunLoop run_loop_;
+    GURL url_;
+    std::string content_disposition_;
+  };
+
+  void SetUpOnMainThread() override {
+    SignedExchangeRequestHandlerBrowserTestBase::SetUpOnMainThread();
+    ASSERT_TRUE(downloads_directory_.CreateUniqueTempDir());
+    ShellDownloadManagerDelegate* delegate =
+        static_cast<ShellDownloadManagerDelegate*>(
+            shell()
+                ->web_contents()
+                ->GetBrowserContext()
+                ->GetDownloadManagerDelegate());
+    delegate->SetDownloadBehaviorForTesting(downloads_directory_.GetPath());
+  }
+
+ private:
+  base::ScopedTempDir downloads_directory_;
+};
+
+IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerDownloadBrowserTest,
+                       Download) {
+  std::unique_ptr<DownloadObserver> observer =
+      std::make_unique<DownloadObserver>(BrowserContext::GetDownloadManager(
+          shell()->web_contents()->GetBrowserContext()));
+
+  embedded_test_server()->ServeFilesFromSourceDirectory("content/test/data");
+  ASSERT_TRUE(embedded_test_server()->Start());
+  NavigateToURL(shell(), embedded_test_server()->GetURL("/empty.html"));
+
+  const std::string load_sxg =
+      "const iframe = document.createElement('iframe');"
+      "iframe.src = './sxg/test.example.org_test_download.sxg';"
+      "document.body.appendChild(iframe);";
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), load_sxg));
+  observer->WaitUntilDownloadCreated();
+  EXPECT_EQ(
+      embedded_test_server()->GetURL("/sxg/test.example.org_test_download.sxg"),
+      observer->observed_url());
+  EXPECT_EQ("attachment; filename=test.sxg",
+            observer->observed_content_disposition());
+}
+
 class SignedExchangeRequestHandlerRealCertVerifierBrowserTest
     : public SignedExchangeRequestHandlerBrowserTestBase {
  public:

content/browser/web_package/signed_exchange_utils.cc

@@ -11,6 +11,7 @@
 #include "base/strings/string_util.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
+#include "content/browser/loader/download_utils_impl.h"
 #include "content/browser/web_package/origins_list.h"
 #include "content/browser/web_package/signed_exchange_devtools_proxy.h"
 #include "content/browser/web_package/signed_exchange_error.h"
@@ -94,6 +95,10 @@ bool ShouldHandleAsSignedHTTPExchange(
     return false;
   if (!SignedExchangeRequestHandler::IsSupportedMimeType(head.mime_type))
     return false;
+  if (download_utils::MustDownload(request_url, head.headers.get(),
+                                   head.mime_type)) {
+    return false;
+  }
   if (base::FeatureList::IsEnabled(features::kSignedHTTPExchange))
     return true;
   if (!base::FeatureList::IsEnabled(features::kSignedHTTPExchangeOriginTrial))

content/test/data/sxg/generate-test-sxgs.sh

@@ -51,6 +51,9 @@ gen-signedexchange \
 # Generate the signed exchange for the invalid content-type test case.
 cp test.example.org_test.sxg test.example.org_test_invalid_content_type.sxg
 
+# Generate the signed exchange for downloading test case.
+cp test.example.org_test.sxg test.example.org_test_download.sxg
+
 # Generate the signed exchange file with invalid magic string
 xxd -p test.example.org_test.sxg |
   sed '1s/^737867312d623200/737867312d787800/' |

content/test/data/sxg/test.example.org_test_download.sxg.mock-http-headers

+HTTP/1.1 200 OK
+Content-Type: application/signed-exchange;v=b2
+Content-Disposition: attachment; filename=test.sxg