From 6abe31af4b341a1c549e4aa0516ae86418c6f321 Mon Sep 17 00:00:00 2001 From: Delisa Mason Date: Mon, 1 Jul 2019 13:15:52 +0100 Subject: [PATCH] fix(ndk): Skip null metadata values from serialization In the event that a metadata value is null, the patched section would crash in GetStringUTFChars(). This is most easily reproduced by calling notify/throwing on an exception with no message. --- CHANGELOG.md | 8 +++++++ .../android/mazerunner/SomeException.kt | 4 ++++ .../HandledExceptionWithoutMessage.kt | 21 +++++++++++++++++++ features/handled_exception.feature | 11 ++++++++++ sdk/src/main/jni/metadata.c | 21 ++++++++++++------- .../android/mazerunner/SomeException.kt | 4 ++++ .../HandledExceptionWithoutMessage.kt | 21 +++++++++++++++++++ tests/features/handled_exception.feature | 9 ++++++++ 8 files changed, 91 insertions(+), 8 deletions(-) create mode 100644 features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/SomeException.kt create mode 100644 features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledExceptionWithoutMessage.kt create mode 100644 tests/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/SomeException.kt create mode 100644 tests/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledExceptionWithoutMessage.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 1af72cca73..b6e462800e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## TBD + +### Bug fixes + +* Fix abort() in native code when storing breadcrumbs with null values in + metadata + [#510](https://github.com/bugsnag/bugsnag-android/pull/510) + ## 4.15.0 (2019-06-10) `bugsnag-android` now supports detecting and reporting C/C++ crashes without a separate library (previously `bugsnag-android-ndk` was required for native error reporting). diff --git a/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/SomeException.kt b/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/SomeException.kt new file mode 100644 index 0000000000..36874901ba --- /dev/null +++ b/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/SomeException.kt @@ -0,0 +1,4 @@ +package com.bugsnag.android.mazerunner + +internal class SomeException() : Exception() { +} diff --git a/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledExceptionWithoutMessage.kt b/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledExceptionWithoutMessage.kt new file mode 100644 index 0000000000..92c50c9d33 --- /dev/null +++ b/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledExceptionWithoutMessage.kt @@ -0,0 +1,21 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.mazerunner.SomeException + +/** + * Sends a handled exception to Bugsnag, which does not include session data. + */ +internal class HandledExceptionWithoutMessageScenario(config: Configuration, + context: Context) : Scenario(config, context) { + init { + config.setAutoCaptureSessions(false) + } + + override fun run() { + super.run() + Bugsnag.notify(SomeException()) + } +} diff --git a/features/handled_exception.feature b/features/handled_exception.feature index 0770b04521..d5d3270ca7 100644 --- a/features/handled_exception.feature +++ b/features/handled_exception.feature @@ -11,6 +11,17 @@ Scenario: Test handled Kotlin Exception And the exception "message" equals "HandledExceptionScenario" And the payload field "events.0.device.cpuAbi" is a non-empty array for request 0 +Scenario: Report a handled exception without a message + When I run "HandledExceptionWithoutMessageScenario" + Then I should receive a request + And the request is valid for the error reporting API + And the "Bugsnag-API-Key" header equals "a35a2a72bd230ac0aa0f52715bbdc6aa" + And the payload field "notifier.name" equals "Android Bugsnag Notifier" + And the payload field "events" is an array with 1 element + And the exception "errorClass" equals "com.bugsnag.android.mazerunner.SomeException" + And the event "exceptions.0.message" is null + And the payload field "events.0.device.cpuAbi" is a non-empty array for request 0 + Scenario: Test handled Java Exception When I run "HandledExceptionJavaScenario" Then I should receive a request diff --git a/sdk/src/main/jni/metadata.c b/sdk/src/main/jni/metadata.c index 2a561ef660..b84426e2e5 100644 --- a/sdk/src/main/jni/metadata.c +++ b/sdk/src/main/jni/metadata.c @@ -189,14 +189,19 @@ void bsg_populate_crumb_metadata(JNIEnv *env, bugsnag_breadcrumb *crumb, jni_cache->arraylist_get, (jint)i); jstring _value = (*env)->CallObjectMethod(env, metadata, jni_cache->map_get, _key); - char *key = (char *)(*env)->GetStringUTFChars(env, _key, 0); - char *value = (char *)(*env)->GetStringUTFChars(env, _value, 0); - bsg_strncpy_safe(crumb->metadata[i].key, key, - sizeof(crumb->metadata[i].key)); - bsg_strncpy_safe(crumb->metadata[i].value, value, - sizeof(crumb->metadata[i].value)); - (*env)->ReleaseStringUTFChars(env, _key, key); - (*env)->ReleaseStringUTFChars(env, _value, value); + if (_value == NULL) { + (*env)->DeleteLocalRef(env, _key); + (*env)->DeleteLocalRef(env, _value); + } else { + char *key = (char *)(*env)->GetStringUTFChars(env, _key, 0); + char *value = (char *)(*env)->GetStringUTFChars(env, _value, 0); + bsg_strncpy_safe(crumb->metadata[i].key, key, + sizeof(crumb->metadata[i].key)); + bsg_strncpy_safe(crumb->metadata[i].value, value, + sizeof(crumb->metadata[i].value)); + (*env)->ReleaseStringUTFChars(env, _key, key); + (*env)->ReleaseStringUTFChars(env, _value, value); + } } free(jni_cache); (*env)->DeleteLocalRef(env, keyset); diff --git a/tests/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/SomeException.kt b/tests/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/SomeException.kt new file mode 100644 index 0000000000..36874901ba --- /dev/null +++ b/tests/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/SomeException.kt @@ -0,0 +1,4 @@ +package com.bugsnag.android.mazerunner + +internal class SomeException() : Exception() { +} diff --git a/tests/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledExceptionWithoutMessage.kt b/tests/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledExceptionWithoutMessage.kt new file mode 100644 index 0000000000..92c50c9d33 --- /dev/null +++ b/tests/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledExceptionWithoutMessage.kt @@ -0,0 +1,21 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.mazerunner.SomeException + +/** + * Sends a handled exception to Bugsnag, which does not include session data. + */ +internal class HandledExceptionWithoutMessageScenario(config: Configuration, + context: Context) : Scenario(config, context) { + init { + config.setAutoCaptureSessions(false) + } + + override fun run() { + super.run() + Bugsnag.notify(SomeException()) + } +} diff --git a/tests/features/handled_exception.feature b/tests/features/handled_exception.feature index b823a15907..a471c2e709 100644 --- a/tests/features/handled_exception.feature +++ b/tests/features/handled_exception.feature @@ -9,6 +9,15 @@ Scenario: Test handled Kotlin Exception And the exception "message" equals "HandledExceptionScenario" And the payload field "events.0.device.cpuAbi" is a non-empty array +Scenario: Report a handled exception without a message + When I run "HandledExceptionWithoutMessageScenario" + Then I wait to receive a request + And the request is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the payload field "events" is an array with 1 element + And the exception "errorClass" equals "com.bugsnag.android.mazerunner.SomeException" + And the event "exceptions.0.message" is null + And the payload field "events.0.device.cpuAbi" is a non-empty array + Scenario: Test handled Java Exception When I run "HandledExceptionJavaScenario" Then I wait to receive a request