From 20c337772923eed4b227d9752852c10b3ec71529 Mon Sep 17 00:00:00 2001
From: fractalwrench <fractalwrench@gmail.com>
Date: Wed, 16 Sep 2020 16:38:34 +0100
Subject: [PATCH] fix: prevent ConcurrentModificationException thrown from
 Metadata class

---
 CHANGELOG.md                                  |  7 +++++++
 .../main/java/com/bugsnag/android/Metadata.kt | 11 +++++-----
 .../MetadataConcurrentModificationTest.kt     | 21 +++++++++++++++++++
 .../src/test/resources/event_redaction.json   |  6 +++---
 .../test/resources/event_serialization_6.json |  6 +++---
 .../bugsnag/android/MetadataDeserializer.java |  4 +++-
 6 files changed, 42 insertions(+), 13 deletions(-)
 create mode 100644 bugsnag-android-core/src/test/java/com/bugsnag/android/MetadataConcurrentModificationTest.kt

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b3f105213c..79c9fbe8d4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,12 @@
 # Changelog
 
+## TBD
+
+### Bug fixes
+
+* Prevent ConcurrentModificationException thrown from Metadata class
+  [#935](https://github.com/bugsnag/bugsnag-android/pull/935)
+
 ## 5.1.0 (2020-09-08)
 
 ### Enhancements
diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt
index 50e8d51dfa..5c028c7884 100644
--- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt
+++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt
@@ -3,7 +3,6 @@
 package com.bugsnag.android
 
 import java.io.IOException
-import java.util.HashMap
 import java.util.HashSet
 import java.util.concurrent.ConcurrentHashMap
 
@@ -14,7 +13,7 @@ import java.util.concurrent.ConcurrentHashMap
  * Diagnostic information is presented on your Bugsnag dashboard in tabs.
  */
 internal data class Metadata @JvmOverloads constructor(
-    internal val store: MutableMap<String, Any> = ConcurrentHashMap(),
+    internal val store: ConcurrentHashMap<String, Any> = ConcurrentHashMap(),
     val jsonStreamer: ObjectJsonStreamer = ObjectJsonStreamer(),
     val redactedKeys: Set<String> = jsonStreamer.redactedKeys
 ) : JsonStream.Streamable, MetadataAware {
@@ -79,8 +78,8 @@ internal data class Metadata @JvmOverloads constructor(
         }
     }
 
-    fun toMap(): Map<String, Any> {
-        val hashMap = HashMap(store)
+    fun toMap(): ConcurrentHashMap<String, Any> {
+        val hashMap = ConcurrentHashMap(store)
 
         // deep copy each section
         store.entries.forEach {
@@ -106,7 +105,7 @@ internal data class Metadata @JvmOverloads constructor(
             return newMeta
         }
 
-        internal fun mergeMaps(data: List<Map<String, Any>>): MutableMap<String, Any> {
+        internal fun mergeMaps(data: List<Map<String, Any>>): ConcurrentHashMap<String, Any> {
             val keys = data.flatMap { it.keys }.toSet()
             val result = ConcurrentHashMap<String, Any>()
 
@@ -144,7 +143,7 @@ internal data class Metadata @JvmOverloads constructor(
     }
 
     fun copy() = this.copy(
-        store = toMap().toMutableMap(),
+        store = toMap(),
         jsonStreamer = jsonStreamer,
         redactedKeys = redactedKeys
     )
diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/MetadataConcurrentModificationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/MetadataConcurrentModificationTest.kt
new file mode 100644
index 0000000000..f581e2e549
--- /dev/null
+++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/MetadataConcurrentModificationTest.kt
@@ -0,0 +1,21 @@
+package com.bugsnag.android
+
+import org.junit.Assert.assertNotNull
+import org.junit.Test
+import java.util.concurrent.Executors
+
+internal class MetadataConcurrentModificationTest {
+
+    @Test()
+    fun testHandlesConcurrentModification() {
+        val metadata = Metadata().copy()
+        val executor = Executors.newSingleThreadExecutor()
+
+        repeat(100) { count ->
+            assertNotNull(metadata.toMap())
+            executor.execute {
+                metadata.store["$count"] = count
+            }
+        }
+    }
+}
diff --git a/bugsnag-android-core/src/test/resources/event_redaction.json b/bugsnag-android-core/src/test/resources/event_redaction.json
index 9f3e56e671..15ac42f05c 100644
--- a/bugsnag-android-core/src/test/resources/event_redaction.json
+++ b/bugsnag-android-core/src/test/resources/event_redaction.json
@@ -3,10 +3,10 @@
     "app": {
       "password": "[REDACTED]"
     },
-    "device": {
+    "baz": {
       "password": "[REDACTED]"
     },
-    "baz": {
+    "device": {
       "password": "[REDACTED]"
     }
   },
@@ -48,4 +48,4 @@
     }
   ],
   "threads": []
-}
\ No newline at end of file
+}
diff --git a/bugsnag-android-core/src/test/resources/event_serialization_6.json b/bugsnag-android-core/src/test/resources/event_serialization_6.json
index 2a36d875f5..42a75129c5 100644
--- a/bugsnag-android-core/src/test/resources/event_serialization_6.json
+++ b/bugsnag-android-core/src/test/resources/event_serialization_6.json
@@ -3,11 +3,11 @@
     "app": {
       "foo": 55
     },
-    "device": {
-      "bar": true
-    },
     "wham": {
       "some_key": "Avalue"
+    },
+    "device": {
+      "bar": true
     }
   },
   "severity": "warning",
diff --git a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/MetadataDeserializer.java b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/MetadataDeserializer.java
index fdb6dedd47..1aebf3bc16 100644
--- a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/MetadataDeserializer.java
+++ b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/MetadataDeserializer.java
@@ -1,10 +1,12 @@
 package com.bugsnag.android;
 
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 class MetadataDeserializer implements MapDeserializer<Metadata> {
     @Override
     public Metadata deserialize(Map<String, Object> map) {
-        return new Metadata(map);
+        ConcurrentHashMap<String, Object> store = new ConcurrentHashMap<>(map);
+        return new Metadata(store);
     }
 }