Merge pull request #1765 from bugsnag/release/v5.28.0

v5.28.0

Author: lemnik

.buildkite/pipeline.full.yml

@@ -388,7 +388,7 @@ steps:
     concurrency_group: 'browserstack-app'
     concurrency_method: eager
 
-  - label: ':android: Android 13 Beta NDK r21 end-to-end tests - batch 1'
+  - label: ':android: Android 13 NDK r21 end-to-end tests - batch 1'
     depends_on: "fixture-r21"
     timeout_in_minutes: 60
     plugins:
@@ -405,15 +405,15 @@ steps:
           - "--exclude=features/full_tests/[^a-k].*.feature"
           - "--app=/app/build/fixture-r21.apk"
           - "--farm=bs"
-          - "--device=ANDROID_13_0_BETA"
+          - "--device=ANDROID_13_0"
           - "--fail-fast"
     env:
       TEST_FIXTURE_SYMBOL_DIR: "build/fixture-r21"
     concurrency: 24
     concurrency_group: 'browserstack-app'
     concurrency_method: eager
 
-  - label: ':android: Android 13 Beta NDK r21 end-to-end tests - batch 2'
+  - label: ':android: Android 13 NDK r21 end-to-end tests - batch 2'
     depends_on: "fixture-r21"
     timeout_in_minutes: 60
     plugins:
@@ -430,7 +430,7 @@ steps:
           - "--exclude=features/full_tests/[^l-z].*.feature"
           - "--app=/app/build/fixture-r21.apk"
           - "--farm=bs"
-          - "--device=ANDROID_13_0_BETA"
+          - "--device=ANDROID_13_0"
           - "--fail-fast"
     env:
       TEST_FIXTURE_SYMBOL_DIR: "build/fixture-r21"

CHANGELOG.md

@@ -1,5 +1,18 @@
 # Changelog
 
+## 5.28.0 (2022-10-13)
+
+### Enhancements
+
+* Bugsnag now supports up to 500 breadcrumbs, with a default max of 100. Note that breadcrumbs will be trimmed
+  (oldest first) if the payload exceeds 1MB.
+  [#1751](https://github.com/bugsnag/bugsnag-android/pull/1751)
+
+### Bug fixes
+
+* Fixed very rare crashes when attempting to unwind NDK stacks over protected memory pages
+  [#1761](https://github.com/bugsnag/bugsnag-android/pull/1761)
+
 ## 5.27.0 (2022-10-06)
 
 ### Enhancements

bugsnag-android-core/detekt-baseline.xml

@@ -15,7 +15,7 @@
     <ID>LongParameterList:DeviceIdStore.kt$DeviceIdStore$( context: Context, deviceIdfile: File = File(context.filesDir, "device-id"), deviceIdGenerator: () -> UUID = { UUID.randomUUID() }, internalDeviceIdfile: File = File(context.filesDir, "internal-device-id"), internalDeviceIdGenerator: () -> UUID = { UUID.randomUUID() }, private val sharedPrefMigrator: SharedPrefMigrator, logger: Logger )</ID>
     <ID>LongParameterList:DeviceWithState.kt$DeviceWithState$( buildInfo: DeviceBuildInfo, jailbroken: Boolean?, id: String?, locale: String?, totalMemory: Long?, runtimeVersions: MutableMap&lt;String, Any>, /** * The number of free bytes of storage available on the device */ var freeDisk: Long?, /** * The number of free bytes of memory available on the device */ var freeMemory: Long?, /** * The orientation of the device when the event occurred: either portrait or landscape */ var orientation: String?, /** * The timestamp on the device when the event occurred */ var time: Date? )</ID>
     <ID>LongParameterList:EventFilenameInfo.kt$EventFilenameInfo.Companion$( obj: Any, uuid: String = UUID.randomUUID().toString(), apiKey: String?, timestamp: Long = System.currentTimeMillis(), config: ImmutableConfig, isLaunching: Boolean? = null )</ID>
-    <ID>LongParameterList:EventInternal.kt$EventInternal$( apiKey: String, breadcrumbs: MutableList&lt;Breadcrumb> = mutableListOf(), discardClasses: Set&lt;String> = setOf(), errors: MutableList&lt;Error> = mutableListOf(), metadata: Metadata = Metadata(), featureFlags: FeatureFlags = FeatureFlags(), originalError: Throwable? = null, projectPackages: Collection&lt;String> = setOf(), severityReason: SeverityReason = SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), threads: MutableList&lt;Thread> = mutableListOf(), user: User = User(), redactionKeys: Set&lt;String>? = null )</ID>
+    <ID>LongParameterList:EventInternal.kt$EventInternal$( apiKey: String, logger: Logger, breadcrumbs: MutableList&lt;Breadcrumb> = mutableListOf(), discardClasses: Set&lt;String> = setOf(), errors: MutableList&lt;Error> = mutableListOf(), metadata: Metadata = Metadata(), featureFlags: FeatureFlags = FeatureFlags(), originalError: Throwable? = null, projectPackages: Collection&lt;String> = setOf(), severityReason: SeverityReason = SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), threads: MutableList&lt;Thread&gt; = mutableListOf(), user: User = User(), redactionKeys: Set&lt;String>? = null )</ID>
     <ID>LongParameterList:EventStorageModule.kt$EventStorageModule$( contextModule: ContextModule, configModule: ConfigModule, dataCollectionModule: DataCollectionModule, bgTaskService: BackgroundTaskService, trackerModule: TrackerModule, systemServiceModule: SystemServiceModule, notifier: Notifier, callbackState: CallbackState )</ID>
     <ID>LongParameterList:NativeStackframe.kt$NativeStackframe$( /** * The name of the method that was being executed */ var method: String?, /** * The location of the source file */ var file: String?, /** * The line number within the source file this stackframe refers to */ var lineNumber: Number?, /** * The address of the instruction where the event occurred. */ var frameAddress: Long?, /** * The address of the function where the event occurred. */ var symbolAddress: Long?, /** * The address of the library where the event occurred. */ var loadAddress: Long?, /** * Whether this frame identifies the program counter */ var isPC: Boolean?, /** * The type of the error */ var type: ErrorType? = null, /** * Identifies the exact build this frame originates from. */ var codeIdentifier: String? = null, )</ID>
     <ID>LongParameterList:StateEvent.kt$StateEvent.Install$( @JvmField val apiKey: String, @JvmField val autoDetectNdkCrashes: Boolean, @JvmField val appVersion: String?, @JvmField val buildUuid: String?, @JvmField val releaseStage: String?, @JvmField val lastRunInfoPath: String, @JvmField val consecutiveLaunchCrashes: Int, @JvmField val sendThreads: ThreadSendPolicy )</ID>
@@ -40,15 +40,14 @@
     <ID>ProtectedMemberInFinalClass:EventInternal.kt$EventInternal$protected fun updateSeverityInternal(severity: Severity)</ID>
     <ID>ProtectedMemberInFinalClass:EventInternal.kt$EventInternal$protected fun updateSeverityReason(@SeverityReason.SeverityReasonType reason: String)</ID>
     <ID>RethrowCaughtException:JsonHelper.kt$JsonHelper$throw ex</ID>
-    <ID>ReturnCount:DefaultDelivery.kt$DefaultDelivery$fun deliver( urlString: String, streamable: JsonStream.Streamable, headers: Map&lt;String, String?> ): DeliveryStatus</ID>
+    <ID>ReturnCount:DefaultDelivery.kt$DefaultDelivery$fun deliver( urlString: String, json: ByteArray, headers: Map&lt;String, String?> ): DeliveryStatus</ID>
     <ID>SwallowedException:BugsnagEventMapper.kt$BugsnagEventMapper$catch (pe: IllegalArgumentException) { ndkDateFormatHolder.get()!!.parse(this) ?: throw IllegalArgumentException("cannot parse date $this") }</ID>
     <ID>SwallowedException:ConnectivityCompat.kt$ConnectivityLegacy$catch (e: NullPointerException) { // in some rare cases we get a remote NullPointerException via Parcel.readException null }</ID>
     <ID>SwallowedException:ContextExtensions.kt$catch (exc: RuntimeException) { null }</ID>
     <ID>SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exc: Exception) { false }</ID>
     <ID>SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get battery status") }</ID>
     <ID>SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get locationStatus") }</ID>
     <ID>SwallowedException:DeviceIdFilePersistence.kt$DeviceIdFilePersistence$catch (exc: OverlappingFileLockException) { Thread.sleep(FILE_LOCK_WAIT_MS) }</ID>
-    <ID>SwallowedException:InternalMetrics.kt$InternalMetrics$catch (exc: Exception) { null }</ID>
     <ID>SwallowedException:JsonHelperTest.kt$JsonHelperTest$catch (e: IllegalArgumentException) { didThrow = true }</ID>
     <ID>SwallowedException:PluginClient.kt$PluginClient$catch (exc: ClassNotFoundException) { logger.d("Plugin '$clz' is not on the classpath - functionality will not be enabled.") null }</ID>
     <ID>ThrowsCount:JsonHelper.kt$JsonHelper$ fun jsonToLong(value: Any?): Long?</ID>

bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ManifestConfigLoaderTest.kt

@@ -45,7 +45,7 @@ class ManifestConfigLoaderTest {
             assertEquals(setOf("password"), redactedKeys)
 
             // misc
-            assertEquals(maxBreadcrumbs, 50)
+            assertEquals(maxBreadcrumbs, 100)
             assertEquals(maxPersistedEvents, 32)
             assertEquals(maxPersistedSessions, 128)
             assertEquals(maxReportedThreads, 200)

bugsnag-android-core/src/main/java/com/bugsnag/android/BreadcrumbInternal.kt

@@ -1,5 +1,7 @@
 package com.bugsnag.android
 
+import com.bugsnag.android.internal.StringUtils
+import com.bugsnag.android.internal.TrimMetrics
 import java.io.IOException
 import java.util.Date
 
@@ -22,6 +24,11 @@ internal class BreadcrumbInternal internal constructor(
         Date()
     )
 
+    internal fun trimMetadataStringsTo(maxStringLength: Int): TrimMetrics {
+        val metadata = this.metadata ?: return TrimMetrics(0, 0)
+        return StringUtils.trimNullableStringValuesTo(maxStringLength, metadata)
+    }
+
     @Throws(IOException::class)
     override fun toStream(writer: JsonStream) {
         writer.beginObject()

bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt

@@ -1,6 +1,7 @@
 package com.bugsnag.android
 
 import com.bugsnag.android.internal.DateUtils
+import com.bugsnag.android.internal.InternalMetricsImpl
 import java.text.DateFormat
 import java.text.SimpleDateFormat
 import java.util.Date
@@ -17,7 +18,7 @@ internal class BugsnagEventMapper(
 
     @Suppress("UNCHECKED_CAST")
     internal fun convertToEventImpl(map: Map<in String, Any?>, apiKey: String): EventInternal {
-        val event = EventInternal(apiKey)
+        val event = EventInternal(apiKey, logger)
 
         // populate exceptions. check this early to avoid unnecessary serialization if
         // no stacktrace was gathered.
@@ -86,6 +87,9 @@ internal class BugsnagEventMapper(
         event.updateSeverityReasonInternal(reason)
         event.normalizeStackframeErrorTypes()
 
+        // populate internalMetrics
+        event.internalMetrics = InternalMetricsImpl(map["usage"] as MutableMap<String, Any>?)
+
         return event
     }
 

bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt

@@ -42,6 +42,7 @@ internal class ConfigInternal(
     var maxPersistedEvents: Int = DEFAULT_MAX_PERSISTED_EVENTS
     var maxPersistedSessions: Int = DEFAULT_MAX_PERSISTED_SESSIONS
     var maxReportedThreads: Int = DEFAULT_MAX_REPORTED_THREADS
+    var maxStringValueLength: Int = DEFAULT_MAX_STRING_VALUE_LENGTH
     var context: String? = null
 
     var redactedKeys: Set<String>
@@ -147,11 +148,12 @@ internal class ConfigInternal(
     }
 
     companion object {
-        private const val DEFAULT_MAX_BREADCRUMBS = 50
+        private const val DEFAULT_MAX_BREADCRUMBS = 100
         private const val DEFAULT_MAX_PERSISTED_SESSIONS = 128
         private const val DEFAULT_MAX_PERSISTED_EVENTS = 32
         private const val DEFAULT_MAX_REPORTED_THREADS = 200
         private const val DEFAULT_LAUNCH_CRASH_THRESHOLD_MS: Long = 5000
+        private const val DEFAULT_MAX_STRING_VALUE_LENGTH = 10000
 
         @JvmStatic
         fun load(context: Context): Configuration = load(context, null)

bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java

@@ -18,7 +18,7 @@
 public class Configuration implements CallbackAware, MetadataAware, UserAware, FeatureFlagAware {
 
     private static final int MIN_BREADCRUMBS = 0;
-    private static final int MAX_BREADCRUMBS = 100;
+    private static final int MAX_BREADCRUMBS = 500;
     private static final int VALID_API_KEY_LEN = 32;
     private static final long MIN_LAUNCH_CRASH_THRESHOLD_MS = 0;
 
@@ -512,7 +512,7 @@ public void setEndpoints(@NonNull EndpointConfiguration endpoints) {
      * Sets the maximum number of breadcrumbs which will be stored. Once the threshold is reached,
      * the oldest breadcrumbs will be deleted.
      *
-     * By default, 50 breadcrumbs are stored: this can be amended up to a maximum of 100.
+     * By default, 100 breadcrumbs are stored: this can be amended up to a maximum of 500.
      */
     public int getMaxBreadcrumbs() {
         return impl.getMaxBreadcrumbs();
@@ -522,7 +522,7 @@ public int getMaxBreadcrumbs() {
      * Sets the maximum number of breadcrumbs which will be stored. Once the threshold is reached,
      * the oldest breadcrumbs will be deleted.
      *
-     * By default, 50 breadcrumbs are stored: this can be amended up to a maximum of 100.
+     * By default, 100 breadcrumbs are stored: this can be amended up to a maximum of 500.
      */
     public void setMaxBreadcrumbs(int maxBreadcrumbs) {
         if (maxBreadcrumbs >= MIN_BREADCRUMBS && maxBreadcrumbs <= MAX_BREADCRUMBS) {
@@ -612,6 +612,32 @@ public void setMaxPersistedSessions(int maxPersistedSessions) {
         }
     }
 
+    /**
+     * Gets the maximum string length in any metadata field. Once the threshold is
+     * reached in a particular string, all excess characters will be deleted.
+     *
+     * By default, the limit is 10,000.
+     */
+    public int getMaxStringValueLength() {
+        return impl.getMaxStringValueLength();
+    }
+
+    /**
+     * Sets the maximum string length in any metadata field. Once the threshold is
+     * reached in a particular string, all excess characters will be deleted.
+     *
+     * By default, the limit is 10,000.
+     */
+    public void setMaxStringValueLength(int maxStringValueLength) {
+        if (maxStringValueLength >= 0) {
+            impl.setMaxStringValueLength(maxStringValueLength);
+        } else {
+            getLogger().e("Invalid configuration value detected. "
+                    + "Option maxStringValueLength should be a positive integer."
+                    + "Supplied value is " + maxStringValueLength);
+        }
+    }
+
     /**
      * Bugsnag uses the concept of "contexts" to help display and group your errors. Contexts
      * represent what was happening in your application at the time an error occurs.

bugsnag-android-core/src/main/java/com/bugsnag/android/DefaultDelivery.kt

@@ -1,45 +1,79 @@
 package com.bugsnag.android
 
 import android.net.TrafficStats
-import java.io.ByteArrayOutputStream
+import com.bugsnag.android.internal.JsonHelper
 import java.io.IOException
-import java.io.PrintWriter
 import java.net.HttpURLConnection
 import java.net.HttpURLConnection.HTTP_BAD_REQUEST
 import java.net.HttpURLConnection.HTTP_CLIENT_TIMEOUT
 import java.net.HttpURLConnection.HTTP_OK
 import java.net.URL
 
-/**
- * Converts a [JsonStream.Streamable] into JSON, placing it in a [ByteArray]
- */
-internal fun serializeJsonPayload(streamable: JsonStream.Streamable): ByteArray {
-    return ByteArrayOutputStream().use { baos ->
-        JsonStream(PrintWriter(baos).buffered()).use(streamable::toStream)
-        baos.toByteArray()
-    }
-}
-
 internal class DefaultDelivery(
     private val connectivity: Connectivity?,
-    val logger: Logger
+    private val apiKey: String,
+    private val maxStringValueLength: Int,
+    private val logger: Logger
 ) : Delivery {
 
+    companion object {
+        // 1MB with some fiddle room in case of encoding overhead
+        const val maxPayloadSize = 999700
+    }
+
     override fun deliver(payload: Session, deliveryParams: DeliveryParams): DeliveryStatus {
-        val status = deliver(deliveryParams.endpoint, payload, deliveryParams.headers)
+        val status = deliver(
+            deliveryParams.endpoint,
+            JsonHelper.serialize(payload),
+            deliveryParams.headers
+        )
         logger.i("Session API request finished with status $status")
         return status
     }
 
+    private fun serializePayload(payload: EventPayload): ByteArray {
+        var json = JsonHelper.serialize(payload)
+        if (json.size <= maxPayloadSize) {
+            return json
+        }
+
+        var event = payload.event
+        if (event == null) {
+            event = MarshalledEventSource(payload.eventFile!!, apiKey, logger).invoke()
+            payload.event = event
+            payload.apiKey = apiKey
+        }
+
+        val (itemsTrimmed, dataTrimmed) = event.impl.trimMetadataStringsTo(maxStringValueLength)
+        event.impl.internalMetrics.setMetadataTrimMetrics(
+            itemsTrimmed,
+            dataTrimmed
+        )
+
+        json = JsonHelper.serialize(payload)
+        if (json.size <= maxPayloadSize) {
+            return json
+        }
+
+        val breadcrumbAndBytesRemovedCounts =
+            event.impl.trimBreadcrumbsBy(json.size - maxPayloadSize)
+        event.impl.internalMetrics.setBreadcrumbTrimMetrics(
+            breadcrumbAndBytesRemovedCounts.itemsTrimmed,
+            breadcrumbAndBytesRemovedCounts.dataTrimmed
+        )
+        return JsonHelper.serialize(payload)
+    }
+
     override fun deliver(payload: EventPayload, deliveryParams: DeliveryParams): DeliveryStatus {
-        val status = deliver(deliveryParams.endpoint, payload, deliveryParams.headers)
+        val json = serializePayload(payload)
+        val status = deliver(deliveryParams.endpoint, json, deliveryParams.headers)
         logger.i("Error API request finished with status $status")
         return status
     }
 
     fun deliver(
         urlString: String,
-        streamable: JsonStream.Streamable,
+        json: ByteArray,
         headers: Map<String, String?>
     ): DeliveryStatus {
 
@@ -50,7 +84,6 @@ internal class DefaultDelivery(
         var conn: HttpURLConnection? = null
 
         try {
-            val json = serializeJsonPayload(streamable)
             conn = makeRequest(URL(urlString), json, headers)
 
             // End the request, get the response code