Set network_userid to empty UUID in anonymous mode to prevent collector_payload_format_violation (close #126)

Author: dilyand

core/src/main/scala/com.snowplowanalytics.snowplow.collectors.scalastream/CollectorService.scala

@@ -82,6 +82,8 @@ class CollectorService(
   override val doNotTrackCookie      = config.doNotTrackHttpCookie
   override val enableDefaultRedirect = config.enableDefaultRedirect
 
+  private val spAnonymousNuid = "00000000-0000-0000-0000-000000000000"
+
   /**
     * Determines the path to be used in the response,
     * based on whether a mapping can be found in the config for the original request path.
@@ -112,7 +114,7 @@ class CollectorService(
       case Right(params) =>
         val redirect = path.startsWith("/r/")
 
-        val nuidOpt  = networkUserId(request, cookie)
+        val nuidOpt  = networkUserId(request, cookie, spAnonymous)
         val bouncing = params.contains(config.cookieBounce.name)
         // we bounce if it's enabled and we couldn't retrieve the nuid and we're not already bouncing
         val bounce = config.cookieBounce.enabled && nuidOpt.isEmpty && !bouncing &&
@@ -243,7 +245,7 @@ class CollectorService(
     userAgent.foreach(e.userAgent   = _)
     refererUri.foreach(e.refererUri = _)
     e.hostname      = hostname
-    e.networkUserId = spAnonymous.fold(networkUserId)(_ => "")
+    e.networkUserId = networkUserId
     e.headers       = (headers(request, spAnonymous) ++ contentType).asJava
     contentType.foreach(e.contentType = _)
     e
@@ -457,8 +459,15 @@ class CollectorService(
     * @param requestCookie cookie associated to the Http request
     * @return a network user id
     */
-  def networkUserId(request: HttpRequest, requestCookie: Option[HttpCookie]): Option[String] =
-    request.uri.query().get("nuid").orElse(requestCookie.map(_.value))
+  def networkUserId(
+    request: HttpRequest,
+    requestCookie: Option[HttpCookie],
+    spAnonymous: Option[String]
+  ): Option[String] =
+    spAnonymous match {
+      case Some(_) => Some(spAnonymousNuid)
+      case None    => request.uri.query().get("nuid").orElse(requestCookie.map(_.value))
+    }
 
   /**
     * Creates an Access-Control-Allow-Origin header which specifically allows the domain which made

core/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/CollectorServiceSpec.scala

@@ -129,7 +129,7 @@ class CollectorServiceSpec extends Specification {
           None,
           Some("b"),
           "p",
-          Some(HttpCookie("sp","cookie-nuid")),
+          Some(HttpCookie("sp", "cookie-nuid")),
           None,
           None,
           "h",
@@ -143,14 +143,14 @@ class CollectorServiceSpec extends Specification {
         l must have size 1
         val newEvent = new CollectorPayload("iglu-schema", "ip", System.currentTimeMillis, "UTF-8", "collector")
         deserializer.deserialize(newEvent, l.head)
-        newEvent.networkUserId shouldEqual ""
+        newEvent.networkUserId shouldEqual "00000000-0000-0000-0000-000000000000"
       }
       "network_userid from cookie should persist if SP-Anonymous is not present" in {
         val (_, l) = service.cookie(
           None,
           Some("b"),
           "p",
-          Some(HttpCookie("sp","cookie-nuid")),
+          Some(HttpCookie("sp", "cookie-nuid")),
           None,
           None,
           "h",
@@ -340,9 +340,10 @@ class CollectorServiceSpec extends Specification {
         e.contentType shouldEqual ct.get
       }
       "fill the correct values if SP-Anonymous is present" in {
-        val l  = `Location`("l")
-        val ct = Some("image/gif")
-        val r  = HttpRequest().withHeaders(l :: hs)
+        val l    = `Location`("l")
+        val ct   = Some("image/gif")
+        val r    = HttpRequest().withHeaders(l :: hs)
+        val nuid = service.networkUserId(r, None, Some("*")).get
         val e =
           service.buildEvent(
             Some("q"),
@@ -353,7 +354,7 @@ class CollectorServiceSpec extends Specification {
             "h",
             "unknown",
             r,
-            "nuid",
+            nuid,
             ct,
             Some("*")
           )
@@ -367,16 +368,29 @@ class CollectorServiceSpec extends Specification {
         e.userAgent shouldEqual "ua"
         e.refererUri shouldEqual "ref"
         e.hostname shouldEqual "h"
-        e.networkUserId shouldEqual ""
+        e.networkUserId shouldEqual "00000000-0000-0000-0000-000000000000"
         e.headers shouldEqual (List(l) ++ ct).map(_.toString).asJava
         e.contentType shouldEqual ct.get
       }
       "have a null queryString if it's None" in {
-        val l  = `Location`("l")
-        val ct = Some("image/gif")
-        val r  = HttpRequest().withHeaders(l :: hs)
+        val l    = `Location`("l")
+        val ct   = Some("image/gif")
+        val r    = HttpRequest().withHeaders(l :: hs)
+        val nuid = service.networkUserId(r, None, Some("*")).get
         val e =
-          service.buildEvent(None, Some("b"), "p", Some("ua"), Some("ref"), "h", "unknown", r, "nuid", ct, Some("*"))
+          service.buildEvent(
+            None,
+            Some("b"),
+            "p",
+            Some("ua"),
+            Some("ref"),
+            "h",
+            "unknown",
+            r,
+            nuid,
+            ct,
+            Some("*")
+          )
         e.schema shouldEqual "iglu:com.snowplowanalytics.snowplow/CollectorPayload/thrift/1-0-0"
         e.ipAddress shouldEqual "unknown"
         e.encoding shouldEqual "UTF-8"
@@ -387,17 +401,30 @@ class CollectorServiceSpec extends Specification {
         e.userAgent shouldEqual "ua"
         e.refererUri shouldEqual "ref"
         e.hostname shouldEqual "h"
-        e.networkUserId shouldEqual ""
+        e.networkUserId shouldEqual "00000000-0000-0000-0000-000000000000"
         e.headers shouldEqual (List(l) ++ ct).map(_.toString).asJava
         e.contentType shouldEqual ct.get
       }
       "have an empty nuid if SP-Anonymous is present" in {
-        val l  = `Location`("l")
-        val ct = Some("image/gif")
-        val r  = HttpRequest().withHeaders(l :: hs)
+        val l    = `Location`("l")
+        val ct   = Some("image/gif")
+        val r    = HttpRequest().withHeaders(l :: hs)
+        val nuid = service.networkUserId(r, None, Some("*")).get
         val e =
-          service.buildEvent(None, Some("b"), "p", Some("ua"), Some("ref"), "h", "unknown", r, "nuid", ct, Some("*"))
-        e.networkUserId shouldEqual ""
+          service.buildEvent(
+            None,
+            Some("b"),
+            "p",
+            Some("ua"),
+            Some("ref"),
+            "h",
+            "unknown",
+            r,
+            nuid,
+            ct,
+            Some("*")
+          )
+        e.networkUserId shouldEqual "00000000-0000-0000-0000-000000000000"
       }
       "have a nuid if SP-Anonymous is not present" in {
         val l  = `Location`("l")
@@ -671,17 +698,42 @@ class CollectorServiceSpec extends Specification {
     }
 
     "netwokUserId" in {
-      "give back the nuid query param if present" in {
-        service.networkUserId(
-          HttpRequest().withUri(Uri().withRawQueryString("nuid=12")),
-          Some(HttpCookie("nuid", "13"))
-        ) shouldEqual Some("12")
-      }
-      "give back the request cookie if there no nuid query param" in {
-        service.networkUserId(HttpRequest(), Some(HttpCookie("nuid", "13"))) shouldEqual Some("13")
+      "with SP-Anonymous header not present" in {
+        "give back the nuid query param if present" in {
+          service.networkUserId(
+            HttpRequest().withUri(Uri().withRawQueryString("nuid=12")),
+            Some(HttpCookie("nuid", "13")),
+            None
+          ) shouldEqual Some("12")
+        }
+        "give back the request cookie if there no nuid query param" in {
+          service.networkUserId(HttpRequest(), Some(HttpCookie("nuid", "13")), None) shouldEqual Some("13")
+        }
+        "give back none otherwise" in {
+          service.networkUserId(HttpRequest(), None, None) shouldEqual None
+        }
       }
-      "give back none otherwise" in {
-        service.networkUserId(HttpRequest(), None) shouldEqual None
+
+      "with SP-Anonymous header present" in {
+        "give back the dummy nuid" in {
+          "if query param is present" in {
+            service.networkUserId(
+              HttpRequest().withUri(Uri().withRawQueryString("nuid=12")),
+              Some(HttpCookie("nuid", "13")),
+              Some("*")
+            ) shouldEqual Some("00000000-0000-0000-0000-000000000000")
+          }
+          "if the request cookie can be used in place of a missing nuid query param" in {
+            service.networkUserId(HttpRequest(), Some(HttpCookie("nuid", "13")), Some("*")) shouldEqual Some(
+              "00000000-0000-0000-0000-000000000000"
+            )
+          }
+          "in any other case" in {
+            service.networkUserId(HttpRequest(), None, Some("*")) shouldEqual Some(
+              "00000000-0000-0000-0000-000000000000"
+            )
+          }
+        }
       }
     }