Skip to content

Commit

Permalink
Set network_userid to empty UUID in anonymous mode to prevent collect…
Browse files Browse the repository at this point in the history 
…or_payload_format_violation (close #126)
  • Loading branch information
@dilyand
dilyand committed Mar 18, 2021
1 parent 33b6e2b commit 3877728
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 32 deletions.
17 changes: 13 additions & 4 deletions ...c/main/scala/com.snowplowanalytics.snowplow.collectors.scalastream/CollectorService.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
108 changes: 80 additions & 28 deletions ...st/scala/com.snowplowanalytics.snowplow.collectors.scalastream/CollectorServiceSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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"),
Expand All @@ -353,7 +354,7 @@ class CollectorServiceSpec extends Specification {
"h",
"unknown",
r,
"nuid",
nuid,
ct,
Some("*")
)
Expand All @@ -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"
Expand All @@ -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")
Expand Down Expand Up @@ -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"
)
}
}
}
}

Expand Down

0 comments on commit 3877728

Please sign in to comment.