From 5b20034641d35bd5080e2d237659ae5ac98034e0 Mon Sep 17 00:00:00 2001 From: Delisa Mason Date: Wed, 28 Aug 2019 11:38:12 +0100 Subject: [PATCH] fix(config): Use latest value when checking release stage A serialization bug in crash reports would erroneously allow or prevent crash reports from being delivered when the value of release stage or notify release stages were changed after start() was called, or in a subsequent build of the same app. Some values in config are serialized as nested values, while others are at the "top" level of config when written to disk. This change checks the expected position and fallback value for every config option. Fixes #405 --- CHANGELOG.md | 9 ++ Source/BugsnagCrashReport.m | 17 ++- .../iOSTestApp.xcodeproj/project.pbxproj | 5 +- .../scenarios/ReleaseStageScenarios.swift | 103 ++++++++++++++++++ features/release_stages.feature | 51 +++++++++ 5 files changed, 179 insertions(+), 6 deletions(-) create mode 100644 features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/ReleaseStageScenarios.swift create mode 100644 features/release_stages.feature diff --git a/CHANGELOG.md b/CHANGELOG.md index 417b1fe28..5aa61b95e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,15 @@ Changelog ========= +## TBD + +### Bug fixes + +* Fix bug in `notifyReleaseStages` where if the release stage of a build was + changed after `start()`, only the initial value was used to determine whether + to send a report + [#405](https://github.com/bugsnag/bugsnag-cocoa/issues/405) + ## 5.22.5 (2019-08-14) ### Bug fixes diff --git a/Source/BugsnagCrashReport.m b/Source/BugsnagCrashReport.m index bfee304ee..73c8124f3 100644 --- a/Source/BugsnagCrashReport.m +++ b/Source/BugsnagCrashReport.m @@ -102,6 +102,14 @@ return error[BSGKeyReason] ?: @""; } +id BSGLoadConfigValue(NSDictionary *report, NSString *valueName) { + NSString *keypath = [NSString stringWithFormat:@"user.config.%@", valueName]; + NSString *fallbackKeypath = [NSString stringWithFormat:@"user.config.config.%@", valueName]; + + return [report valueForKeyPath:keypath] + ?: [report valueForKeyPath:fallbackKeypath]; // some custom values are nested +} + NSString *BSGParseContext(NSDictionary *report, NSDictionary *metaData) { id context = [report valueForKeyPath:@"user.overrides.context"]; if ([context isKindOfClass:[NSString class]]) @@ -109,7 +117,7 @@ context = metaData[BSGKeyContext]; if ([context isKindOfClass:[NSString class]]) return context; - context = [report valueForKeyPath:@"user.config.context"]; + context = BSGLoadConfigValue(report, @"context"); if ([context isKindOfClass:[NSString class]]) return context; return nil; @@ -132,7 +140,7 @@ NSString *BSGParseReleaseStage(NSDictionary *report) { return [report valueForKeyPath:@"user.overrides.releaseStage"] - ?: [report valueForKeyPath:@"user.config.releaseStage"]; + ?: BSGLoadConfigValue(report, @"releaseStage"); } BSGSeverity BSGParseSeverity(NSString *severity) { @@ -253,8 +261,7 @@ - (instancetype)initWithKSReport:(NSDictionary *)report } } else { FallbackReportData *fallback = [[FallbackReportData alloc] initWithMetadata:metadata]; - _notifyReleaseStages = - [report valueForKeyPath:@"user.config.notifyReleaseStages"]; + _notifyReleaseStages = BSGLoadConfigValue(report, @"notifyReleaseStages"); _releaseStage = BSGParseReleaseStage(report); _incomplete = report.count == 0; _threads = [report valueForKeyPath:@"crash.threads"]; @@ -276,7 +283,7 @@ - (instancetype)initWithKSReport:(NSDictionary *)report _deviceState = BSGParseDeviceState(report); _device = BSGParseDevice(report); _app = BSGParseApp(report); - _appState = BSGParseAppState(report[BSGKeySystem], [report valueForKeyPath:@"user.config.appVersion"]); + _appState = BSGParseAppState(report[BSGKeySystem], BSGLoadConfigValue(report, @"appVersion")); _groupingHash = BSGParseGroupingHash(report, _metaData); _overrides = [report valueForKeyPath:@"user.overrides"]; _customException = BSGParseCustomException(report, [_errorClass copy], diff --git a/features/fixtures/ios-swift-cocoapods/iOSTestApp.xcodeproj/project.pbxproj b/features/fixtures/ios-swift-cocoapods/iOSTestApp.xcodeproj/project.pbxproj index cdae5d41a..1439a5d13 100644 --- a/features/fixtures/ios-swift-cocoapods/iOSTestApp.xcodeproj/project.pbxproj +++ b/features/fixtures/ios-swift-cocoapods/iOSTestApp.xcodeproj/project.pbxproj @@ -16,6 +16,7 @@ 8A530CCC22FDDBF000F0C108 /* ManyConcurrentNotifyScenario.m in Sources */ = {isa = PBXBuildFile; fileRef = 8A530CCB22FDDBF000F0C108 /* ManyConcurrentNotifyScenario.m */; }; 8A840FBA21AF5C450041DBFA /* SwiftAssertion.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A840FB921AF5C450041DBFA /* SwiftAssertion.swift */; }; 8A98400320FD11BF0023ECD1 /* AutoSessionCustomVersionScenario.m in Sources */ = {isa = PBXBuildFile; fileRef = 8A98400220FD11BF0023ECD1 /* AutoSessionCustomVersionScenario.m */; }; + 8AB1081923301FE600672818 /* ReleaseStageScenarios.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AB1081823301FE600672818 /* ReleaseStageScenarios.swift */; }; 8AB8866420404DD30003E444 /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AB8866320404DD30003E444 /* AppDelegate.swift */; }; 8AB8866620404DD30003E444 /* ViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AB8866520404DD30003E444 /* ViewController.swift */; }; 8AB8866920404DD30003E444 /* Main.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 8AB8866720404DD30003E444 /* Main.storyboard */; }; @@ -89,6 +90,7 @@ 8A840FB921AF5C450041DBFA /* SwiftAssertion.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SwiftAssertion.swift; sourceTree = ""; }; 8A98400120FD11BF0023ECD1 /* AutoSessionCustomVersionScenario.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = AutoSessionCustomVersionScenario.h; sourceTree = ""; }; 8A98400220FD11BF0023ECD1 /* AutoSessionCustomVersionScenario.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = AutoSessionCustomVersionScenario.m; sourceTree = ""; }; + 8AB1081823301FE600672818 /* ReleaseStageScenarios.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ReleaseStageScenarios.swift; sourceTree = ""; }; 8AB8866020404DD30003E444 /* iOSTestApp.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = iOSTestApp.app; sourceTree = BUILT_PRODUCTS_DIR; }; 8AB8866320404DD30003E444 /* AppDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppDelegate.swift; sourceTree = ""; }; 8AB8866520404DD30003E444 /* ViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ViewController.swift; sourceTree = ""; }; @@ -238,6 +240,7 @@ F42953DE2BB41023C0B07F41 /* scenarios */ = { isa = PBXGroup; children = ( + 8AB1081823301FE600672818 /* ReleaseStageScenarios.swift */, 8A56EE8022E22ED80066B9DC /* OOMWillTerminateScenario.h */, 8A56EE7F22E22ED80066B9DC /* OOMWillTerminateScenario.m */, 8A14F0F42282D4AE00337B05 /* ReportBackgroundOOMsEnabledScenario.h */, @@ -455,10 +458,10 @@ F42955E0916B8851F074D9B3 /* UserEmailScenario.swift in Sources */, F4295968571A4118D6A4606A /* UserEnabledScenario.swift in Sources */, F4295A036B228AF608641699 /* UserDisabledScenario.swift in Sources */, - 8A56EE8222E22ED80066B9DC /* OOMWillTerminateScenario.m in Sources */, 8A14F0F62282D4AE00337B05 /* ReportOOMsDisabledScenario.m in Sources */, E7767F13221C21E30006648C /* ResumedSessionScenario.swift in Sources */, 8AEFC73120F8D1A000A78779 /* AutoSessionWithUserScenario.m in Sources */, + 8AB1081923301FE600672818 /* ReleaseStageScenarios.swift in Sources */, 8AF6FD7A225E3FA00056EF9E /* ResumeSessionOOMScenario.m in Sources */, F429565A951303E2C3136D0D /* UserIdScenario.swift in Sources */, 8AEEBBD020FC9E1D00C60763 /* AutoSessionMixedEventsScenario.m in Sources */, diff --git a/features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/ReleaseStageScenarios.swift b/features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/ReleaseStageScenarios.swift new file mode 100644 index 000000000..36a729f57 --- /dev/null +++ b/features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/ReleaseStageScenarios.swift @@ -0,0 +1,103 @@ +import UIKit + +class MagicError : NSError {} + +class NotifyWhenReleaseStageInNotifyReleaseStages : Scenario { + + override func startBugsnag() { + self.config.shouldAutoCaptureSessions = false; + self.config.releaseStage = "prod" + self.config.notifyReleaseStages = ["dev", "prod"] + super.startBugsnag() + } + + override func run() { + Bugsnag.notifyError(MagicError(domain: "com.example", + code: 43, + userInfo: [NSLocalizedDescriptionKey: "incoming!"])) + } +} + +class CrashWhenReleaseStageInNotifyReleaseStages : Scenario { + + override func startBugsnag() { + self.config.shouldAutoCaptureSessions = false; + self.config.releaseStage = "prod" + self.config.notifyReleaseStages = ["dev", "prod"] + super.startBugsnag() + } + + override func run() { + abort(); + } +} + +class CrashWhenReleaseStageInNotifyReleaseStagesChanges : Scenario { + + override func startBugsnag() { + self.config.shouldAutoCaptureSessions = false; + if (self.eventMode == "noevent") { + // The event is evaluated whether to be sent + self.config.releaseStage = "test" + } else { + // A crash will occur + self.config.releaseStage = "prod" + } + self.config.notifyReleaseStages = ["dev", "prod"] + super.startBugsnag() + } + + override func run() { + abort(); + } +} + +class CrashWhenReleaseStageNotInNotifyReleaseStagesChanges : Scenario { + + override func startBugsnag() { + self.config.shouldAutoCaptureSessions = false; + if (self.eventMode == "noevent") { + // The event is evaluated whether to be sent + self.config.releaseStage = "prod" + } else { + // A crash will occur + self.config.releaseStage = "test" + } + self.config.notifyReleaseStages = ["dev", "prod"] + super.startBugsnag() + } + + override func run() { + abort(); + } +} + +class NotifyWhenReleaseStageNotInNotifyReleaseStages : Scenario { + + override func startBugsnag() { + self.config.shouldAutoCaptureSessions = false; + self.config.releaseStage = "dev" + self.config.notifyReleaseStages = ["prod"] + super.startBugsnag() + } + + override func run() { + Bugsnag.notifyError(MagicError(domain: "com.example", + code: 43, + userInfo: [NSLocalizedDescriptionKey: "incoming!"])) + } +} + +class CrashWhenReleaseStageNotInNotifyReleaseStages : Scenario { + + override func startBugsnag() { + self.config.shouldAutoCaptureSessions = false; + self.config.releaseStage = "dev" + self.config.notifyReleaseStages = ["prod"] + super.startBugsnag() + } + + override func run() { + abort(); + } +} diff --git a/features/release_stages.feature b/features/release_stages.feature new file mode 100644 index 000000000..b859f00c6 --- /dev/null +++ b/features/release_stages.feature @@ -0,0 +1,51 @@ +Feature: Discarding reports based on release stage + + Scenario: Crash when release stage is not present in "notify release stages" + When I crash the app using "CrashWhenReleaseStageNotInNotifyReleaseStages" + And I relaunch the app + And I wait for 10 seconds + Then I should receive 0 requests + + Scenario: Crash when release stage is present in "notify release stages" + When I crash the app using "CrashWhenReleaseStageInNotifyReleaseStages" + And I relaunch the app + And I wait for a request + Then the request is valid for the error reporting API + And the exception "errorClass" equals "SIGABRT" + And the event "unhandled" is true + And the event "app.releaseStage" equals "prod" + + Scenario: Crash when release stage is changed to not present in "notify release stages" before the event + If the current run has a different release stage than the crashing context, + the report should only be sent if the release stage was in "notify release stages" + at the time of the crash. Release stages can change for a single build of an app + if the app is used as a test harness or if the build can receive code updates, + such as JavaScript execution contexts. + + When I crash the app using "CrashWhenReleaseStageNotInNotifyReleaseStagesChanges" + And I relaunch the app + And I wait for 10 seconds + Then I should receive 0 requests + + Scenario: Crash when release stage is changed to be present in "notify release stages" before the event + When I crash the app using "CrashWhenReleaseStageInNotifyReleaseStagesChanges" + And I relaunch the app + And I wait for a request + Then the request is valid for the error reporting API + And the exception "errorClass" equals "SIGABRT" + And the event "unhandled" is true + And the event "app.releaseStage" equals "prod" + + Scenario: Notify when release stage is not present in "notify release stages" + When I run "NotifyWhenReleaseStageNotInNotifyReleaseStages" + And I wait for 10 seconds + Then I should receive 0 requests + + Scenario: Notify when release stage is present in "notify release stages" + When I run "NotifyWhenReleaseStageInNotifyReleaseStages" + And I wait for a request + Then the request is valid for the error reporting API + And the exception "errorClass" equals "iOSTestApp.MagicError" + And the exception "message" equals "incoming!" + And the event "unhandled" is false + And the event "app.releaseStage" equals "prod"