Merge a7be328 into 59720bf

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## 4.X.X (TBD)
+
+### Bug fixes
+
+* Handle null values in MetaData.mergeMaps, preventing potential NPE
+ [#386](https://github.com/bugsnag/bugsnag-android/pull/386)
+
+
 ## 4.9.2 (2018-11-07)
 
 ### Bug fixes

features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/MetaDataNestedNullScenario.kt

@@ -0,0 +1,35 @@
+package com.bugsnag.android.mazerunner.scenarios
+
+import android.content.Context
+import com.bugsnag.android.Bugsnag
+import com.bugsnag.android.Configuration
+import java.util.HashMap
+
+/**
+ * Sends a handled exception to Bugsnag, which includes a nested null value in a metadata map
+ */
+internal class MetaDataNestedNullScenario(
+    config: Configuration,
+    context: Context
+) : Scenario(config, context) {
+
+    init {
+        config.setAutoCaptureSessions(false)
+
+    }
+
+    override fun run() {
+        super.run()
+
+        val configMap = HashMap<String, Any?>()
+        configMap["test"] = null
+        Bugsnag.addToTab("Custom", "foo", configMap)
+
+        Bugsnag.notify(RuntimeException("MetaDataScenario")) {
+            val map = HashMap<String, Any?>()
+            map["test"] = null
+            it.error?.addToTab("Custom", "foo", map)
+        }
+    }
+
+}

features/meta_data_scenario.feature

@@ -5,3 +5,8 @@ Scenario: Sends a handled exception which includes custom metadata added in a no
     Then I should receive a request
     And the request is a valid for the error reporting API
     And the event "metaData.Custom.foo" equals "Hello World!"
+
+Scenario: Add nested null value to metadata tab
+    When I run "MetaDataNestedNullScenario"
+    Then I should receive a request
+    And the request is a valid for the error reporting API

sdk/src/androidTest/java/com/bugsnag/android/MetaDataMergeTest.java

@@ -0,0 +1,23 @@
+package com.bugsnag.android;
+
+import static org.junit.Assert.assertFalse;
+
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class MetaDataMergeTest {
+
+    @Test
+    public void testConcurrentMapMerge() {
+        MetaData metaData = new MetaData();
+
+        Map<Object, Object> nestedMap = new HashMap<>();
+        metaData.addToTab("foo", "bar", nestedMap);
+        nestedMap.put("whoops", null);
+
+        Map<String, Object> mergedMap = MetaData.merge(metaData, metaData).store;
+        assertFalse(mergedMap.containsKey("whoops"));
+    }
+}

sdk/src/main/java/com/bugsnag/android/MetaData.java

@@ -146,9 +146,7 @@ private static Map<String, Object> mergeMaps(@NonNull Map<String, Object>... map
                 Object overridesValue = map.get(key);
 
                 if (overridesValue != null) {
-                    if (baseValue != null
-                        && baseValue instanceof Map
-                        && overridesValue instanceof Map) {
+                    if (baseValue instanceof Map && overridesValue instanceof Map) {
                         // Both original and overrides are Maps, go deeper
                         @SuppressWarnings("unchecked")
                         Map<String, Object> first = (Map<String, Object>) baseValue;
@@ -159,8 +157,9 @@ private static Map<String, Object> mergeMaps(@NonNull Map<String, Object>... map
                         result.put(key, overridesValue);
                     }
                 } else {
-                    // No collision, just use base value
-                    result.put(key, baseValue);
+                    if (baseValue != null) { // No collision, just use base value
+                        result.put(key, baseValue);
+                    }
                 }
             }
         }