Common: remove url decoding the user agent (close #695)

snowplow · Sep 29, 2022 · bf95a6e · bf95a6e
1 parent 31dbd46
commit bf95a6e
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 17 deletions.
diff --git a/modules/common-fs2/src/test/resources/include_current.txt b/modules/common-fs2/src/test/resources/include_current.txt
@@ -7,4 +7,4 @@ Some user agent|1|0
 #Commented browser|1|1
 Inactive Browser|0|0|03/30/2017
 # Exclude UA from IabEnrichmentSpec
-Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0) Gecko/20100101 Firefox/12.0|1|1
+Mozilla/5.0%20(Windows%20NT%206.1;%20WOW64;%20rv:12.0)%20Gecko/20100101%20Firefox/12.0|1|1
diff --git a/...in/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/EnrichmentManager.scala b/...in/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/EnrichmentManager.scala
@@ -265,13 +265,13 @@ object EnrichmentManager {
       .toOption
       .orNull // May be updated later by 'ip'
     // May be updated later if we have a `ua` parameter
-    val useragent = setUseragent(e, raw.context.useragent, raw.source.encoding).toValidatedNel
+    setUseragent(e, raw.context.useragent)
     // Validate that the collectorTstamp exists and is Redshift-compatible
     val collectorTstamp = setCollectorTstamp(e, raw.context.timestamp).toValidatedNel
     // Map/validate/transform input fields to enriched event fields
     val transformed = Transform.transform(raw, e)
 
-    (useragent |+| collectorTstamp |+| transformed)
+    (collectorTstamp |+| transformed)
       .leftMap { enrichmentFailures =>
         EnrichmentManager.buildEnrichmentFailuresBadRow(
           enrichmentFailures,
@@ -292,22 +292,13 @@ object EnrichmentManager {
 
   def setUseragent(
     event: EnrichedEvent,
-    useragent: Option[String],
-    encoding: String
-  ): Either[FailureDetails.EnrichmentFailure, Unit] =
+    useragent: Option[String]
+  ): Unit =
     useragent match {
       case Some(ua) =>
-        CU.decodeString(Charset.forName(encoding), ua)
-          .map { ua =>
-            event.useragent = ua
-          }
-          .leftMap(f =>
-            FailureDetails.EnrichmentFailure(
-              None,
-              FailureDetails.EnrichmentFailureMessage.Simple(f)
-            )
-          )
-      case None => ().asRight // No fields updated
+        val s = ua.replaceAll("(\\r|\\n)", "").replaceAll("\\t", "    ")
+        event.useragent = s
+      case None => () // No fields updated
     }
 
   // The load fails if the collector version is not set

diff --git a/...cala/com.snowplowanalytics.snowplow.enrich.common/enrichments/EnrichmentManagerSpec.scala b/...cala/com.snowplowanalytics.snowplow.enrich.common/enrichments/EnrichmentManagerSpec.scala
@@ -736,6 +736,50 @@ class EnrichmentManagerSpec extends Specification with EitherMatchers {
       )
       enriched.value.map(_.useragent) must beRight("header-useragent")
     }
+
+    "accept user agent of HTTP header when it is not URL decodable" >> {
+      val parameters = Map(
+        "e" -> "pp",
+        "tv" -> "js-0.13.1",
+        "p" -> "web"
+      ).toOpt
+      val ua = "Mozilla/5.0 (X11; Linux x86_64; rv:75.0) Gecko/20100101 %1$s/%2$s Firefox/75.0"
+      val contextWithUa = context.copy(useragent = Some(ua))
+      val rawEvent = RawEvent(api, parameters, None, source, contextWithUa)
+      val enriched = EnrichmentManager.enrichEvent[Id](
+        enrichmentReg,
+        client,
+        processor,
+        timestamp,
+        rawEvent,
+        AcceptInvalid.featureFlags,
+        AcceptInvalid.countInvalid
+      )
+      enriched.value.map(_.useragent) must beRight(ua)
+    }
+
+    "accept 'ua' in query string when it is not URL decodable" >> {
+      val qs_ua = "Mozilla/5.0 (X11; Linux x86_64; rv:75.0) Gecko/20100101 %1$s/%2$s Firefox/75.0"
+      val parameters = Map(
+        "e" -> "pp",
+        "tv" -> "js-0.13.1",
+        "ua" -> qs_ua,
+        "p" -> "web"
+      ).toOpt
+      val contextWithUa = context.copy(useragent = Some("header-useragent"))
+      val rawEvent = RawEvent(api, parameters, None, source, contextWithUa)
+      val enriched = EnrichmentManager.enrichEvent[Id](
+        enrichmentReg,
+        client,
+        processor,
+        timestamp,
+        rawEvent,
+        AcceptInvalid.featureFlags,
+        AcceptInvalid.countInvalid
+      )
+      enriched.value.map(_.useragent) must beRight(qs_ua)
+      enriched.value.map(_.derived_contexts) must beRight((_: String).contains("\"agentName\":\"%1$S\""))
+    }
   }
 
   "getIabContext" should {