Only log hashes of headers from 1p intents

Bug: 873178
Change-Id: Id0ae7b02ba2d2ecd135331da5649d415033088b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1936294Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719193}

chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java

@@ -780,6 +780,9 @@ public class IntentHandler {
 
         // We do some logging to determine what kinds of headers developers are inserting.
         IntentHeadersRecorder recorder = shouldLogHeaders ? new IntentHeadersRecorder() : null;
+        boolean firstParty = shouldLogHeaders
+                ? IntentHandler.notSecureIsIntentChromeOrFirstParty(intent)
+                : false;
 
         for (String key : bundleExtraHeaders.keySet()) {
             String value = bundleExtraHeaders.getString(key);
@@ -787,7 +790,7 @@ public class IntentHandler {
             // Strip the custom header that can only be added by ourselves.
             if ("x-chrome-intent-type".equals(key.toLowerCase(Locale.US))) continue;
 
-            if (shouldLogHeaders) recorder.recordHeader(key, value);
+            if (shouldLogHeaders) recorder.recordHeader(key, value, firstParty);
 
             if (!HttpUtil.isAllowedHeader(key, value)) continue;
 
@@ -797,9 +800,7 @@ public class IntentHandler {
             extraHeaders.append(value);
         }
 
-        if (shouldLogHeaders) {
-            recorder.report(IntentHandler.notSecureIsIntentChromeOrFirstParty(intent));
-        }
+        if (shouldLogHeaders) recorder.report(firstParty);
         return extraHeaders.length() == 0 ? null : extraHeaders.toString();
     }
 

chrome/android/java/src/org/chromium/chrome/browser/IntentHeadersRecorder.java

@@ -26,8 +26,9 @@ public class IntentHeadersRecorder {
     /** Determines whether a header is CORS Safelisted or not. */
     @JNINamespace("chrome::android")
     /* package */ static class HeaderClassifier {
-        /* package */ boolean isCorsSafelistedHeader(String name, String value) {
-            return IntentHeadersRecorderJni.get().isCorsSafelistedHeader(name, value);
+        /* package */ boolean isCorsSafelistedHeader(
+                String name, String value, boolean firstParty) {
+            return IntentHeadersRecorderJni.get().isCorsSafelistedHeader(name, value, firstParty);
         }
     }
 
@@ -65,9 +66,12 @@ public class IntentHeadersRecorder {
     }
 
     /* Records that a HTTP header has been used. */
-    public void recordHeader(String name, String value) {
-        if (mClassifier.isCorsSafelistedHeader(name, value)) mSafeHeaders++;
-        else mUnsafeHeaders++;
+    public void recordHeader(String name, String value, boolean firstParty) {
+        if (mClassifier.isCorsSafelistedHeader(name, value, firstParty)) {
+            mSafeHeaders++;
+        } else {
+            mUnsafeHeaders++;
+        }
     }
 
     /**
@@ -102,6 +106,6 @@ public class IntentHeadersRecorder {
 
     @NativeMethods
     interface Natives {
-        boolean isCorsSafelistedHeader(String name, String value);
+        boolean isCorsSafelistedHeader(String name, String value, boolean firstParty);
     }
 }

chrome/android/junit/src/org/chromium/chrome/browser/IntentHeadersRecorderTest.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser;
 
+import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
@@ -38,8 +39,12 @@ public class IntentHeadersRecorderTest {
         ShadowRecordHistogram.reset();
         MockitoAnnotations.initMocks(this);
 
-        doReturn(true).when(mClassifier).isCorsSafelistedHeader(eq(SAFE_HEADER), anyString());
-        doReturn(false).when(mClassifier).isCorsSafelistedHeader(eq(UNSAFE_HEADER), anyString());
+        doReturn(true)
+                .when(mClassifier)
+                .isCorsSafelistedHeader(eq(SAFE_HEADER), anyString(), anyBoolean());
+        doReturn(false)
+                .when(mClassifier)
+                .isCorsSafelistedHeader(eq(UNSAFE_HEADER), anyString(), anyBoolean());
 
         mRecorder = new IntentHeadersRecorder(mClassifier);
     }
@@ -58,44 +63,44 @@ public class IntentHeadersRecorderTest {
 
     @Test
     public void safeHeaders_firstParty() {
-        mRecorder.recordHeader(SAFE_HEADER, "");
+        mRecorder.recordHeader(SAFE_HEADER, "", true);
         mRecorder.report(true);
         assertUma(0, 1, 0, 0, 0, 0);
     }
 
     @Test
     public void safeHeaders_thirdParty() {
-        mRecorder.recordHeader(SAFE_HEADER, "");
+        mRecorder.recordHeader(SAFE_HEADER, "", false);
         mRecorder.report(false);
         assertUma(0, 0, 0, 0, 1, 0);
     }
 
     @Test
     public void unsafeHeaders_firstParty() {
-        mRecorder.recordHeader(UNSAFE_HEADER, "");
+        mRecorder.recordHeader(UNSAFE_HEADER, "", true);
         mRecorder.report(true);
         assertUma(0, 0, 1, 0, 0, 0);
     }
 
     @Test
     public void unsafeHeaders_thirdParty() {
-        mRecorder.recordHeader(UNSAFE_HEADER, "");
+        mRecorder.recordHeader(UNSAFE_HEADER, "", false);
         mRecorder.report(false);
         assertUma(0, 0, 0, 0, 0, 1);
     }
 
     @Test
     public void mixedHeaders_firstParty() {
-        mRecorder.recordHeader(SAFE_HEADER, "");
-        mRecorder.recordHeader(UNSAFE_HEADER, "");
+        mRecorder.recordHeader(SAFE_HEADER, "", true);
+        mRecorder.recordHeader(UNSAFE_HEADER, "", true);
         mRecorder.report(true);
         assertUma(0, 0, 1, 0, 0, 0);
     }
 
     @Test
     public void mixedHeaders_thirdParty() {
-        mRecorder.recordHeader(SAFE_HEADER, "");
-        mRecorder.recordHeader(UNSAFE_HEADER, "");
+        mRecorder.recordHeader(SAFE_HEADER, "", false);
+        mRecorder.recordHeader(UNSAFE_HEADER, "", false);
         mRecorder.report(false);
         assertUma(0, 0, 0, 0, 0, 1);
     }

chrome/browser/android/headers_classifier.cc

@@ -17,16 +17,19 @@ namespace android {
 jboolean JNI_IntentHeadersRecorder_IsCorsSafelistedHeader(
     JNIEnv* env,
     const JavaParamRef<jstring>& j_header_name,
-    const JavaParamRef<jstring>& j_header_value) {
+    const JavaParamRef<jstring>& j_header_value,
+    jboolean is_first_party) {
   std::string header_name(ConvertJavaStringToUTF8(env, j_header_name));
   std::string header_value(ConvertJavaStringToUTF8(env, j_header_value));
 
   if (network::cors::IsCorsSafelistedHeader(header_name, header_value))
     return true;
 
-  base::UmaHistogramSparse(
-      "Android.IntentNonSafelistedHeaderNames",
-      base::PersistentHash(base::ToLowerASCII(header_name)));
+  if (is_first_party) {
+    base::UmaHistogramSparse(
+        "Android.IntentNonSafelistedHeaderNames",
+        base::PersistentHash(base::ToLowerASCII(header_name)));
+  }
 
   return false;
 }