Merge branch 'feat/data-collection-cookies' into feat/data-collection-user-info

Author: adinauer

sentry/src/main/java/io/sentry/util/HttpUtils.java

@@ -156,7 +156,7 @@ public static boolean containsSensitiveHeader(final @NotNull String header) {
       }
       return filteredCookies.toString();
     } catch (Throwable ignored) {
-      return null;
+      return SENSITIVE_DATA_SUBSTITUTE;
     }
   }
 
@@ -170,20 +170,27 @@ public static boolean containsSensitiveHeader(final @NotNull String header) {
       final int attributesSeparator = cookie.indexOf(';');
       final @NotNull String cookieValue =
           attributesSeparator < 0 ? cookie : cookie.substring(0, attributesSeparator);
+      if (!isValidCookiePair(cookieValue)) {
+        return SENSITIVE_DATA_SUBSTITUTE;
+      }
       final @NotNull String attributes =
           attributesSeparator < 0 ? "" : cookie.substring(attributesSeparator);
       return filterCookie(cookieValue, behavior, null) + attributes;
     } catch (Throwable ignored) {
-      return null;
+      return SENSITIVE_DATA_SUBSTITUTE;
     }
   }
 
   private static @NotNull String filterCookie(
       final @NotNull String cookie,
       final @NotNull KeyValueCollectionBehavior behavior,
       final @Nullable List<String> additionalSensitiveCookieNames) {
+    if (!isValidCookiePair(cookie)) {
+      return SENSITIVE_DATA_SUBSTITUTE;
+    }
+
     final int separator = cookie.indexOf('=');
-    final @NotNull String name = separator < 0 ? cookie : cookie.substring(0, separator);
+    final @NotNull String name = cookie.substring(0, separator);
     final @NotNull String normalizedName = name.trim();
     final boolean sensitive =
         containsTerm(normalizedName, SENSITIVE_DATA_KEYS)
@@ -200,6 +207,11 @@ public static boolean containsSensitiveHeader(final @NotNull String header) {
     return cookie;
   }
 
+  private static boolean isValidCookiePair(final @NotNull String cookie) {
+    final int separator = cookie.indexOf('=');
+    return separator >= 0 && !cookie.substring(0, separator).trim().isEmpty();
+  }
+
   public static @NotNull Map<String, String> filterHeaders(
       final @NotNull Map<String, String> headers,
       final @NotNull KeyValueCollectionBehavior behavior) {

sentry/src/test/java/io/sentry/util/HttpUtilsTest.kt

@@ -97,6 +97,18 @@ class HttpUtilsTest {
       .isEqualTo("theme=dark; sessionId=[Filtered]; language=[Filtered]")
   }
 
+  @Test
+  fun `cookie filter preserves empty and padded base64 values`() {
+    assertThat(
+        HttpUtils.filterCookies(
+          "empty=; data=YWJjZA==",
+          KeyValueCollectionBehavior.denyList(),
+          emptyList(),
+        )
+      )
+      .isEqualTo("empty=; data=YWJjZA==")
+  }
+
   @Test
   fun `cookie filter uses only the first equals separator`() {
     assertThat(
@@ -109,6 +121,30 @@ class HttpUtilsTest {
       .isEqualTo("theme=dark=contrast; token=[Filtered]")
   }
 
+  @Test
+  fun `cookie filter replaces malformed pairs without discarding valid pairs`() {
+    assertThat(
+        HttpUtils.filterCookies(
+          "theme=dark; opaque; =secret; empty=; sessionId=secret",
+          KeyValueCollectionBehavior.denyList(),
+          emptyList(),
+        )
+      )
+      .isEqualTo("theme=dark;[Filtered];[Filtered]; empty=; sessionId=[Filtered]")
+  }
+
+  @Test
+  fun `cookie allow list never exposes malformed pairs`() {
+    assertThat(
+        HttpUtils.filterCookies(
+          "theme=dark; opaque; =secret",
+          KeyValueCollectionBehavior.allowList("theme", "opaque"),
+          emptyList(),
+        )
+      )
+      .isEqualTo("theme=dark;[Filtered];[Filtered]")
+  }
+
   @Test
   fun `set cookie filter preserves attributes`() {
     assertThat(
@@ -120,6 +156,26 @@ class HttpUtilsTest {
       .isEqualTo("sessionId=[Filtered]; Path=/; HttpOnly; SameSite=Lax")
   }
 
+  @Test
+  fun `set cookie filter preserves empty and padded base64 values`() {
+    assertThat(
+        HttpUtils.filterSetCookie(
+          "data=YWJjZA==; Expires=Wed, 09 Jun 2021 10:18:14 GMT; Max-Age=3600; Domain=example.com; Path=/; Secure; HttpOnly; SameSite=Lax",
+          KeyValueCollectionBehavior.denyList(),
+        )
+      )
+      .isEqualTo(
+        "data=YWJjZA==; Expires=Wed, 09 Jun 2021 10:18:14 GMT; Max-Age=3600; Domain=example.com; Path=/; Secure; HttpOnly; SameSite=Lax"
+      )
+    assertThat(
+        HttpUtils.filterSetCookie(
+          "empty=; Path=/",
+          KeyValueCollectionBehavior.denyList(),
+        )
+      )
+      .isEqualTo("empty=; Path=/")
+  }
+
   @Test
   fun `set cookie allow list retains allowed non-sensitive value and attributes`() {
     assertThat(
@@ -131,6 +187,35 @@ class HttpUtilsTest {
       .isEqualTo("theme=dark; Path=/; Secure")
   }
 
+  @Test
+  fun `set cookie filter replaces malformed cookie pair and discards attributes`() {
+    assertThat(
+        HttpUtils.filterSetCookie(
+          "opaque; Path=/; HttpOnly",
+          KeyValueCollectionBehavior.denyList(),
+        )
+      )
+      .isEqualTo("[Filtered]")
+    assertThat(
+        HttpUtils.filterSetCookie(
+          "=secret; Path=/; HttpOnly",
+          KeyValueCollectionBehavior.denyList(),
+        )
+      )
+      .isEqualTo("[Filtered]")
+  }
+
+  @Test
+  fun `set cookie allow list never exposes malformed cookie pair`() {
+    assertThat(
+        HttpUtils.filterSetCookie(
+          "opaque; Path=/; HttpOnly",
+          KeyValueCollectionBehavior.allowList("opaque"),
+        )
+      )
+      .isEqualTo("[Filtered]")
+  }
+
   @Test
   fun `set cookie filter disables collection in off mode`() {
     assertThat(