Merge pull request #911 from bugsnag/nickdowell/fix-incorrect-version…

…-numbers

[PLAT-5503] Fix app version reported for crashes before v6.2.3

Bugsnag.xcodeproj/project.pbxproj

@@ -1279,6 +1279,7 @@
 		0198762E2567D5AB000A7AF3 /* BugsnagStackframe+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "BugsnagStackframe+Private.h"; sourceTree = "<group>"; };
 		01B14C55251CE55F00118748 /* report-react-native-promise-rejection.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "report-react-native-promise-rejection.json"; sourceTree = "<group>"; };
 		01C17AE62542ED7F00C102C9 /* KSCrashReportWriterTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = KSCrashReportWriterTests.m; sourceTree = "<group>"; };
+		01D8EC3C256FC6C3006F2A2D /* BugsnagConfiguration+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "BugsnagConfiguration+Private.h"; sourceTree = "<group>"; };
 		01E8765C256684E700F4B70A /* URLSessionMock.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = URLSessionMock.h; sourceTree = "<group>"; };
 		01E8765D256684E700F4B70A /* URLSessionMock.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = URLSessionMock.m; sourceTree = "<group>"; };
 		3A700A8024A63A8E0068CD1B /* BugsnagThread.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BugsnagThread.h; sourceTree = "<group>"; };
@@ -1727,6 +1728,7 @@
 				008967CA2486DA2D00DC48C2 /* BSGConfigurationBuilder.h */,
 				008967CC2486DA2D00DC48C2 /* BSGConfigurationBuilder.m */,
 				008967CB2486DA2D00DC48C2 /* BugsnagConfiguration.m */,
+				01D8EC3C256FC6C3006F2A2D /* BugsnagConfiguration+Private.h */,
 				008967C92486DA2D00DC48C2 /* BugsnagEndpointConfiguration.m */,
 				008967CF2486DA2D00DC48C2 /* BugsnagErrorTypes.m */,
 			);

Bugsnag/Client/BugsnagClient.m

@@ -42,6 +42,7 @@
 #import "Bugsnag.h"
 #import "BugsnagBreadcrumbs.h"
 #import "BugsnagCollections.h"
+#import "BugsnagConfiguration+Private.h"
 #import "BugsnagCrashSentry.h"
 #import "BugsnagError+Private.h"
 #import "BugsnagErrorTypes.h"

Bugsnag/Configuration/BugsnagConfiguration+Private.h

@@ -0,0 +1,24 @@
+//
+//  BugsnagConfiguration+Private.h
+//  Bugsnag
+//
+//  Created by Nick Dowell on 26/11/2020.
+//  Copyright © 2020 Bugsnag Inc. All rights reserved.
+//
+
+#import <Bugsnag/BugsnagConfiguration.h>
+
+NS_ASSUME_NONNULL_BEGIN
+
+@interface BugsnagConfiguration ()
+
+/// Initializes the configuration with values previously stored in metadata.
+- (instancetype)initWithMetadata:(NSDictionary *)JSONObject NS_DESIGNATED_INITIALIZER;
+
+/// Throws an NSInvalidArgumentException if the API key is empty or missing.
+/// Logs a warning message if the API key is not in the expected format.
+- (void)validate;
+
+@end
+
+NS_ASSUME_NONNULL_END

Bugsnag/Configuration/BugsnagConfiguration.m

@@ -24,7 +24,7 @@
 // THE SOFTWARE.
 //
 
-#import "BugsnagConfiguration.h"
+#import "BugsnagConfiguration+Private.h"
 
 #import "BSGConfigurationBuilder.h"
 #import "BugsnagApiClient.h"
@@ -231,6 +231,18 @@ - (instancetype)initWithApiKey:(NSString *)apiKey {
     return self;
 }
 
+- (instancetype)initWithMetadata:(NSDictionary *)metadata {
+    if (!(self = [super init])) {
+        return nil;
+    }
+    _appVersion = metadata[BSGKeyAppVersion];
+    _context = metadata[BSGKeyContext];
+    _bundleVersion = metadata[BSGKeyBundleVersion];
+    _enabledReleaseStages = metadata[BSGKeyEnabledReleaseStages];
+    _releaseStage = metadata[BSGKeyReleaseStage];
+    return self;
+}
+
 // -----------------------------------------------------------------------------
 // MARK: - Instance Methods
 // -----------------------------------------------------------------------------

Bugsnag/Payload/BugsnagApp.m

@@ -57,13 +57,10 @@ + (void)populateFields:(BugsnagApp *)app
 {
     NSDictionary *system = event[BSGKeySystem];
     app.id = system[@"CFBundleIdentifier"];
-    app.bundleVersion = [event valueForKeyPath:@"user.config.bundleVersion"] ?:
-            (config.bundleVersion ?: system[@"CFBundleVersion"]);
+    app.bundleVersion = config.bundleVersion ?: system[@"CFBundleVersion"];
     app.dsymUuid = system[@"app_uuid"];
-    // Preferentially take App version values from the event, the config and the system
-    app.version = [event valueForKeyPath:@"user.config.appVersion"] ?:
-        (config.appVersion ?: system[@"CFBundleShortVersionString"]);
-    app.releaseStage = [event valueForKeyPath:@"user.config.releaseStage"] ?: config.releaseStage;
+    app.version = config.appVersion ?: system[@"CFBundleShortVersionString"];
+    app.releaseStage = config.releaseStage;
     app.codeBundleId = [event valueForKeyPath:@"user.state.app.codeBundleId"] ?: codeBundleId;
     app.type = config.appType;
 }

Bugsnag/Payload/BugsnagEvent.m

@@ -21,6 +21,7 @@
 #import "Bugsnag.h"
 #import "BugsnagBreadcrumbs.h"
 #import "BugsnagCollections.h"
+#import "BugsnagConfiguration+Private.h"
 #import "BugsnagError+Private.h"
 #import "BugsnagEvent+Private.h"
 #import "BugsnagHandledState.h"
@@ -328,7 +329,6 @@ - (instancetype)initWithOOMData:(NSDictionary *)event {
  * @return a BugsnagEvent containing the parsed information
  */
 - (instancetype)initWithKSCrashData:(NSDictionary *)event {
-    BugsnagConfiguration *config = [Bugsnag configuration];
     NSDictionary *error = [event valueForKeyPath:@"crash.error"];
     NSString *errorType = error[BSGKeyType];
 
@@ -397,7 +397,12 @@ - (instancetype)initWithKSCrashData:(NSDictionary *)event {
     NSString *deviceAppHash = [event valueForKeyPath:@"system.device_app_hash"];
     BugsnagDeviceWithState *device = [BugsnagDeviceWithState deviceWithDictionary:event];
     BugsnagUser *user = [self parseUser:event deviceAppHash:deviceAppHash deviceId:device.id];
-    BugsnagEvent *obj = [self initWithApp:[BugsnagAppWithState appWithDictionary:event config:config codeBundleId:self.codeBundleId]
+    BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithMetadata:[event valueForKeyPath:@"user.config"]];
+    BugsnagAppWithState *app = [BugsnagAppWithState appWithDictionary:event config:config codeBundleId:self.codeBundleId];
+    if (!app.type) { // Configuration.type does not get stored in the crash report at the time of writing.
+        app.type = [Bugsnag configuration].appType;
+    }
+    BugsnagEvent *obj = [self initWithApp:app
                                    device:device
                              handledState:handledState
                                      user:user

Bugsnag/Private.h

@@ -17,16 +17,6 @@
 
 #pragma mark -
 
-@interface BugsnagConfiguration ()
-
-/// Throws an NSInvalidArgumentException if the API key is empty or missing.
-/// Logs a warning message if the API key is not in the expected format.
-- (void)validate;
-
-@end
-
-#pragma mark -
-
 @interface BugsnagBreadcrumb ()
 
 - (NSDictionary *_Nullable)objectValue;

CHANGELOG.md

@@ -1,6 +1,11 @@
 Changelog
 =========
 
+## TBD
+
+* Fixed incorrect app version reported when sending crash reports from older versions of Bugsnag (before 6.2.3)
+  [#911](https://github.com/bugsnag/bugsnag-cocoa/pull/911)
+
 ## 6.2.6 (2020-11-25)
 
 ### Bug fixes

Tests/BSGConfigurationBuilderTests.m

@@ -1,8 +1,9 @@
 #import <XCTest/XCTest.h>
 
+#import <Bugsnag/Bugsnag.h>
 #import "BSGConfigurationBuilder.h"
+#import "BugsnagConfiguration+Private.h"
 #import "BugsnagTestConstants.h"
-#import "Private.h"
 
 @interface BSGConfigurationBuilderTests : XCTestCase
 @end

Tests/BugsnagAppTest.m

@@ -9,7 +9,7 @@
 #import <XCTest/XCTest.h>
 
 #import "BugsnagAppWithState.h"
-#import "BugsnagConfiguration.h"
+#import "BugsnagConfiguration+Private.h"
 #import "BugsnagTestConstants.h"
 
 @interface BugsnagApp ()
@@ -60,7 +60,7 @@ - (void)setUp {
             }
     };
 
-    self.config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
+    self.config = [[BugsnagConfiguration alloc] initWithMetadata:self.data[@"user"][@"config"]];
     self.config.appType = @"iOS";
     self.config.bundleVersion = nil;
     self.config.appVersion = @"3.14.159";
@@ -202,16 +202,6 @@ - (void)testAppVersionPrecedence {
     self.config.appVersion = @"4.2.6";
     app = [BugsnagAppWithState appWithDictionary:self.data config:self.config codeBundleId:self.codeBundleId];
     XCTAssertEqualObjects(@"4.2.6", app.version);
-
-    // 1st precedence is user.config.appVersion
-    NSMutableDictionary *data = [self.data mutableCopy];
-    data[@"user"] = @{
-            @"config": @{
-                    @"appVersion": @"1.2.3"
-            }
-    };
-    app = [BugsnagAppWithState appWithDictionary:data config:self.config codeBundleId:self.codeBundleId];
-    XCTAssertEqualObjects(@"1.2.3", app.version);
 }
 
 - (void)testBundleVersionPrecedence {
@@ -224,16 +214,6 @@ - (void)testBundleVersionPrecedence {
     self.config.bundleVersion = @"4.2.6";
     app = [BugsnagAppWithState appWithDictionary:self.data config:self.config codeBundleId:self.codeBundleId];
     XCTAssertEqualObjects(@"4.2.6", app.bundleVersion);
-
-    // 1st precedence is user.config.bundleVersion
-    NSMutableDictionary *data = [self.data mutableCopy];
-    data[@"user"] = @{
-            @"config": @{
-                    @"bundleVersion": @"1.2.3"
-            }
-    };
-    app = [BugsnagAppWithState appWithDictionary:data config:self.config codeBundleId:self.codeBundleId];
-    XCTAssertEqualObjects(@"1.2.3", app.bundleVersion);
 }
 
 @end

Tests/BugsnagConfigurationTests.m

@@ -4,11 +4,12 @@
 
 #import <XCTest/XCTest.h>
 
+#import "BSG_KSCrashType.h"
 #import "BugsnagClient+Private.h"
-#import "BugsnagTestConstants.h"
+#import "BugsnagConfiguration+Private.h"
 #import "BugsnagCrashSentry.h"
 #import "BugsnagSessionTracker.h"
-#import "BSG_KSCrashType.h"
+#import "BugsnagTestConstants.h"
 #import "Private.h"
 
 // =============================================================================

Tests/BugsnagErrorReportSinkTests.m

@@ -55,8 +55,6 @@ - (void)setUp {
                                                                    sessions:@"http://localhost:1234"];
     BugsnagClient *client = [[BugsnagClient alloc] initWithConfiguration:config];
     [client start];
-    // required due to BugsnagEvent using global singleton
-    [Bugsnag configuration].bundleVersion = @"3.2";
     BugsnagEvent *report =
             [[BugsnagEvent alloc] initWithKSReport:self.rawReportData];
     self.processedData = [[BugsnagErrorReportSink new] prepareEventPayload:report];
@@ -310,7 +308,7 @@ - (void)testEventApp {
     XCTAssertEqualObjects(app[@"id"], @"net.hockeyapp.CrashProbeiOS");
     XCTAssertNotNil(app[@"type"]);
     XCTAssertEqualObjects(app[@"version"], @"1.0");
-    XCTAssertEqualObjects(app[@"bundleVersion"], @"3.2");
+    XCTAssertEqualObjects(app[@"bundleVersion"], @"1");
     XCTAssertEqualObjects(app[@"releaseStage"], @"production");
     XCTAssertEqualObjects(app[@"dsymUUIDs"], @[@"D0A41830-4FD2-3B02-A23B-0741AD4C7F52"]);
     XCTAssertEqualObjects(app[@"duration"], @4000);

Tests/BugsnagEventFromKSCrashReportTest.m

@@ -112,8 +112,8 @@ - (void)testAddMetadataRemovesValue {
 
 - (void)testAppVersion {
     NSDictionary *dictionary = [self.event toJson];
-    XCTAssertEqualObjects(@"1.0", dictionary[@"app"][@"version"]);
-    XCTAssertEqualObjects(@"3", dictionary[@"app"][@"bundleVersion"]);
+    XCTAssertEqualObjects(dictionary[@"app"][@"version"], @"1.0");
+    XCTAssertEqualObjects(dictionary[@"app"][@"bundleVersion"], @"1");
 }
 
 - (void)testThreadsPopulated {

Tests/BugsnagSessionTest.m

@@ -9,11 +9,11 @@
 #import <XCTest/XCTest.h>
 
 #import "BugsnagApp.h"
+#import "BugsnagConfiguration+Private.h"
 #import "BugsnagDevice.h"
 #import "BugsnagSession.h"
 #import "BugsnagSessionInternal.h"
 #import "BSG_RFC3339DateTool.h"
-#import "BugsnagConfiguration.h"
 #import "BugsnagTestConstants.h"
 
 @interface BugsnagApp ()
@@ -78,7 +78,7 @@ - (BugsnagApp *)generateApp {
             }
     };
 
-    BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
+    BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithMetadata:appData[@"user"][@"config"]];
     config.appType = @"iOS";
     config.bundleVersion = nil;
     return [BugsnagApp appWithDictionary:appData config:config codeBundleId:@"bundle-123"];

Tests/BugsnagSessionTrackingPayloadTest.m

@@ -11,7 +11,7 @@
 #import "BugsnagApp.h"
 #import "BugsnagDevice.h"
 #import "BugsnagSessionTrackingPayload.h"
-#import "BugsnagConfiguration.h"
+#import "BugsnagConfiguration+Private.h"
 #import "BugsnagTestConstants.h"
 #import "BugsnagSessionInternal.h"
 
@@ -71,7 +71,7 @@ - (BugsnagApp *)generateApp {
             }
     };
 
-    BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
+    BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithMetadata:appData[@"user"][@"config"]];
     config.appType = @"iOS";
     config.bundleVersion = nil;
     return [BugsnagApp appWithDictionary:appData config:config codeBundleId:@"bundle-123"];