Save-Data should not be added to CORS Access-Control-Request-Headers

content/browser/service_worker/service_worker_browsertest.cc

 // Copyright 2014 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 <stddef.h>
 #include <stdint.h>
 #include <map>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string16.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "content/browser/fileapi/chrome_blob_storage_context.h"
 #include "content/browser/service_worker/embedded_worker_instance.h"
 #include "content/browser/service_worker/embedded_worker_registry.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_observer.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
@@ skipping 184 matching lines @@
   return std::move(http_response);
 }
 
 scoped_ptr<net::test_server::HttpResponse> VerifySaveDataHeaderNotInRequest(
     const net::test_server::HttpRequest& request) {
   auto it = request.headers.find("Save-Data");
   EXPECT_EQ(request.headers.end(), it);
   return make_scoped_ptr(new net::test_server::BasicHttpResponse());
 }
 
+scoped_ptr<net::test_server::HttpResponse>
+VerifySaveDataNotInAccessControlRequestHeader(
+    const net::test_server::HttpRequest& request) {
+  // Save-Data should be present.
+  auto it = request.headers.find("Save-Data");
+  EXPECT_NE(request.headers.end(), it);
+  EXPECT_EQ("on", it->second);
+
+  scoped_ptr<net::test_server::BasicHttpResponse> http_response(
+      new net::test_server::BasicHttpResponse());
+  if (request.method == net::test_server::METHOD_OPTIONS) {
+    // Access-Control-Request-Headers should contain 'X-Custom-Header' and not
+    // contain 'Save-Data'.
+    auto acrh_iter = request.headers.find("Access-Control-Request-Headers");
+    EXPECT_NE(request.headers.end(), acrh_iter);
+    EXPECT_NE(std::string::npos, acrh_iter->second.find("x-custom-header"));
+    EXPECT_EQ(std::string::npos, acrh_iter->second.find("save-data"));
+    http_response->AddCustomHeader("Access-Control-Allow-Headers",
+                                   acrh_iter->second);
+    http_response->AddCustomHeader("Access-Control-Allow-Methods", "GET");
+    http_response->AddCustomHeader("Access-Control-Allow-Origin", "*");
+  } else {
+    http_response->AddCustomHeader("Access-Control-Allow-Origin", "*");
+    http_response->set_content("PASS");
+  }
+  return std::move(http_response);
+}
+
 // The ImportsBustMemcache test requires that the imported script
 // would naturally be cached in blink's memcache, but the embedded
 // test server doesn't produce headers that allow the blink's memcache
 // to do that. This interceptor injects headers that give the import
 // an experiration far in the future.
 class LongLivedResourceInterceptor : public net::URLRequestInterceptor {
  public:
   explicit LongLivedResourceInterceptor(const std::string& body)
       : body_(body) {}
   ~LongLivedResourceInterceptor() override {}
@@ skipping 879 matching lines @@
   SetBrowserClientForTesting(old_client);
   shell()->Close();
 
   base::RunLoop run_loop;
   public_context()->UnregisterServiceWorker(
       embedded_test_server()->GetURL(kPageUrl),
       base::Bind(&ExpectResultAndRun, true, run_loop.QuitClosure()));
   run_loop.Run();
 }
 
+// Tests that when data saver is enabled and a cross-origin fetch by a webpage
+// is intercepted by a serviceworker, and the serviceworker does a fetch, the
+// preflight request does not have save-data in Access-Control-Request-Headers.
+IN_PROC_BROWSER_TEST_F(ServiceWorkerBrowserTest, CrossOriginFetchWithSaveData) {
+  const char kPageUrl[] = "/service_worker/fetch_cross_origin.html";
+  const char kWorkerUrl[] = "/service_worker/fetch_event_pass_through.js";
+  net::EmbeddedTestServer cross_origin_server;
+  cross_origin_server.ServeFilesFromSourceDirectory("content/test/data");
+  cross_origin_server.RegisterRequestHandler(
+      base::Bind(&VerifySaveDataNotInAccessControlRequestHeader));
+  cross_origin_server.Start();
+
+  MockContentBrowserClient content_browser_client;
+  content_browser_client.set_data_saver_enabled(true);
+  ContentBrowserClient* old_client =
+      SetBrowserClientForTesting(&content_browser_client);
+  shell()->web_contents()->GetRenderViewHost()->OnWebkitPreferencesChanged();
+  scoped_refptr<WorkerActivatedObserver> observer =
+      new WorkerActivatedObserver(wrapper());
+  observer->Init();
+  public_context()->RegisterServiceWorker(
+      embedded_test_server()->GetURL(kPageUrl),
+      embedded_test_server()->GetURL(kWorkerUrl),
+      base::Bind(&ExpectResultAndRun, true, base::Bind(&base::DoNothing)));
+  observer->Wait();
+
+  NavigateToURL(shell(),
+                embedded_test_server()->GetURL(base::StringPrintf(
+                    "%s?%s", kPageUrl,
+                    cross_origin_server.GetURL("/cross_origin_request.html")
+                        .spec()
+                        .c_str())));
+  const base::string16 title = base::ASCIIToUTF16("PASS");
+  TitleWatcher title_watcher(shell()->web_contents(), title);

[Paweł Hajdan Jr., 2016/04/06 20:15:46]

Shouldn't TitleWatcher be instantiated *before* the test triggers navigation?

My understand is that was the point of separating its construction from
WaitAndGetTitleCall. Otherwise you wouldn't need an object.

And the general idea with such design is to avoid race conditions, i.e. to
register for notifications before they might be triggered.

[Raj, 2016/04/06 22:04:10]

Done.

+  EXPECT_EQ(title, title_watcher.WaitAndGetTitle());
+
+  SetBrowserClientForTesting(old_client);
+  shell()->Close();
+
+  base::RunLoop run_loop;
+  public_context()->UnregisterServiceWorker(
+      embedded_test_server()->GetURL(kPageUrl),
+      base::Bind(&ExpectResultAndRun, true, run_loop.QuitClosure()));
+  run_loop.Run();
+}
+
 IN_PROC_BROWSER_TEST_F(ServiceWorkerBrowserTest,
                        FetchPageWithSaveDataPassThroughOnFetch) {
   const char kPageUrl[] = "/service_worker/pass_through_fetch.html";
   const char kWorkerUrl[] = "/service_worker/fetch_event_pass_through.js";
   MockContentBrowserClient content_browser_client;
   content_browser_client.set_data_saver_enabled(true);
   ContentBrowserClient* old_client =
       SetBrowserClientForTesting(&content_browser_client);
   shell()->web_contents()->GetRenderViewHost()->OnWebkitPreferencesChanged();
   scoped_refptr<WorkerActivatedObserver> observer =
@@ skipping 356 matching lines @@
   ASSERT_EQ(SERVICE_WORKER_OK, status);
   // Stop the worker.
   StopWorker(SERVICE_WORKER_OK);
   // Restart the worker.
   StartWorker(SERVICE_WORKER_OK);
   // Stop the worker.
   StopWorker(SERVICE_WORKER_OK);
 }
 
 }  // namespace content