[PLAT-6896] Fix NSNull handling

Bugsnag/Delivery/BSGEventUploadOperation.m

@@ -9,6 +9,7 @@
 #import "BSGEventUploadOperation.h"
 
 #import "BSGFileLocations.h"
+#import "BSGInternalErrorReporter.h"
 #import "BSG_RFC3339DateTool.h"
 #import "BugsnagAppWithState+Private.h"
 #import "BugsnagConfiguration+Private.h"
@@ -174,9 +175,16 @@ - (void)start {
     self.state = BSGEventUploadOperationStateExecuting;
     [self didChangeValueForKey:NSStringFromSelector(@selector(isExecuting))];
 
-    [self runWithDelegate:delegate completionHandler:^{
+    @try {
+        [self runWithDelegate:delegate completionHandler:^{
+            [self setFinished];
+        }];
+    } @catch (NSException *exception) {
+        [BSGInternalErrorReporter.sharedInstance reportException:exception diagnostics:nil groupingHash:
+         [NSString stringWithFormat:@"BSGEventUploadOperation -[runWithDelegate:completionHandler:] %@ %@",
+          exception.name, exception.reason]];
         [self setFinished];
-    }];
+    }
 }
 
 - (void)setFinished {

Bugsnag/Helpers/BSGInternalErrorReporter.h

@@ -55,13 +55,21 @@ FOUNDATION_EXPORT NSString *BSGErrorDescription(NSError *error);
                  diagnostics:(nullable NSDictionary<NSString *, id> *)diagnostics
                 groupingHash:(nullable NSString *)groupingHash;
 
+- (void)reportException:(NSException *)exception
+            diagnostics:(nullable NSDictionary<NSString *, id> *)diagnostics
+           groupingHash:(nullable NSString *)groupingHash;
+
 // Private
 
 - (nullable BugsnagEvent *)eventWithErrorClass:(NSString *)errorClass
                                        message:(nullable NSString *)message
                                    diagnostics:(nullable NSDictionary<NSString *, id> *)diagnostics
                                   groupingHash:(nullable NSString *)groupingHash;
 
+- (nullable BugsnagEvent *)eventWithException:(NSException *)exception
+                                  diagnostics:(nullable NSDictionary<NSString *, id> *)diagnostics
+                                 groupingHash:(nullable NSString *)groupingHash;
+
 - (nullable NSURLRequest *)requestForEvent:(BugsnagEvent *)event error:(NSError * __autoreleasing *)errorPtr;
 
 @end

Bugsnag/Helpers/BSGInternalErrorReporter.m

@@ -84,12 +84,57 @@ - (void)reportErrorWithClass:(NSString *)errorClass
     }
 }
 
+- (void)reportException:(NSException *)exception
+            diagnostics:(nullable NSDictionary<NSString *, id> *)diagnostics
+           groupingHash:(nullable NSString *)groupingHash {
+    @try {
+        BugsnagEvent *event = [self eventWithException:exception diagnostics:diagnostics groupingHash:groupingHash];
+        if (event) {
+            [self sendEvent:event];
+        }
+    } @catch (NSException *exception) {
+        bsg_log_err(@"%@", exception);
+    }
+}
+
 // MARK: Private API
 
 - (nullable BugsnagEvent *)eventWithErrorClass:(NSString *)errorClass
                                        message:(nullable NSString *)message
                                    diagnostics:(nullable NSDictionary<NSString *, id> *)diagnostics
                                   groupingHash:(nullable NSString *)groupingHash {
+
+    NSArray<BugsnagStackframe *> *stacktrace = [BugsnagStackframe stackframesWithCallStackReturnAddresses:
+                                                BSGArraySubarrayFromIndex(NSThread.callStackReturnAddresses, 2)];
+
+    BugsnagError *error =
+    [[BugsnagError alloc] initWithErrorClass:errorClass
+                                errorMessage:message
+                                   errorType:BSGErrorTypeCocoa
+                                  stacktrace:stacktrace];
+
+    return [self eventWithError:error diagnostics:diagnostics groupingHash:groupingHash];
+}
+
+- (nullable BugsnagEvent *)eventWithException:(NSException *)exception
+                                  diagnostics:(nullable NSDictionary<NSString *, id> *)diagnostics
+                                 groupingHash:(nullable NSString *)groupingHash {
+
+    NSArray<BugsnagStackframe *> *stacktrace = [BugsnagStackframe stackframesWithCallStackReturnAddresses:exception.callStackReturnAddresses];
+
+    BugsnagError *error =
+    [[BugsnagError alloc] initWithErrorClass:exception.name
+                                errorMessage:exception.reason
+                                   errorType:BSGErrorTypeCocoa
+                                  stacktrace:stacktrace];
+
+    return [self eventWithError:error diagnostics:diagnostics groupingHash:groupingHash];
+}
+
+- (nullable BugsnagEvent *)eventWithError:(BugsnagError *)error
+                              diagnostics:(nullable NSDictionary<NSString *, id> *)diagnostics
+                             groupingHash:(nullable NSString *)groupingHash {
+
     id<BSGInternalErrorReporterDataSource> dataSource = self.dataSource;
     if (!dataSource) {
         return nil;
@@ -101,15 +146,6 @@ - (nullable BugsnagEvent *)eventWithErrorClass:(NSString *)errorClass
     }
     [metadata addMetadata:dataSource.configuration.apiKey withKey:BSGKeyApiKey toSection:BugsnagDiagnosticsKey];
 
-    NSArray<BugsnagStackframe *> *stacktrace = [BugsnagStackframe stackframesWithCallStackReturnAddresses:
-                                                BSGArraySubarrayFromIndex(NSThread.callStackReturnAddresses, 2)];
-
-    BugsnagError *error =
-    [[BugsnagError alloc] initWithErrorClass:errorClass
-                                errorMessage:message
-                                   errorType:BSGErrorTypeCocoa
-                                  stacktrace:stacktrace];
-
     NSDictionary *systemInfo = [BSG_KSSystemInfo systemInfo];
 
     BugsnagEvent *event =
@@ -128,6 +164,8 @@ - (nullable BugsnagEvent *)eventWithErrorClass:(NSString *)errorClass
     return event;
 }
 
+// MARK: Delivery
+
 - (NSURLRequest *)requestForEvent:(nonnull BugsnagEvent *)event error:(NSError * __autoreleasing *)errorPtr {
     id<BSGInternalErrorReporterDataSource> dataSource = self.dataSource;
     if (!dataSource) {

Bugsnag/Helpers/BugsnagCollections.h

@@ -71,4 +71,6 @@ NSString * _Nullable BSGDeserializeString(id _Nullable rawValue);
 
 NSDate * _Nullable BSGDeserializeDate(id _Nullable rawValue);
 
+NSNumber * _Nullable BSGDeserializeNumber(id  _Nullable rawValue);
+
 NS_ASSUME_NONNULL_END

Bugsnag/Helpers/BugsnagCollections.m

@@ -151,3 +151,10 @@ id _Nullable BSGDeserializeArrayOfObjects(id _Nullable rawValue, id _Nullable (^
     }
     return [BSG_RFC3339DateTool dateFromString:(NSString *)rawValue];
 }
+
+NSNumber * _Nullable BSGDeserializeNumber(id  _Nullable rawValue) {
+    if (![rawValue isKindOfClass:[NSNumber class]]) {
+        return nil;
+    }
+    return (NSNumber *)rawValue;
+}

Bugsnag/Payload/BugsnagError.m

@@ -112,23 +112,13 @@ - (instancetype)initWithErrorClass:(NSString *)errorClass
 }
 
 + (BugsnagError *)errorFromJson:(NSDictionary *)json {
-    NSArray *trace = json[BSGKeyStacktrace];
-    NSMutableArray *data = [NSMutableArray new];
-
-    if (trace != nil) {
-        for (NSDictionary *dict in trace) {
-            BugsnagStackframe *frame = [BugsnagStackframe frameFromJson:dict];
-
-            if (frame != nil) {
-                [data addObject:frame];
-            }
-        }
-    }
     BugsnagError *error = [[BugsnagError alloc] init];
-    error.errorClass = json[BSGKeyErrorClass];
-    error.errorMessage = json[BSGKeyMessage];
-    error.stacktrace = data;
-    error.typeString = json[BSGKeyType] ?: BSGErrorTypeStringCocoa;
+    error.errorClass = BSGDeserializeString(json[BSGKeyErrorClass]);
+    error.errorMessage = BSGDeserializeString(json[BSGKeyMessage]);
+    error.stacktrace = BSGDeserializeArrayOfObjects(json[BSGKeyStacktrace], ^id _Nullable(NSDictionary * _Nonnull dict) {
+        return [BugsnagStackframe frameFromJson:dict];
+    }) ?: @[];
+    error.typeString = BSGDeserializeString(json[BSGKeyType]) ?: BSGErrorTypeStringCocoa;
     return error;
 }
 

Bugsnag/Payload/BugsnagStackframe+Private.h

@@ -31,6 +31,13 @@ NS_ASSUME_NONNULL_BEGIN
 
 @property (nonatomic) BOOL needsSymbolication;
 
+// MARK: - Properties not used for Cocoa stack frames, but used by React Native and Unity.
+
+@property (strong, nullable, nonatomic) NSNumber *columnNumber;
+@property (copy, nullable, nonatomic) NSString *file;
+@property (strong, nullable, nonatomic) NSNumber *inProject;
+@property (strong, nullable, nonatomic) NSNumber *lineNumber;
+
 @end
 
 NS_ASSUME_NONNULL_END

Bugsnag/Payload/BugsnagStackframe.m

@@ -22,18 +22,6 @@
 }
 
 
-// MARK: - Properties not used for Cocoa stack frames, but used by React Native and Unity.
-
-@interface BugsnagStackframe ()
-
-@property (strong, nullable, nonatomic) NSNumber *columnNumber;
-@property (copy, nullable, nonatomic) NSString *file;
-@property (strong, nullable, nonatomic) NSNumber *inProject;
-@property (strong, nullable, nonatomic) NSNumber *lineNumber;
-
-@end
-
-
 // MARK: -
 
 @implementation BugsnagStackframe
@@ -49,26 +37,26 @@ + (NSDictionary *_Nullable)findImageAddr:(unsigned long)addr inImages:(NSArray *
 
 + (BugsnagStackframe *)frameFromJson:(NSDictionary *)json {
     BugsnagStackframe *frame = [BugsnagStackframe new];
-    frame.machoFile = json[BSGKeyMachoFile];
-    frame.method = json[BSGKeyMethod];
-    frame.isPc = [json[BSGKeyIsPC] boolValue];
-    frame.isLr = [json[BSGKeyIsLR] boolValue];
-    frame.machoUuid = json[BSGKeyMachoUUID];
+    frame.machoFile = BSGDeserializeString(json[BSGKeyMachoFile]);
+    frame.method = BSGDeserializeString(json[BSGKeyMethod]);
+    frame.isPc = BSGDeserializeNumber(json[BSGKeyIsPC]).boolValue;
+    frame.isLr = BSGDeserializeNumber(json[BSGKeyIsLR]).boolValue;
+    frame.machoUuid = BSGDeserializeString(json[BSGKeyMachoUUID]);
     frame.machoVmAddress = [self readInt:json key:BSGKeyMachoVMAddress];
     frame.frameAddress = [self readInt:json key:BSGKeyFrameAddress];
     frame.symbolAddress = [self readInt:json key:BSGKeySymbolAddr];
     frame.machoLoadAddress = [self readInt:json key:BSGKeyMachoLoadAddr];
-    frame.type = json[BSGKeyType];
-    frame.columnNumber = json[@"columnNumber"];
-    frame.file = json[@"file"];
-    frame.inProject = json[@"inProject"];
-    frame.lineNumber = json[@"lineNumber"];
+    frame.type = BSGDeserializeString(json[BSGKeyType]);
+    frame.columnNumber = BSGDeserializeNumber(json[@"columnNumber"]);
+    frame.file = BSGDeserializeString(json[@"file"]);
+    frame.inProject = BSGDeserializeNumber(json[@"inProject"]);
+    frame.lineNumber = BSGDeserializeNumber(json[@"lineNumber"]);
     return frame;
 }
 
 + (NSNumber *)readInt:(NSDictionary *)json key:(NSString *)key {
-    NSString *obj = json[key];
-    if (obj) {
+    id obj = json[key];
+    if ([obj isKindOfClass:[NSString class]]) {
         return @(strtoul([obj UTF8String], NULL, 16));
     }
     return nil;

CHANGELOG.md

@@ -1,6 +1,14 @@
 Changelog
 =========
 
+## TBD
+
+### Bug fixes
+
+* Fix `NSNull` handling in `+[BugsnagError errorFromJson:]` and `+[BugsnagStackframe frameFromJson:]`.
+  [#1143](https://github.com/bugsnag/bugsnag-cocoa/pull/1143)
+  [#1138](https://github.com/bugsnag/bugsnag-cocoa/issues/1138)
+
 ## 6.10.0 (2021-06-30)
 
 ### Bug fixes

Tests/BSGInternalErrorReporterTests.m

@@ -28,14 +28,39 @@ - (void)setUp {
     self.notifier = [[BugsnagNotifier alloc] init];
 }
 
-- (void)testEventForError {
+- (void)testEventWithErrorClass {
     BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:@"0192837465afbecd0192837465afbecd"];
     BSGInternalErrorReporter *reporter = [[BSGInternalErrorReporter alloc] initWithDataSource:self];
 
     BugsnagEvent *event = [reporter eventWithErrorClass:@"Internal error" message:@"Something went wrong" diagnostics:@{} groupingHash:@"test"];
     XCTAssertEqualObjects(event.errors[0].errorClass, @"Internal error");
     XCTAssertEqualObjects(event.errors[0].errorMessage, @"Something went wrong");
     XCTAssertEqualObjects(event.groupingHash, @"test");
+    XCTAssertEqualObjects(event.threads, @[]);
+    XCTAssertGreaterThan(event.errors[0].stacktrace.count, 0);
+    XCTAssertNil(event.apiKey);
+
+    NSDictionary *diagnostics = [event.metadata getMetadataFromSection:@"BugsnagDiagnostics"];
+    XCTAssertEqualObjects(diagnostics[@"apiKey"], configuration.apiKey);
+}
+
+- (void)testEventWithException {
+    BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:@"0192837465afbecd0192837465afbecd"];
+    BSGInternalErrorReporter *reporter = [[BSGInternalErrorReporter alloc] initWithDataSource:self];
+
+    NSException *exception = nil;
+    @try {
+        NSLog(@"%@", @[][0]);
+    } @catch (NSException *e) {
+        exception = e;
+    }
+
+    BugsnagEvent *event = [reporter eventWithException:exception diagnostics:nil groupingHash:@"test"];
+    XCTAssertEqualObjects(event.errors[0].errorClass, @"NSRangeException");
+    XCTAssertEqualObjects(event.errors[0].errorMessage, @"*** -[__NSArray0 objectAtIndex:]: index 0 beyond bounds for empty NSArray");
+    XCTAssertEqualObjects(event.groupingHash, @"test");
+    XCTAssertEqualObjects(event.threads, @[]);
+    XCTAssertGreaterThan(event.errors[0].stacktrace.count, 0);
     XCTAssertNil(event.apiKey);
 
     NSDictionary *diagnostics = [event.metadata getMetadataFromSection:@"BugsnagDiagnostics"];

Tests/BugsnagErrorTest.m

@@ -77,6 +77,20 @@ - (void)testErrorLoad {
     XCTAssertEqualObjects(@"D0A41830-4FD2-3B02-A23B-0741AD4C7F52", frame.machoUuid);
 }
 
+- (void)testErrorFromInvalidJson {
+    BugsnagError *error;
+
+    error = [BugsnagError errorFromJson:@{
+        @"stacktrace": [NSNull null],
+    }];
+    XCTAssertEqualObjects(error.stacktrace, @[]);
+
+    error = [BugsnagError errorFromJson:@{
+        @"stacktrace": @{@"foo": @"bar"},
+    }];
+    XCTAssertEqualObjects(error.stacktrace, @[]);
+}
+
 - (void)testToDictionary {
     BugsnagThread *thread = [self findErrorReportingThread:self.event];
     BugsnagError *error = [[BugsnagError alloc] initWithKSCrashReport:self.event stacktrace:thread.stacktrace];

Tests/BugsnagStackframeTest.m

@@ -66,9 +66,12 @@ - (void)testStackframeToDict {
 
 - (void)testStackframeFromJson {
     BugsnagStackframe *frame = [BugsnagStackframe frameFromJson:@{
+        @"columnNumber":        @72,
         @"frameAddress":        @"0x10b5756bf",
+        @"inProject":           @YES,
         @"isLR":                @NO,
-        @"isPC":                @NO,
+        @"isPC":                @YES,
+        @"lineNumber":          @42,
         @"machoFile":           @"/Users/foo/Bugsnag.h",
         @"machoLoadAddress":    @"0x10b54b000",
         @"machoUUID":           @"B6D80CB5-A772-3D2F-B5A1-A3A137B8B58F",
@@ -78,8 +81,11 @@ - (void)testStackframeFromJson {
         @"type":                @"cocoa",
     }];
     XCTAssertEqual(frame.isLr, NO);
-    XCTAssertEqual(frame.isPc, NO);
+    XCTAssertEqual(frame.isPc, YES);
+    XCTAssertEqualObjects(frame.columnNumber,       @72);
     XCTAssertEqualObjects(frame.frameAddress,       @0x10b5756bf);
+    XCTAssertEqualObjects(frame.inProject,          @YES);
+    XCTAssertEqualObjects(frame.lineNumber,         @42);
     XCTAssertEqualObjects(frame.machoFile,          @"/Users/foo/Bugsnag.h");
     XCTAssertEqualObjects(frame.machoLoadAddress,   @0x10b54b000);
     XCTAssertEqualObjects(frame.machoUuid,          @"B6D80CB5-A772-3D2F-B5A1-A3A137B8B58F");
@@ -89,6 +95,37 @@ - (void)testStackframeFromJson {
     XCTAssertEqualObjects(frame.type,               BugsnagStackframeTypeCocoa);
 }
 
+- (void)testStackframeFromJsonWithNullValues {
+    BugsnagStackframe *frame = [BugsnagStackframe frameFromJson:@{
+        @"columnNumber":        [NSNull null],
+        @"frameAddress":        [NSNull null],
+        @"inProject":           [NSNull null],
+        @"isLR":                [NSNull null],
+        @"isPC":                [NSNull null],
+        @"lineNumber":          [NSNull null],
+        @"machoFile":           [NSNull null],
+        @"machoLoadAddress":    [NSNull null],
+        @"machoUUID":           [NSNull null],
+        @"machoVMAddress":      [NSNull null],
+        @"method":              [NSNull null],
+        @"symbolAddress":       [NSNull null],
+        @"type":                [NSNull null],
+    }];
+    XCTAssertEqual(frame.isLr, NO);
+    XCTAssertEqual(frame.isPc, NO);
+    XCTAssertNil(frame.columnNumber);
+    XCTAssertNil(frame.frameAddress);
+    XCTAssertNil(frame.inProject);
+    XCTAssertNil(frame.lineNumber);
+    XCTAssertNil(frame.machoFile);
+    XCTAssertNil(frame.machoLoadAddress);
+    XCTAssertNil(frame.machoUuid);
+    XCTAssertNil(frame.machoVmAddress);
+    XCTAssertNil(frame.method);
+    XCTAssertNil(frame.symbolAddress);
+    XCTAssertNil(frame.type);
+}
+
 - (void)testStackframeFromJsonWithoutType {
     BugsnagStackframe *frame = [BugsnagStackframe frameFromJson:@{}];
     XCTAssertNil(frame.type);