Switch variations http headers to only be reported over https.

BUG=672950

Review-Url: https://codereview.chromium.org/2563103002
Cr-Commit-Position: refs/heads/master@{#438200}

components/variations/net/variations_http_headers.cc

@@ -7,6 +7,7 @@
 #include <stddef.h>
 
 #include "base/macros.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
 #include "components/google/core/browser/google_util.h"
 #include "components/variations/variations_http_header_provider.h"
@@ -39,6 +40,50 @@ const char* kHostsToSetHeadersFor[] = {
 const char kChromeUMAEnabled[] = "X-Chrome-UMA-Enabled";
 const char kClientData[] = "X-Client-Data";
 
+// The result of checking if a URL should have variations headers appended.
+// This enum is used to record UMA histogram values, and should not be
+// reordered.
+enum URLValidationResult {
+  INVALID_URL,
+  NOT_HTTPS,
+  NOT_GOOGLE_DOMAIN,
+  SHOULD_APPEND,
+  URL_VALIDATION_RESULT_SIZE,
+};
+
+// Checks whether headers should be appended to the |url|, based on the domain
+// of |url|. |url| is assumed to be valid, and to have the https scheme.
+bool IsGoogleDomain(const GURL& url) {
+  if (google_util::IsGoogleDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
+                                     google_util::ALLOW_NON_STANDARD_PORTS)) {
+    return true;
+  }
+  if (google_util::IsYoutubeDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
+                                      google_util::ALLOW_NON_STANDARD_PORTS)) {
+    return true;
+  }
+
+  // Some domains don't have international TLD extensions, so testing for them
+  // is very straight forward.
+  const std::string host = url.host();
+  for (size_t i = 0; i < arraysize(kSuffixesToSetHeadersFor); ++i) {
+    if (base::EndsWith(host, kSuffixesToSetHeadersFor[i],
+                       base::CompareCase::INSENSITIVE_ASCII))
+      return true;
+  }
+  for (size_t i = 0; i < arraysize(kHostsToSetHeadersFor); ++i) {
+    if (base::LowerCaseEqualsASCII(host, kHostsToSetHeadersFor[i]))
+      return true;
+  }
+
+  return false;
+}
+
+void LogUrlValidationHistogram(URLValidationResult result) {
+  UMA_HISTOGRAM_ENUMERATION("Variations.Headers.URLValidationResult", result,
+                            URL_VALIDATION_RESULT_SIZE);
+}
+
 }  // namespace
 
 void AppendVariationHeaders(const GURL& url,
@@ -82,29 +127,21 @@ namespace internal {
 
 // static
 bool ShouldAppendVariationHeaders(const GURL& url) {
-  if (google_util::IsGoogleDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
-                                     google_util::ALLOW_NON_STANDARD_PORTS)) {
-    return true;
+  if (!url.is_valid()) {
+    LogUrlValidationHistogram(INVALID_URL);
+    return false;
   }
-
-  if (!url.is_valid() || !url.SchemeIsHTTPOrHTTPS())
+  if (!url.SchemeIs("https")) {
+    LogUrlValidationHistogram(NOT_HTTPS);
     return false;
-
-  // Some domains don't have international TLD extensions, so testing for them
-  // is very straight forward.
-  const std::string host = url.host();
-  for (size_t i = 0; i < arraysize(kSuffixesToSetHeadersFor); ++i) {
-    if (base::EndsWith(host, kSuffixesToSetHeadersFor[i],
-                       base::CompareCase::INSENSITIVE_ASCII))
-      return true;
   }
-  for (size_t i = 0; i < arraysize(kHostsToSetHeadersFor); ++i) {
-    if (base::LowerCaseEqualsASCII(host, kHostsToSetHeadersFor[i]))
-      return true;
+  if (!IsGoogleDomain(url)) {
+    LogUrlValidationHistogram(NOT_GOOGLE_DOMAIN);
+    return false;
   }
 
-  return google_util::IsYoutubeDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
-                                         google_util::ALLOW_NON_STANDARD_PORTS);
+  LogUrlValidationHistogram(SHOULD_APPEND);
+  return true;
 }
 
 }  // namespace internal

components/variations/net/variations_http_headers.h

@@ -35,7 +35,8 @@ std::set<std::string> GetVariationHeaderNames();
 namespace internal {
 
 // Checks whether variation headers should be appended to requests to the
-// specified |url|. Returns true for google.<TLD> and youtube.<TLD> URLs.
+// specified |url|. Returns true for google.<TLD> and youtube.<TLD> URLs with
+// the https scheme.
 bool ShouldAppendVariationHeaders(const GURL& url);
 
 }  // namespace internal

components/variations/net/variations_http_headers_unittest.cc

@@ -17,94 +17,133 @@ TEST(VariationsHttpHeadersTest, ShouldAppendHeaders) {
     const char* url;
     bool should_append_headers;
   } cases[] = {
-      {"http://google.com", true},
-      {"http://www.google.com", true},
-      {"http://m.google.com", true},
-      {"http://google.ca", true},
+      {"http://google.com", false},
+      {"https://google.com", true},
+      {"http://www.google.com", false},
+      {"https://www.google.com", true},
+      {"http://m.google.com", false},
+      {"https://m.google.com", true},
+      {"http://google.ca", false},
       {"https://google.ca", true},
-      {"http://google.co.uk", true},
-      {"http://google.co.uk:8080/", true},
-      {"http://www.google.co.uk:8080/", true},
-      {"http://google", false},
+      {"http://google.co.uk", false},
+      {"https://google.co.uk", true},
+      {"http://google.co.uk:8080/", false},
+      {"https://google.co.uk:8080/", true},
+      {"http://www.google.co.uk:8080/", false},
+      {"https://www.google.co.uk:8080/", true},
+      {"https://google", false},
 
-      {"http://youtube.com", true},
-      {"http://www.youtube.com", true},
-      {"http://www.youtube.ca", true},
-      {"http://www.youtube.co.uk:8080/", true},
+      {"http://youtube.com", false},
+      {"https://youtube.com", true},
+      {"http://www.youtube.com", false},
       {"https://www.youtube.com", true},
-      {"http://youtube", false},
+      {"http://www.youtube.ca", false},
+      {"https://www.youtube.ca", true},
+      {"http://www.youtube.co.uk:8080/", false},
+      {"https://www.youtube.co.uk:8080/", true},
+      {"https://youtube", false},
 
-      {"http://www.yahoo.com", false},
+      {"https://www.yahoo.com", false},
 
-      {"http://ad.doubleclick.net", true},
+      {"http://ad.doubleclick.net", false},
+      {"https://ad.doubleclick.net", true},
       {"https://a.b.c.doubleclick.net", true},
       {"https://a.b.c.doubleclick.net:8081", true},
-      {"http://www.doubleclick.com", true},
-      {"http://www.doubleclick.org", false},
+      {"http://www.doubleclick.com", false},
+      {"https://www.doubleclick.com", true},
+      {"https://www.doubleclick.org", false},
       {"http://www.doubleclick.net.com", false},
       {"https://www.doubleclick.net.com", false},
 
-      {"http://ad.googlesyndication.com", true},
+      {"http://ad.googlesyndication.com", false},
+      {"https://ad.googlesyndication.com", true},
       {"https://a.b.c.googlesyndication.com", true},
       {"https://a.b.c.googlesyndication.com:8080", true},
       {"http://www.doubleclick.edu", false},
       {"http://www.googlesyndication.com.edu", false},
       {"https://www.googlesyndication.com.com", false},
 
-      {"http://www.googleadservices.com", true},
-      {"http://www.googleadservices.com:8080", true},
+      {"http://www.googleadservices.com", false},
       {"https://www.googleadservices.com", true},
+      {"http://www.googleadservices.com:8080", false},
+      {"https://www.googleadservices.com:8080", true},
       {"https://www.internal.googleadservices.com", true},
       {"https://www2.googleadservices.com", true},
       {"https://www.googleadservices.org", false},
       {"https://www.googleadservices.com.co.uk", false},
 
-      {"http://WWW.ANDROID.COM", true},
-      {"http://www.android.com", true},
-      {"http://www.doubleclick.com", true},
-      {"http://www.doubleclick.net", true},
-      {"http://www.ggpht.com", true},
-      {"http://www.googleadservices.com", true},
-      {"http://www.googleapis.com", true},
-      {"http://www.googlesyndication.com", true},
-      {"http://www.googleusercontent.com", true},
-      {"http://www.googlevideo.com", true},
-      {"http://ssl.gstatic.com", true},
-      {"http://www.gstatic.com", true},
-      {"http://www.ytimg.com", true},
-      {"http://wwwytimg.com", false},
-      {"http://ytimg.com", false},
+      {"http://WWW.ANDROID.COM", false},
+      {"https://WWW.ANDROID.COM", true},
+      {"http://www.android.com", false},
+      {"https://www.android.com", true},
+      {"http://www.doubleclick.com", false},
+      {"https://www.doubleclick.com", true},
+      {"http://www.doubleclick.net", false},
+      {"https://www.doubleclick.net", true},
+      {"http://www.ggpht.com", false},
+      {"https://www.ggpht.com", true},
+      {"http://www.googleadservices.com", false},
+      {"https://www.googleadservices.com", true},
+      {"http://www.googleapis.com", false},
+      {"https://www.googleapis.com", true},
+      {"http://www.googlesyndication.com", false},
+      {"https://www.googlesyndication.com", true},
+      {"http://www.googleusercontent.com", false},
+      {"https://www.googleusercontent.com", true},
+      {"http://www.googlevideo.com", false},
+      {"https://www.googlevideo.com", true},
+      {"http://ssl.gstatic.com", false},
+      {"https://ssl.gstatic.com", true},
+      {"http://www.gstatic.com", false},
+      {"https://www.gstatic.com", true},
+      {"http://www.ytimg.com", false},
+      {"https://www.ytimg.com", true},
+      {"https://wwwytimg.com", false},
+      {"https://ytimg.com", false},
 
-      {"http://www.android.org", false},
-      {"http://www.doubleclick.org", false},
-      {"http://www.doubleclick.net", true},
-      {"http://www.ggpht.org", false},
-      {"http://www.googleadservices.org", false},
-      {"http://www.googleapis.org", false},
-      {"http://www.googlesyndication.org", false},
-      {"http://www.googleusercontent.org", false},
-      {"http://www.googlevideo.org", false},
-      {"http://ssl.gstatic.org", false},
-      {"http://www.gstatic.org", false},
-      {"http://www.ytimg.org", false},
+      {"https://www.android.org", false},
+      {"https://www.doubleclick.org", false},
+      {"http://www.doubleclick.net", false},
+      {"https://www.doubleclick.net", true},
+      {"https://www.ggpht.org", false},
+      {"https://www.googleadservices.org", false},
+      {"https://www.googleapis.org", false},
+      {"https://www.googlesyndication.org", false},
+      {"https://www.googleusercontent.org", false},
+      {"https://www.googlevideo.org", false},
+      {"https://ssl.gstatic.org", false},
+      {"https://www.gstatic.org", false},
+      {"https://www.ytimg.org", false},
 
-      {"http://a.b.android.com", true},
-      {"http://a.b.doubleclick.com", true},
-      {"http://a.b.doubleclick.net", true},
-      {"http://a.b.ggpht.com", true},
-      {"http://a.b.googleadservices.com", true},
-      {"http://a.b.googleapis.com", true},
-      {"http://a.b.googlesyndication.com", true},
-      {"http://a.b.googleusercontent.com", true},
-      {"http://a.b.googlevideo.com", true},
-      {"http://ssl.gstatic.com", true},
-      {"http://a.b.gstatic.com", true},
-      {"http://a.b.ytimg.com", true},
-      {"http://googleweblight.com", true},
+      {"http://a.b.android.com", false},
+      {"https://a.b.android.com", true},
+      {"http://a.b.doubleclick.com", false},
+      {"https://a.b.doubleclick.com", true},
+      {"http://a.b.doubleclick.net", false},
+      {"https://a.b.doubleclick.net", true},
+      {"http://a.b.ggpht.com", false},
+      {"https://a.b.ggpht.com", true},
+      {"http://a.b.googleadservices.com", false},
+      {"https://a.b.googleadservices.com", true},
+      {"http://a.b.googleapis.com", false},
+      {"https://a.b.googleapis.com", true},
+      {"http://a.b.googlesyndication.com", false},
+      {"https://a.b.googlesyndication.com", true},
+      {"http://a.b.googleusercontent.com", false},
+      {"https://a.b.googleusercontent.com", true},
+      {"http://a.b.googlevideo.com", false},
+      {"https://a.b.googlevideo.com", true},
+      {"http://ssl.gstatic.com", false},
+      {"https://ssl.gstatic.com", true},
+      {"http://a.b.gstatic.com", false},
+      {"https://a.b.gstatic.com", true},
+      {"http://a.b.ytimg.com", false},
+      {"https://a.b.ytimg.com", true},
+      {"http://googleweblight.com", false},
       {"https://googleweblight.com", true},
       {"http://wwwgoogleweblight.com", false},
-      {"http://www.googleweblight.com", false},
-      {"http://a.b.googleweblight.com", false},
+      {"https://www.googleweblight.com", false},
+      {"https://a.b.googleweblight.com", false},
   };
 
   for (size_t i = 0; i < arraysize(cases); ++i) {

tools/metrics/histograms/histograms.xml

@@ -70523,6 +70523,14 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="Variations.Headers.URLValidationResult"
+    enum="VariationsHeadersURLValidationResult">
+  <owner>jwd@chromium.org</owner>
+  <summary>
+    The result of the check of whether to append Variations headers to a url.
+  </summary>
+</histogram>
+
 <histogram name="Variations.LoadPermanentConsistencyCountryResult"
     enum="VariationsPermanentConsistencyCountryResult">
   <owner>sclittle@chromium.org</owner>
@@ -105701,6 +105709,13 @@ value.
   <int value="505" label="505: HTTP Version Not Supported"/>
 </enum>
 
+<enum name="VariationsHeadersURLValidationResult" type="int">
+  <int value="0" label="Rejected: Invalid URL."/>
+  <int value="1" label="Rejected: Not https."/>
+  <int value="2" label="Rejected: Not Google domain."/>
+  <int value="3" label="Should append headers."/>
+</enum>
+
 <enum name="VariationsPermanentConsistencyCountryResult" type="int">
   <int value="0" label="Saved pref missing and no country code in seed"/>
   <int value="1" label="Saved pref missing and country code in seed"/>