Fix handling immutable collections on SentryEvent and protocol objects (

#1468)
getsentry · May 12, 2021 · 93f3c22 · 93f3c22
1 parent 112c46d
commit 93f3c22
Show file tree

Hide file tree

Showing 29 changed files with 267 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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

diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api
@@ -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
 }
 

diff --git a/sentry/src/main/java/io/sentry/Breadcrumb.java b/sentry/src/main/java/io/sentry/Breadcrumb.java
@@ -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 =

diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java
@@ -214,7 +214,7 @@ public void setFingerprint(final @NotNull List<String> fingerprint) {
     if (fingerprint == null) {
       return;
     }
-    this.fingerprint = fingerprint;
+    this.fingerprint = new ArrayList<>(fingerprint);
   }
 
   /**

diff --git a/sentry/src/main/java/io/sentry/SentryBaseEvent.java b/sentry/src/main/java/io/sentry/SentryBaseEvent.java
@@ -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;
@@ -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) {
@@ -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) {
@@ -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) {

diff --git a/sentry/src/main/java/io/sentry/SentryEvent.java b/sentry/src/main/java/io/sentry/SentryEvent.java
@@ -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;
@@ -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
@@ -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) {

diff --git a/sentry/src/main/java/io/sentry/SentryValues.java b/sentry/src/main/java/io/sentry/SentryValues.java
@@ -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() {

diff --git a/sentry/src/main/java/io/sentry/SpanContext.java b/sentry/src/main/java/io/sentry/SpanContext.java
@@ -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;
     }

diff --git a/sentry/src/main/java/io/sentry/protocol/App.java b/sentry/src/main/java/io/sentry/protocol/App.java
@@ -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;
   }

diff --git a/sentry/src/main/java/io/sentry/protocol/Browser.java b/sentry/src/main/java/io/sentry/protocol/Browser.java
@@ -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;
   }

diff --git a/sentry/src/main/java/io/sentry/protocol/DebugMeta.java b/sentry/src/main/java/io/sentry/protocol/DebugMeta.java
@@ -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;
@@ -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() {

diff --git a/sentry/src/main/java/io/sentry/protocol/Device.java b/sentry/src/main/java/io/sentry/protocol/Device.java
@@ -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;
   }

diff --git a/sentry/src/main/java/io/sentry/protocol/Gpu.java b/sentry/src/main/java/io/sentry/protocol/Gpu.java
@@ -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;
   }

diff --git a/sentry/src/main/java/io/sentry/protocol/Mechanism.java b/sentry/src/main/java/io/sentry/protocol/Mechanism.java
@@ -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;
@@ -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

diff --git a/sentry/src/main/java/io/sentry/protocol/Message.java b/sentry/src/main/java/io/sentry/protocol/Message.java
@@ -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;
@@ -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

diff --git a/sentry/src/main/java/io/sentry/protocol/OperatingSystem.java b/sentry/src/main/java/io/sentry/protocol/OperatingSystem.java
@@ -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;
   }

diff --git a/sentry/src/main/java/io/sentry/protocol/Request.java b/sentry/src/main/java/io/sentry/protocol/Request.java
@@ -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);
   }
 
   /**
@@ -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;
   }

diff --git a/sentry/src/main/java/io/sentry/protocol/SentryRuntime.java b/sentry/src/main/java/io/sentry/protocol/SentryRuntime.java
@@ -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;
   }

diff --git a/sentry/src/main/java/io/sentry/protocol/SentrySpan.java b/sentry/src/main/java/io/sentry/protocol/SentrySpan.java
@@ -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();

diff --git a/sentry/src/main/java/io/sentry/protocol/User.java b/sentry/src/main/java/io/sentry/protocol/User.java
@@ -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);
   }
 
   /**
@@ -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;
   }

diff --git a/sentry/src/main/java/io/sentry/util/CollectionUtils.java b/sentry/src/main/java/io/sentry/util/CollectionUtils.java
@@ -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;
@@ -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.
    *

diff --git a/sentry/src/test/java/io/sentry/ScopeTest.kt b/sentry/src/test/java/io/sentry/ScopeTest.kt
@@ -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? {