Fix #1560 - Handle null user agent strings while scrubbing (#1561)

* Fix #1560 - Handle null user agent strings while scrubbing * Fix typo in attributes * Add test that does not throw an exception
mozilla · Feb 9, 2021 · 74a3db6 · 74a3db6
1 parent 56bf105
commit 74a3db6
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 5 deletions.
diff --git a/ingestion-beam/src/main/java/com/mozilla/telemetry/decoder/MessageScrubber.java b/ingestion-beam/src/main/java/com/mozilla/telemetry/decoder/MessageScrubber.java
@@ -79,6 +79,7 @@ public static void scrub(Map<String, String> attributes, ObjectNode json)
     final String appVersion = attributes.get(Attribute.APP_VERSION);
     final String appUpdateChannel = attributes.get(Attribute.APP_UPDATE_CHANNEL);
     final String appBuildId = attributes.get(Attribute.APP_BUILD_ID);
+    // NOTE: this value may be null
     final String userAgent = attributes.get(Attribute.USER_AGENT);
 
     // Check for toxic data that should be dropped without sending to error output.
@@ -133,7 +134,8 @@ public static void scrub(Map<String, String> attributes, ObjectNode json)
 
     // Glean enforces a particular user-agent string that a rogue fuzzer is not abiding by
     // https://searchfox.org/mozilla-central/source/third_party/rust/glean-core/src/upload/request.rs#35,72-75
-    if ("firefox-desktop".equals(namespace) && !userAgent.startsWith("Glean")) {
+    if ("firefox-desktop".equals(namespace)
+        && (userAgent == null || !userAgent.startsWith("Glean"))) {
       throw new UnwantedDataException("1684980");
     }
 

diff --git a/ingestion-beam/src/test/java/com/mozilla/telemetry/decoder/MessageScrubberTest.java b/ingestion-beam/src/test/java/com/mozilla/telemetry/decoder/MessageScrubberTest.java
@@ -547,19 +547,41 @@ public void testUnwantedData1684980GleanUserAgent() {
         () -> MessageScrubber.scrub(mozAttributes, Json.createObjectNode()));
 
     // empty agent strings are also scrubbed
-    Map<String, String> emptyAtrributes = ImmutableMap.<String, String>builder()
+    Map<String, String> emptyAttributes = ImmutableMap.<String, String>builder()
         .put(Attribute.DOCUMENT_NAMESPACE, "firefox-desktop") //
         .put(Attribute.USER_AGENT, "") //
         .build();
 
     assertThrows(UnwantedDataException.class,
-        () -> MessageScrubber.scrub(emptyAtrributes, Json.createObjectNode()));
+        () -> MessageScrubber.scrub(emptyAttributes, Json.createObjectNode()));
 
-    Map<String, String> gleanAtrributes = ImmutableMap.<String, String>builder()
+    // null agent strings...
+    Map<String, String> nullAttributes = ImmutableMap.<String, String>builder()
+        .put(Attribute.DOCUMENT_NAMESPACE, "firefox-desktop") //
+        .build();
+
+    assertThrows(UnwantedDataException.class,
+        () -> MessageScrubber.scrub(nullAttributes, Json.createObjectNode()));
+
+    Map<String, String> gleanAttributes = ImmutableMap.<String, String>builder()
         .put(Attribute.DOCUMENT_NAMESPACE, "firefox-desktop") //
         .put(Attribute.USER_AGENT, "Glean/33.9.1 (Rust on Windows)") //
         .build();
 
-    MessageScrubber.scrub(gleanAtrributes, Json.createObjectNode());
+    MessageScrubber.scrub(gleanAttributes, Json.createObjectNode());
+  }
+
+  @Test
+  public void testScrubValidDocument() {
+    Map<String, String> attributes = ImmutableMap.<String, String>builder()
+        .put(Attribute.DOCUMENT_NAMESPACE, "namespace") //
+        .put(Attribute.DOCUMENT_TYPE, "type") //
+        .put(Attribute.APP_NAME, "name") //
+        .put(Attribute.APP_VERSION, "version") //
+        .put(Attribute.APP_UPDATE_CHANNEL, "channel") //
+        .put(Attribute.APP_BUILD_ID, "build_id") //
+        // USER_AGENT may be null
+        .build();
+    MessageScrubber.scrub(attributes, Json.createObjectNode());
   }
 }