Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling immutable collections on SentryEvent and protocol objects #1468

Merged
merged 10 commits into from
May 12, 2021
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Unreleased

* Fix: handling immutable collections on SentryEvent and protocol objects (#1468)
* Fix: associate event with transaction when thrown exception is not a direct cause (#1463)

# 5.0.0-beta.2
Expand Down
4 changes: 3 additions & 1 deletion sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -1853,7 +1853,9 @@ public final class io/sentry/util/ApplyScopeUtils {

public final class io/sentry/util/CollectionUtils {
public static fun filterMapEntries (Ljava/util/Map;Lio/sentry/util/CollectionUtils$Predicate;)Ljava/util/Map;
public static fun shallowCopy (Ljava/util/Map;)Ljava/util/Map;
public static fun newArrayList (Ljava/util/List;)Ljava/util/List;
public static fun newConcurrentHashMap (Ljava/util/Map;)Ljava/util/Map;
public static fun newHashMap (Ljava/util/Map;)Ljava/util/Map;
public static fun size (Ljava/lang/Iterable;)I
}

Expand Down
4 changes: 2 additions & 2 deletions sentry/src/main/java/io/sentry/Breadcrumb.java
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,11 @@ Map<String, Object> getUnknown() {
public @NotNull Breadcrumb clone() throws CloneNotSupportedException {
final Breadcrumb clone = (Breadcrumb) super.clone();

final Map<String, Object> dataCopy = CollectionUtils.shallowCopy(this.data);
final Map<String, Object> dataCopy = CollectionUtils.newConcurrentHashMap(this.data);
if (dataCopy != null) {
clone.data = dataCopy;
}
clone.unknown = CollectionUtils.shallowCopy(unknown);
clone.unknown = CollectionUtils.newConcurrentHashMap(unknown);

final SentryLevel levelRef = level;
clone.level =
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/Scope.java
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ public void setFingerprint(final @NotNull List<String> fingerprint) {
if (fingerprint == null) {
return;
}
this.fingerprint = fingerprint;
this.fingerprint = new ArrayList<>(fingerprint);
}

/**
Expand Down
7 changes: 4 additions & 3 deletions sentry/src/main/java/io/sentry/SentryBaseEvent.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.sentry.protocol.SdkVersion;
import io.sentry.protocol.SentryId;
import io.sentry.protocol.User;
import io.sentry.util.CollectionUtils;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -186,7 +187,7 @@ public void setThrowable(final @Nullable Throwable throwable) {
}

public void setTags(@Nullable Map<String, String> tags) {
this.tags = tags;
this.tags = CollectionUtils.newHashMap(tags);
}

public void removeTag(@NotNull String key) {
Expand Down Expand Up @@ -262,7 +263,7 @@ public void setUser(final @Nullable User user) {
}

public void setBreadcrumbs(final @Nullable List<Breadcrumb> breadcrumbs) {
this.breadcrumbs = breadcrumbs;
this.breadcrumbs = CollectionUtils.newArrayList(breadcrumbs);
}

public void addBreadcrumb(final @NotNull Breadcrumb breadcrumb) {
Expand All @@ -278,7 +279,7 @@ Map<String, Object> getExtras() {
}

public void setExtras(final @Nullable Map<String, Object> extra) {
this.extra = extra;
this.extra = CollectionUtils.newHashMap(extra);
}

public void setExtra(final @NotNull String key, final @NotNull Object value) {
Expand Down
6 changes: 4 additions & 2 deletions sentry/src/main/java/io/sentry/SentryEvent.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package io.sentry;

import io.sentry.protocol.*;
import io.sentry.util.CollectionUtils;
import java.util.ArrayList;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -167,7 +169,7 @@ public void setTransaction(final @Nullable String transaction) {
}

public void setFingerprints(final @Nullable List<String> fingerprint) {
this.fingerprint = fingerprint;
this.fingerprint = fingerprint != null ? new ArrayList<>(fingerprint) : null;
}

@ApiStatus.Internal
Expand All @@ -187,7 +189,7 @@ Map<String, String> getModules() {
}

public void setModules(final @Nullable Map<String, String> modules) {
this.modules = modules;
this.modules = CollectionUtils.newHashMap(modules);
}

public void setModule(final @NotNull String key, final @NotNull String value) {
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/SentryValues.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ final class SentryValues<T> {
if (values == null) {
values = new ArrayList<>(0);
}
this.values = values;
this.values = new ArrayList<>(values);
}

public @NotNull List<T> getValues() {
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/SpanContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ void setSampled(final @Nullable Boolean sampled) {
@Override
public SpanContext clone() throws CloneNotSupportedException {
final SpanContext clone = (SpanContext) super.clone();
final Map<String, String> copiedTags = CollectionUtils.shallowCopy(this.tags);
final Map<String, String> copiedTags = CollectionUtils.newConcurrentHashMap(this.tags);
if (copiedTags != null) {
clone.tags = copiedTags;
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/protocol/App.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public void acceptUnknownProperties(@NotNull Map<String, Object> unknown) {
public @NotNull App clone() throws CloneNotSupportedException {
final App clone = (App) super.clone();

clone.unknown = CollectionUtils.shallowCopy(unknown);
clone.unknown = CollectionUtils.newConcurrentHashMap(unknown);

return clone;
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/protocol/Browser.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void acceptUnknownProperties(final @NotNull Map<String, Object> unknown)
public @NotNull Browser clone() throws CloneNotSupportedException {
final Browser clone = (Browser) super.clone();

clone.unknown = CollectionUtils.shallowCopy(unknown);
clone.unknown = CollectionUtils.newConcurrentHashMap(unknown);

return clone;
}
Expand Down
3 changes: 2 additions & 1 deletion sentry/src/main/java/io/sentry/protocol/DebugMeta.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.sentry.protocol;

import io.sentry.IUnknownPropertiesConsumer;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import org.jetbrains.annotations.ApiStatus;
Expand Down Expand Up @@ -32,7 +33,7 @@ public final class DebugMeta implements IUnknownPropertiesConsumer {
}

public void setImages(final @Nullable List<DebugImage> images) {
this.images = images;
this.images = images != null ? new ArrayList<>(images) : null;
}

public @Nullable SdkInfo getSdkInfo() {
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/protocol/Device.java
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ public void acceptUnknownProperties(final @NotNull Map<String, Object> unknown)
final TimeZone timezoneRef = this.timezone;
clone.timezone = timezoneRef != null ? (TimeZone) timezoneRef.clone() : null;

clone.unknown = CollectionUtils.shallowCopy(unknown);
clone.unknown = CollectionUtils.newConcurrentHashMap(unknown);

return clone;
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/protocol/Gpu.java
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public void acceptUnknownProperties(final @NotNull Map<String, Object> unknown)
public @NotNull Gpu clone() throws CloneNotSupportedException {
final Gpu clone = (Gpu) super.clone();

clone.unknown = CollectionUtils.shallowCopy(unknown);
clone.unknown = CollectionUtils.newConcurrentHashMap(unknown);

return clone;
}
Expand Down
5 changes: 3 additions & 2 deletions sentry/src/main/java/io/sentry/protocol/Mechanism.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.sentry.protocol;

import io.sentry.IUnknownPropertiesConsumer;
import io.sentry.util.CollectionUtils;
import java.util.Map;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
Expand Down Expand Up @@ -108,15 +109,15 @@ public void setHandled(final @Nullable Boolean handled) {
}

public void setMeta(final @Nullable Map<String, Object> meta) {
this.meta = meta;
this.meta = CollectionUtils.newHashMap(meta);
}

public @Nullable Map<String, Object> getData() {
return data;
}

public void setData(final @Nullable Map<String, Object> data) {
this.data = data;
this.data = CollectionUtils.newHashMap(data);
}

@Nullable
Expand Down
3 changes: 2 additions & 1 deletion sentry/src/main/java/io/sentry/protocol/Message.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.sentry.protocol;

import io.sentry.IUnknownPropertiesConsumer;
import io.sentry.util.CollectionUtils;
import java.util.List;
import java.util.Map;
import org.jetbrains.annotations.ApiStatus;
Expand Down Expand Up @@ -74,7 +75,7 @@ public void setMessage(final @Nullable String message) {
}

public void setParams(final @Nullable List<String> params) {
this.params = params;
this.params = CollectionUtils.newArrayList(params);
}

@ApiStatus.Internal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void acceptUnknownProperties(final @NotNull Map<String, Object> unknown)
public @NotNull OperatingSystem clone() throws CloneNotSupportedException {
final OperatingSystem clone = (OperatingSystem) super.clone();

clone.unknown = CollectionUtils.shallowCopy(unknown);
clone.unknown = CollectionUtils.newConcurrentHashMap(unknown);

return clone;
}
Expand Down
14 changes: 7 additions & 7 deletions sentry/src/main/java/io/sentry/protocol/Request.java
Original file line number Diff line number Diff line change
Expand Up @@ -144,23 +144,23 @@ public void setCookies(final @Nullable String cookies) {
}

public void setHeaders(final @Nullable Map<String, String> headers) {
this.headers = headers;
this.headers = CollectionUtils.newConcurrentHashMap(headers);
}

public @Nullable Map<String, String> getEnvs() {
return env;
}

public void setEnvs(final @Nullable Map<String, String> env) {
this.env = env;
this.env = CollectionUtils.newConcurrentHashMap(env);
}

public @Nullable Map<String, String> getOthers() {
return other;
}

public void setOthers(final @Nullable Map<String, String> other) {
this.other = other;
this.other = CollectionUtils.newConcurrentHashMap(other);
}

/**
Expand Down Expand Up @@ -190,10 +190,10 @@ public void acceptUnknownProperties(final @NotNull Map<String, Object> unknown)
public @NotNull Request clone() throws CloneNotSupportedException {
final Request clone = (Request) super.clone();

clone.headers = CollectionUtils.shallowCopy(headers);
clone.env = CollectionUtils.shallowCopy(env);
clone.other = CollectionUtils.shallowCopy(other);
clone.unknown = CollectionUtils.shallowCopy(unknown);
clone.headers = CollectionUtils.newConcurrentHashMap(headers);
clone.env = CollectionUtils.newConcurrentHashMap(env);
clone.other = CollectionUtils.newConcurrentHashMap(other);
clone.unknown = CollectionUtils.newConcurrentHashMap(unknown);

return clone;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void acceptUnknownProperties(final @NotNull Map<String, Object> unknown)
public @NotNull SentryRuntime clone() throws CloneNotSupportedException {
final SentryRuntime clone = (SentryRuntime) super.clone();

clone.unknown = CollectionUtils.shallowCopy(unknown);
clone.unknown = CollectionUtils.newConcurrentHashMap(unknown);

return clone;
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/protocol/SentrySpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public SentrySpan(final @NotNull Span span) {
this.parentSpanId = span.getParentSpanId();
this.traceId = span.getTraceId();
this.status = span.getStatus();
final Map<String, String> tagsCopy = CollectionUtils.shallowCopy(span.getTags());
final Map<String, String> tagsCopy = CollectionUtils.newConcurrentHashMap(span.getTags());
this.tags = tagsCopy != null ? tagsCopy : new ConcurrentHashMap<>();
this.timestamp = span.getTimestamp();
this.startTimestamp = span.getStartTimestamp();
Expand Down
10 changes: 3 additions & 7 deletions sentry/src/main/java/io/sentry/protocol/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,7 @@ public void setIpAddress(final @Nullable String ipAddress) {
* @param other the other user related data..
*/
public void setOthers(final @Nullable Map<String, @NotNull String> other) {
if (other != null) {
this.other = new ConcurrentHashMap<>(other);
} else {
this.other = null;
}
this.other = CollectionUtils.newConcurrentHashMap(other);
}

/**
Expand Down Expand Up @@ -164,8 +160,8 @@ public void acceptUnknownProperties(final @NotNull Map<String, Object> unknown)
public @NotNull User clone() throws CloneNotSupportedException {
final User clone = (User) super.clone();

clone.other = CollectionUtils.shallowCopy(other);
clone.unknown = CollectionUtils.shallowCopy(unknown);
clone.other = CollectionUtils.newConcurrentHashMap(other);
clone.unknown = CollectionUtils.newConcurrentHashMap(unknown);

return clone;
}
Expand Down
38 changes: 36 additions & 2 deletions sentry/src/main/java/io/sentry/util/CollectionUtils.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package io.sentry.util;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.jetbrains.annotations.ApiStatus;
Expand Down Expand Up @@ -32,21 +34,53 @@ public static int size(final @NotNull Iterable<?> data) {
}

/**
* Creates a shallow copy of map given by parameter.
* Creates a new {@link ConcurrentHashMap} as a shallow copy of map given by parameter.
*
* @param map the map to copy
* @param <K> the type of map keys
* @param <V> the type of map values
* @return the shallow copy of map
*/
public static <K, V> @Nullable Map<K, @NotNull V> shallowCopy(@Nullable Map<K, @NotNull V> map) {
public static <K, V> @Nullable Map<K, @NotNull V> newConcurrentHashMap(
@Nullable Map<K, @NotNull V> map) {
if (map != null) {
return new ConcurrentHashMap<>(map);
} else {
return null;
}
}

/**
* Creates a new {@link HashMap} as a shallow copy of map given by parameter.
*
* @param map the map to copy
* @param <K> the type of map keys
* @param <V> the type of map values
* @return a new {@link HashMap} or {@code null} if parameter is {@code null}
*/
public static <K, V> @Nullable Map<K, @NotNull V> newHashMap(@Nullable Map<K, @NotNull V> map) {
if (map != null) {
return new HashMap<>(map);
} else {
return null;
}
}

/**
* Creates a new {@link ArrayList} as a shallow copy of list given by parameter.
*
* @param list the list to copy
* @param <T> the type of list entries
* @return a new {@link ArrayList} or {@code null} if parameter is {@code null}
*/
public static <T> @Nullable List<T> newArrayList(@Nullable List<T> list) {
if (list != null) {
return new ArrayList<>(list);
} else {
return null;
}
}

/**
* Returns a new map which entries match a predicate specified by a parameter.
*
Expand Down
11 changes: 11 additions & 0 deletions sentry/src/test/java/io/sentry/ScopeTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,17 @@ class ScopeTest {
}
}

@Test
fun `when setFingerprints receives immutable list as an argument, its still possible to add more fingerprints`() {
val scope = Scope(SentryOptions()).apply {
fingerprint = listOf("a", "b")
fingerprint.add("c")
}
assertNotNull(scope.fingerprint) {
assertEquals(listOf("a", "b", "c"), it)
}
}

private fun eventProcessor(): EventProcessor {
return object : EventProcessor {
override fun process(event: SentryEvent, hint: Any?): SentryEvent? {
Expand Down
Loading