diff --git a/CHANGELOG.md b/CHANGELOG.md index 99864a4ab8..6cf66a1c9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Only send userid in `baggage` if `sendDefaultPii` is `true` ([#2145](https://github.com/getsentry/sentry-java/pull/2145)) + ### Features - New package `sentry-android-navigation` for AndroidX Navigation support ([#2136](https://github.com/getsentry/sentry-java/pull/2136)) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 6267f40405..222acede3b 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -21,10 +21,10 @@ public final class io/sentry/Attachment { } public final class io/sentry/Baggage { - public fun (Lio/sentry/ILogger;)V - public fun (Ljava/util/Map;Lio/sentry/ILogger;)V - public static fun fromHeader (Ljava/lang/String;Lio/sentry/ILogger;)Lio/sentry/Baggage; - public static fun fromHeader (Ljava/util/List;Lio/sentry/ILogger;)Lio/sentry/Baggage; + public fun (Lio/sentry/SentryOptions;)V + public fun (Ljava/util/Map;Lio/sentry/SentryOptions;)V + public static fun fromHeader (Ljava/lang/String;Lio/sentry/SentryOptions;)Lio/sentry/Baggage; + public static fun fromHeader (Ljava/util/List;Lio/sentry/SentryOptions;)Lio/sentry/Baggage; public fun get (Ljava/lang/String;)Ljava/lang/String; public fun set (Ljava/lang/String;Ljava/lang/String;)V public fun setEnvironment (Ljava/lang/String;)V @@ -1634,7 +1634,7 @@ public final class io/sentry/TraceContext : io/sentry/JsonSerializable, io/sentr public fun getUserSegment ()Ljava/lang/String; public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V public fun setUnknown (Ljava/util/Map;)V - public fun toBaggage (Lio/sentry/ILogger;)Lio/sentry/Baggage; + public fun toBaggage (Lio/sentry/SentryOptions;)Lio/sentry/Baggage; } public final class io/sentry/TraceContext$Deserializer : io/sentry/JsonDeserializer { diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index c93eef61e6..638172174b 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -20,31 +20,31 @@ public final class Baggage { static final @NotNull Integer MAX_BAGGAGE_LIST_MEMBER_COUNT = 64; final @NotNull Map keyValues; - final @NotNull ILogger logger; + final @NotNull SentryOptions options; public static Baggage fromHeader( - final @Nullable List headerValues, final @NotNull ILogger logger) { + final @Nullable List headerValues, final @NotNull SentryOptions options) { final Map keyValues = new HashMap<>(); if (headerValues != null) { for (final @Nullable String headerValue : headerValues) { final Map keyValuesToAdd = - extractKeyValuesFromBaggageString(headerValue, logger); + extractKeyValuesFromBaggageString(headerValue, options); keyValues.putAll(keyValuesToAdd); } } - return new Baggage(keyValues, logger); + return new Baggage(keyValues, options); } public static Baggage fromHeader( - final @Nullable String headerValue, final @NotNull ILogger logger) { - final Map keyValues = extractKeyValuesFromBaggageString(headerValue, logger); - return new Baggage(keyValues, logger); + final @Nullable String headerValue, final @NotNull SentryOptions options) { + final Map keyValues = extractKeyValuesFromBaggageString(headerValue, options); + return new Baggage(keyValues, options); } private static Map extractKeyValuesFromBaggageString( - final @Nullable String headerValue, final @NotNull ILogger logger) { + final @Nullable String headerValue, final @NotNull SentryOptions options) { final @NotNull Map keyValues = new HashMap<>(); if (headerValue != null) { @@ -62,28 +62,40 @@ private static Map extractKeyValuesFromBaggageString( keyValues.put(keyDecoded, valueDecoded); } else { - logger.log( - SentryLevel.ERROR, "Unable to decode baggage key value pair %s", keyValueString); + options + .getLogger() + .log( + SentryLevel.ERROR, + "Unable to decode baggage key value pair %s", + keyValueString); } } catch (Throwable e) { - logger.log( - SentryLevel.ERROR, e, "Unable to decode baggage key value pair %s", keyValueString); + options + .getLogger() + .log( + SentryLevel.ERROR, + e, + "Unable to decode baggage key value pair %s", + keyValueString); } } } catch (Throwable e) { - logger.log(SentryLevel.ERROR, e, "Unable to decode baggage header %s", headerValue); + options + .getLogger() + .log(SentryLevel.ERROR, e, "Unable to decode baggage header %s", headerValue); } } return keyValues; } - public Baggage(final @NotNull ILogger logger) { - this(new HashMap<>(), logger); + public Baggage(final @NotNull SentryOptions options) { + this(new HashMap<>(), options); } - public Baggage(final @NotNull Map keyValues, final @NotNull ILogger logger) { + public Baggage( + final @NotNull Map keyValues, final @NotNull SentryOptions options) { this.keyValues = keyValues; - this.logger = logger; + this.options = options; } public @NotNull String toHeaderString() { @@ -97,11 +109,13 @@ public Baggage(final @NotNull Map keyValues, final @NotNull ILog if (value != null) { if (listMemberCount >= MAX_BAGGAGE_LIST_MEMBER_COUNT) { - logger.log( - SentryLevel.ERROR, - "Not adding baggage value %s as the total number of list members would exceed the maximum of %s.", - key, - MAX_BAGGAGE_LIST_MEMBER_COUNT); + options + .getLogger() + .log( + SentryLevel.ERROR, + "Not adding baggage value %s as the total number of list members would exceed the maximum of %s.", + key, + MAX_BAGGAGE_LIST_MEMBER_COUNT); } else { try { final String encodedKey = encode(key); @@ -111,23 +125,27 @@ public Baggage(final @NotNull Map keyValues, final @NotNull ILog final int valueLength = encodedKeyValue.length(); final int totalLengthIfValueAdded = sb.length() + valueLength; if (totalLengthIfValueAdded > MAX_BAGGAGE_STRING_LENGTH) { - logger.log( - SentryLevel.ERROR, - "Not adding baggage value %s as the total header value length would exceed the maximum of %s.", - key, - MAX_BAGGAGE_STRING_LENGTH); + options + .getLogger() + .log( + SentryLevel.ERROR, + "Not adding baggage value %s as the total header value length would exceed the maximum of %s.", + key, + MAX_BAGGAGE_STRING_LENGTH); } else { listMemberCount++; sb.append(encodedKeyValue); separator = ","; } } catch (Throwable e) { - logger.log( - SentryLevel.ERROR, - e, - "Unable to encode baggage key value pair (key=%s,value=%s).", - key, - value); + options + .getLogger() + .log( + SentryLevel.ERROR, + e, + "Unable to encode baggage key value pair (key=%s,value=%s).", + key, + value); } } } @@ -169,7 +187,9 @@ public void setRelease(final @Nullable String release) { } public void setUserId(final @Nullable String userId) { - set("sentry-user_id", userId); + if (options.isSendDefaultPii()) { + set("sentry-user_id", userId); + } } public void setUserSegment(final @Nullable String userSegment) { diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index a05d37c579..003bf91c35 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -382,7 +382,7 @@ public void finish(@Nullable SpanStatus status) { public @Nullable BaggageHeader toBaggageHeader() { final TraceContext traceContext = traceContext(); if (hub.getOptions().isTraceSampling() && traceContext != null) { - final Baggage baggage = traceContext.toBaggage(hub.getOptions().getLogger()); + final Baggage baggage = traceContext.toBaggage(hub.getOptions()); return new BaggageHeader(baggage); } else { return null; diff --git a/sentry/src/main/java/io/sentry/TraceContext.java b/sentry/src/main/java/io/sentry/TraceContext.java index 83c2fda212..2357c0b7df 100644 --- a/sentry/src/main/java/io/sentry/TraceContext.java +++ b/sentry/src/main/java/io/sentry/TraceContext.java @@ -126,8 +126,8 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { return sampleRate; } - public @NotNull Baggage toBaggage(@NotNull ILogger logger) { - Baggage baggage = new Baggage(logger); + public @NotNull Baggage toBaggage(@NotNull SentryOptions options) { + Baggage baggage = new Baggage(options); baggage.setTraceId(traceId.toString()); baggage.setPublicKey(publicKey); diff --git a/sentry/src/test/java/io/sentry/BaggageTest.kt b/sentry/src/test/java/io/sentry/BaggageTest.kt index 1ced5f9fd3..72e977e577 100644 --- a/sentry/src/test/java/io/sentry/BaggageTest.kt +++ b/sentry/src/test/java/io/sentry/BaggageTest.kt @@ -12,16 +12,17 @@ import kotlin.test.assertNotNull import kotlin.test.assertTrue class BaggageTest { - lateinit var logger: ILogger + + lateinit var optionsWithPiiOn: SentryOptions @BeforeTest fun setup() { - logger = NoOpLogger.getInstance() + optionsWithPiiOn = SentryOptions().also { it.isSendDefaultPii = true } } @Test fun `can parse single baggage string with white spaces in it`() { - val baggage = Baggage.fromHeader("userId = alice , serverNode = DF%2028,isProduction=false", logger) + val baggage = Baggage.fromHeader("userId = alice , serverNode = DF%2028,isProduction=false", optionsWithPiiOn) assertEquals("alice", baggage.get("userId")) assertEquals("DF 28", baggage.get("serverNode")) @@ -32,7 +33,7 @@ class BaggageTest { @Test fun `can parse single baggage string`() { - val baggage = Baggage.fromHeader("userId=alice,serverNode=DF%2028,isProduction=false", logger) + val baggage = Baggage.fromHeader("userId=alice,serverNode=DF%2028,isProduction=false", optionsWithPiiOn) assertEquals("alice", baggage.get("userId")) assertEquals("DF 28", baggage.get("serverNode")) @@ -43,7 +44,7 @@ class BaggageTest { @Test fun `keys are encoded and decoded as well`() { - val baggage = Baggage.fromHeader("user%2Bid=alice,server%2Bnode=DF%2028,is%2Bproduction=false", logger) + val baggage = Baggage.fromHeader("user%2Bid=alice,server%2Bnode=DF%2028,is%2Bproduction=false", optionsWithPiiOn) assertEquals("alice", baggage.get("user+id")) assertEquals("DF 28", baggage.get("server+node")) @@ -59,7 +60,7 @@ class BaggageTest { "userId=alice", "serverNode=DF%2028,isProduction=false" ), - logger + optionsWithPiiOn ) assertEquals("alice", baggage.get("userId")) @@ -76,7 +77,7 @@ class BaggageTest { "userId = alice", "serverNode = DF%2028, isProduction = false" ), - logger + optionsWithPiiOn ) assertEquals("alice", baggage.get("userId")) @@ -89,32 +90,32 @@ class BaggageTest { @Test fun `can parse null baggage string`() { val nothing: String? = null - val baggage = Baggage.fromHeader(nothing, logger) + val baggage = Baggage.fromHeader(nothing, optionsWithPiiOn) assertEquals("", baggage.toHeaderString()) } @Test fun `can parse blank baggage string`() { - val baggage = Baggage.fromHeader("", logger) + val baggage = Baggage.fromHeader("", optionsWithPiiOn) assertEquals("", baggage.toHeaderString()) } @Test fun `can parse whitespace only baggage string`() { - val baggage = Baggage.fromHeader(" ", logger) + val baggage = Baggage.fromHeader(" ", optionsWithPiiOn) assertEquals("", baggage.toHeaderString()) } @Test fun `can parse whitespace only baggage strings`() { - val baggage = Baggage.fromHeader(listOf(" ", " "), logger) + val baggage = Baggage.fromHeader(listOf(" ", " "), optionsWithPiiOn) assertEquals("", baggage.toHeaderString()) } @Test fun `single large value is dropped and small values are kept`() { val largeValue = Faker.instance().random().hex(8193) - val baggage = Baggage.fromHeader("smallValue=remains,largeValue=$largeValue,otherValue=kept", logger) + val baggage = Baggage.fromHeader("smallValue=remains,largeValue=$largeValue,otherValue=kept", optionsWithPiiOn) assertEquals("remains", baggage.get("smallValue")) assertNotNull(baggage.get("largeValue")) @@ -126,7 +127,7 @@ class BaggageTest { @Test fun `medium size value can cause small values to be dropped`() { val mediumValue = Faker.instance().random().hex(MAX_BAGGAGE_STRING_LENGTH - 12 - 15 - 1) // 8192 - "mediumValue=" - "otherValue=kept" - "," - val baggage = Baggage.fromHeader("mediumValue=$mediumValue,smallValue=removed,otherValue=kept", logger) + val baggage = Baggage.fromHeader("mediumValue=$mediumValue,smallValue=removed,otherValue=kept", optionsWithPiiOn) assertEquals("removed", baggage.get("smallValue")) assertEquals(mediumValue, baggage.get("mediumValue")) @@ -141,7 +142,7 @@ class BaggageTest { fun `medium size value can cause all values to be dropped`() { // nothing else will fit after mediumValue as the separator + any key/value would exceed the limit val mediumValue = Faker.instance().random().hex(MAX_BAGGAGE_STRING_LENGTH - 12 - 15) // 8192 - "mediumValue=" - "otherValue=lost" - val baggage = Baggage.fromHeader("mediumValue=$mediumValue,smallValue=stripped,otherValue=lost", logger) + val baggage = Baggage.fromHeader("mediumValue=$mediumValue,smallValue=stripped,otherValue=lost", optionsWithPiiOn) assertEquals("stripped", baggage.get("smallValue")) assertEquals(mediumValue, baggage.get("mediumValue")) @@ -154,7 +155,7 @@ class BaggageTest { @Test fun `exceeding entry limit causes values to be dropped`() { - val baggage = Baggage(logger) + val baggage = Baggage(optionsWithPiiOn) val expectedItems = mutableListOf() for (i in 1..100) { @@ -171,7 +172,7 @@ class BaggageTest { @Test fun `null value is omitted from header string`() { - val baggage = Baggage(logger) + val baggage = Baggage(optionsWithPiiOn) baggage.setTraceId(null) baggage.setPublicKey(null) @@ -186,7 +187,7 @@ class BaggageTest { @Test fun `can set values from trace context`() { - val baggage = Baggage(logger) + val baggage = Baggage(optionsWithPiiOn) val publicKey = Dsn(dsnString).publicKey val traceId = SentryId().toString() val userId = UUID.randomUUID().toString() @@ -203,15 +204,34 @@ class BaggageTest { assertEquals("sentry-environment=production,sentry-public_key=$publicKey,sentry-release=1.0-rc.1,sentry-sample_rate=0.3333333333333333,sentry-trace_id=$traceId,sentry-transaction=TX,sentry-user_id=$userId,sentry-user_segment=segmentA", baggage.toHeaderString()) } + @Test + fun `userId is skipped if sendDefaultPii is off`() { + val baggage = Baggage(SentryOptions()) + val publicKey = Dsn(dsnString).publicKey + val traceId = SentryId().toString() + val userId = UUID.randomUUID().toString() + + baggage.setTraceId(traceId) + baggage.setPublicKey(publicKey) + baggage.setRelease("1.0-rc.1") + baggage.setEnvironment("production") + baggage.setTransaction("TX") + baggage.setUserId(userId) + baggage.setUserSegment("segmentA") + baggage.setSampleRate((1.0 / 3.0).toString()) + + assertEquals("sentry-environment=production,sentry-public_key=$publicKey,sentry-release=1.0-rc.1,sentry-sample_rate=0.3333333333333333,sentry-trace_id=$traceId,sentry-transaction=TX,sentry-user_segment=segmentA", baggage.toHeaderString()) + } + @Test fun `duplicate entries are lost`() { - val baggage = Baggage.fromHeader("duplicate=a,duplicate=b", logger) + val baggage = Baggage.fromHeader("duplicate=a,duplicate=b", optionsWithPiiOn) assertEquals("duplicate=b", baggage.toHeaderString()) } @Test fun `setting a value multiple times only keeps the last`() { - val baggage = Baggage.fromHeader("sentry-trace_id=a", logger) + val baggage = Baggage.fromHeader("sentry-trace_id=a", optionsWithPiiOn) baggage.setTraceId("b") baggage.setTraceId("c") @@ -221,7 +241,7 @@ class BaggageTest { @Test fun `value may contain = sign`() { - val baggage = Baggage(logger) + val baggage = Baggage(optionsWithPiiOn) baggage.setTransaction("a=b") @@ -230,19 +250,19 @@ class BaggageTest { @Test fun `corrupted string does not throw out`() { - val baggage = Baggage.fromHeader("a", logger) + val baggage = Baggage.fromHeader("a", optionsWithPiiOn) assertEquals("", baggage.toHeaderString()) } @Test fun `corrupted string does not throw out 2`() { - val baggage = Baggage.fromHeader("a=b=", logger) + val baggage = Baggage.fromHeader("a=b=", optionsWithPiiOn) assertEquals("", baggage.toHeaderString()) } @Test fun `corrupted string can be parsed partially`() { - val baggage = Baggage.fromHeader("a=value,b", logger) + val baggage = Baggage.fromHeader("a=value,b", optionsWithPiiOn) assertEquals("a=value", baggage.toHeaderString()) } @@ -345,7 +365,7 @@ class BaggageTest { val failures = mutableListOf() values.forEach { key, value -> - val baggage = Baggage(logger) + val baggage = Baggage(optionsWithPiiOn) baggage.setTransaction(key) val headerString = baggage.toHeaderString() @@ -353,7 +373,7 @@ class BaggageTest { failures.add("$key should be $value but was >$headerString<") } - val decodedBaggage = Baggage.fromHeader(headerString, logger) + val decodedBaggage = Baggage.fromHeader(headerString, optionsWithPiiOn) decodedBaggage.get("sentry-transaction") } @@ -362,17 +382,17 @@ class BaggageTest { @Test fun `all characters defined as valid for keys can be used`() { - val baggage = Baggage(logger) + val baggage = Baggage(optionsWithPiiOn) val key = validTokenCharacters().joinToString("") baggage.set(key, "value") - val reparsedBaggage = Baggage.fromHeader(baggage.toHeaderString(), logger) + val reparsedBaggage = Baggage.fromHeader(baggage.toHeaderString(), optionsWithPiiOn) assertEquals("value", reparsedBaggage.get(key)) } @Test fun `baggage key replaces invalid characters`() { - val baggage = Baggage(logger) + val baggage = Baggage(optionsWithPiiOn) baggage.set(invalidTokenCharacters().joinToString(""), "value") assertEquals("%22%28%29%2C%2F%3A%3B%3C%3D%3E%3F%40%5B%5C%5D%7B%7D=value", baggage.toHeaderString()) diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index 527a2c9af0..1e76cb9d1d 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -525,6 +525,7 @@ class SentryTracerTest { it.isTraceSampling = true it.environment = "production" it.release = "1.0.99-rc.7" + it.isSendDefaultPii = true }) fixture.hub.setUser(