X-Chrome-Connected is stripped when it should not be in headers.

chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate_browsertest.cc

Index: chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate_browsertest.cc
diff --git a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate_browsertest.cc b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate_browsertest.cc
index 57388aaed81d6c3ad32f7421455e84acb653a35b..818b46b898a68a07446061b674332fc1d054a335 100644
--- a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate_browsertest.cc
+++ b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate_browsertest.cc
@@ -12,6 +12,7 @@
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/strings/string_util.h"
+#include "base/test/scoped_command_line.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/policy/cloud/policy_header_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -25,18 +26,31 @@
 #include "components/policy/core/common/cloud/policy_header_io_helper.h"
 #include "components/policy/core/common/cloud/policy_header_service.h"
 #include "components/policy/core/common/policy_switches.h"
+#include "components/prefs/pref_service.h"
+#include "components/signin/core/common/signin_pref_names.h"
+#include "components/signin/core/common/signin_switches.h"
 #include "content/public/browser/navigation_data.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/resource_dispatcher_host.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/browser_test_utils.h"
+#include "net/base/io_buffer.h"
 #include "net/http/http_request_headers.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_util.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/url_request/url_request.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 "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 using content::ResourceType;
+using testing::HasSubstr;
+using testing::Not;
 
 namespace {
 static const char kTestPolicyHeader[] = "test_header";
@@ -251,3 +265,223 @@ IN_PROC_BROWSER_TEST_F(ChromeResourceDispatcherHostDelegateBrowserTest,
     ui_test_utils::NavigateToURL(browser(), embedded_test_server()->base_url());
   }
 }
+
+namespace {
+
+// A mock URLRequestJob to that allows to inject specific response headers
+// and a specific response body to a network request.
+class MirrorMockURLRequestJob : public net::URLRequestJob {

[sky, 2016/08/18 19:29:50]

Can you use some of the test support classes in net for mocking rather than
rolling your own?

[Ramin Halavati, 2016/08/26 17:04:31]

Done.

+ public:
+  // Callback function on the UI thread to report a (URL, request headers)
+  // pair. Indicating that the |request headers| will be sent for the |URL|.
+  using ReportResponseHeadersOnUI =
+      base::Callback<void(const std::string&, const std::string&)>;
+
+  MirrorMockURLRequestJob(net::URLRequest* request,
+                          net::NetworkDelegate* network_delegate,
+                          const std::string& response_headers,
+                          const std::string& response_body,
+                          ReportResponseHeadersOnUI report_on_ui)
+      : net::URLRequestJob(request, network_delegate),
+        response_headers_(response_headers),
+        response_body_(response_body),
+        response_body_offset_(0),
+        report_on_ui_(report_on_ui),
+        weak_factory_(this) {}
+
+  void Start() override {
+    // Report the observed request headers on the UI thread.
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::Bind(report_on_ui_, request_->url().spec(),
+                   request_->extra_request_headers().ToString()));
+
+    // Start reading asynchronously so that all error reporting and data
+    // callbacks happen as they would for network requests.
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(&MirrorMockURLRequestJob::StartAsync,
+                              weak_factory_.GetWeakPtr()));
+  }
+
+  int ReadRawData(net::IOBuffer* buf, int buf_size) override {
+    size_t bytes_read =
+        std::min(static_cast<size_t>(buf_size),
+                 response_body_.length() - response_body_offset_);
+    memcpy(buf->data(), response_body_.c_str() + response_body_offset_,
+           bytes_read);
+    response_body_offset_ += bytes_read;
+    return bytes_read;
+  }
+
+  int GetResponseCode() const override {
+    net::HttpResponseInfo info;
+    GetResponseInfoConst(&info);
+    return info.headers->response_code();
+  }
+
+  void GetResponseInfo(net::HttpResponseInfo* info) override {
+    // Forward to private const version.
+    GetResponseInfoConst(info);
+  }
+
+ protected:
+  void StartAsync() {
+    if (!request_)
+      return;
+    set_expected_content_size(response_body_.length());
+    NotifyHeadersComplete();
+  }
+
+  // Private const version.
+  void GetResponseInfoConst(net::HttpResponseInfo* info) const {
+    // Send back mock headers.
+    std::string raw_headers;
+    raw_headers.append(response_headers_);
+    if (!response_body_.empty()) {
+      raw_headers.append(base::StringPrintf(
+          "Content-Length: %d\n", static_cast<int>(response_body_.length())));
+    }
+    info->headers =
+        new net::HttpResponseHeaders(net::HttpUtil::AssembleRawHeaders(
+            raw_headers.c_str(), static_cast<int>(raw_headers.length())));
+  }
+
+  const std::string response_headers_;  // All headers but 'Content-Length'.
+  const std::string response_body_;
+  size_t response_body_offset_;
+  const ReportResponseHeadersOnUI report_on_ui_;
+  base::WeakPtrFactory<MirrorMockURLRequestJob> weak_factory_;
+};
+
+// A URLRequestInterceptor to inject MirrorMockURLRequestJobs.
+class MirrorMockJobInterceptor : public net::URLRequestInterceptor {
+ public:
+  using ReportResponseHeadersOnUI =
+      MirrorMockURLRequestJob::ReportResponseHeadersOnUI;
+
+  MirrorMockJobInterceptor(const std::string& response_headers,
+                           const std::string& response_body,
+                           ReportResponseHeadersOnUI report_on_ui)
+      : response_headers_(response_headers),
+        response_body_(response_body),
+        report_on_ui_(report_on_ui){}
+  ~MirrorMockJobInterceptor() override = default;
+
+  // URLRequestInterceptor implementation
+  net::URLRequestJob* MaybeInterceptRequest(
+      net::URLRequest* request,
+      net::NetworkDelegate* network_delegate) const override {
+    return new MirrorMockURLRequestJob(request, network_delegate,
+                                       response_headers_, response_body_,
+                                       report_on_ui_);
+  }
+
+  static void Register(const GURL& url,
+                       const std::string& response_headers,
+                       const std::string& response_body,
+                       ReportResponseHeadersOnUI report_on_ui) {
+    EXPECT_TRUE(
+        content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+    net::URLRequestFilter::GetInstance()->AddHostnameInterceptor(
+        url.scheme(), url.host(),
+        base::WrapUnique(new MirrorMockJobInterceptor(
+            response_headers, response_body, report_on_ui)));
+  }
+
+  static void Unregister(const GURL& url) {
+    EXPECT_TRUE(
+        content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+    net::URLRequestFilter::GetInstance()->RemoveHostnameHandler(url.scheme(),
+                                                                url.host());
+  }
+
+ private:
+  std::string response_headers_;
+  std::string response_body_;
+  ReportResponseHeadersOnUI report_on_ui_;
+
+  DISALLOW_COPY_AND_ASSIGN(MirrorMockJobInterceptor);
+};
+
+void ReportRequestHeaders(std::map<std::string, std::string>* request_headers,
+                          const std::string& url,
+                          const std::string& headers) {
+  (*request_headers)[url] = headers;
+}
+
+void EmptyClosure() {}
+
+}  // namespace
+
+IN_PROC_BROWSER_TEST_F(ChromeResourceDispatcherHostDelegateBrowserTest,
+                       MirrorRequestHeader) {
+  // Verify that X-Chrome-Connected is appended on Google domains if account
+  // consistency is enabled, but also that it is stripped in case a request
+  // is redirected to a non-google domain.
+  // This is a regression test for crbug.com/588492.
+
+  // Enable account consistency so that mirror actually sets the
+  // X-Chrome-Connected header in requests to Google.
+  base::test::ScopedCommandLine command_line;
+  command_line.GetProcessCommandLine()->AppendSwitch(
+      switches::kEnableAccountConsistency);
+
+  browser()->profile()->GetPrefs()->SetString(prefs::kGoogleServicesUsername,
+                                              "user@gmail.com");
+  browser()->profile()->GetPrefs()->SetString(
+      prefs::kGoogleServicesUserAccountId, "account_id");
+
+  GURL google_com("http://www.google.com/");
+  GURL redirected_com("http://www.redirected.com/");
+
+  // The HTTP Request headers (i.e. the ones that are managed on the URLRequest
+  // layer, not on the URLRequestJob layer) sent from the browser are collected
+  // in this map. The keys are URLs the values the request headers.
+  std::map<std::string, std::string> request_headers;
+  MirrorMockURLRequestJob::ReportResponseHeadersOnUI report_request_headers =
+      base::Bind(&ReportRequestHeaders, &request_headers);
+
+  // Register MockJobInterceptors that redirect from google.com to
+  // redirected.com and serve static content on redirected.com.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&MirrorMockJobInterceptor::Register, google_com,
+                 "HTTP/1.1 301 Moved Permanently\n"
+                 "Location: http://www.redirected.com/\n",
+                 std::string(), report_request_headers));
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&MirrorMockJobInterceptor::Register, redirected_com,
+                 "HTTP/1.1 200 OK\n"
+                 "Content-type: text/plain\n",
+                 "success", report_request_headers));
+
+  // Perform a navigation to google.com to observe the request headers.
+  ui_test_utils::NavigateToURL(browser(), google_com);
+
+  // Cleanup before verifying the observed headers.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&MirrorMockJobInterceptor::Unregister, redirected_com));
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&MirrorMockJobInterceptor::Unregister, google_com));
+
+  // Ensure that the response headers have been reported to the UI thread
+  // and unregistration has been processed on the IO thread.
+  base::RunLoop run_loop;
+  content::BrowserThread::PostTaskAndReply(content::BrowserThread::IO,
+                                           FROM_HERE,
+                                           // Flush IO thread...
+                                           base::Bind(&EmptyClosure),
+                                           // ... and UI thread.
+                                           run_loop.QuitClosure());
+  run_loop.Run();
+
+  ASSERT_EQ(1u, request_headers.count(google_com.spec()));
+  EXPECT_THAT(request_headers[google_com.spec()],
+              HasSubstr("X-Chrome-Connected"));
+  ASSERT_EQ(1u, request_headers.count(redirected_com.spec()));
+  EXPECT_THAT(request_headers[redirected_com.spec()],
+              Not(HasSubstr("X-Chrome-Connected")));
+}