Merge pull request #987 from bugsnag/nickdowell/fix-json-exception-er…

…rors [PLAT-5729 ] Fix malformed NSError from BSGJSONSerialization
bugsnag · Feb 1, 2021 · 2c68530 · 2c68530
2 parents 4291a03 + 0a221d8
commit 2c68530
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 18 deletions.
diff --git a/Bugsnag.xcodeproj/project.pbxproj b/Bugsnag.xcodeproj/project.pbxproj
@@ -789,9 +789,9 @@
 		CBCF77A725010648004AF22A /* BSGJSONSerialization.m in Sources */ = {isa = PBXBuildFile; fileRef = CBCF77A225010648004AF22A /* BSGJSONSerialization.m */; };
 		CBCF77A825010648004AF22A /* BSGJSONSerialization.m in Sources */ = {isa = PBXBuildFile; fileRef = CBCF77A225010648004AF22A /* BSGJSONSerialization.m */; };
 		CBCF77A925010648004AF22A /* BSGJSONSerialization.m in Sources */ = {isa = PBXBuildFile; fileRef = CBCF77A225010648004AF22A /* BSGJSONSerialization.m */; };
-		CBCF77AB250142E0004AF22A /* BSGJSONSerializerTest.m in Sources */ = {isa = PBXBuildFile; fileRef = CBCF77AA250142E0004AF22A /* BSGJSONSerializerTest.m */; };
-		CBCF77AC250142E0004AF22A /* BSGJSONSerializerTest.m in Sources */ = {isa = PBXBuildFile; fileRef = CBCF77AA250142E0004AF22A /* BSGJSONSerializerTest.m */; };
-		CBCF77AD250142E0004AF22A /* BSGJSONSerializerTest.m in Sources */ = {isa = PBXBuildFile; fileRef = CBCF77AA250142E0004AF22A /* BSGJSONSerializerTest.m */; };
+		CBCF77AB250142E0004AF22A /* BSGJSONSerializationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CBCF77AA250142E0004AF22A /* BSGJSONSerializationTests.m */; };
+		CBCF77AC250142E0004AF22A /* BSGJSONSerializationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CBCF77AA250142E0004AF22A /* BSGJSONSerializationTests.m */; };
+		CBCF77AD250142E0004AF22A /* BSGJSONSerializationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CBCF77AA250142E0004AF22A /* BSGJSONSerializationTests.m */; };
 		CBDD6D0725AC3EFF00A2E12B /* BSGStorageMigratorTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CB6419AA25A73E8C00613D25 /* BSGStorageMigratorTests.m */; };
 		CBDD6D0F25AC3EFF00A2E12B /* BSGStorageMigratorTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CB6419AA25A73E8C00613D25 /* BSGStorageMigratorTests.m */; };
 		CBE9062A25A34DAB0045B965 /* BSGStorageMigratorV0V1.h in Headers */ = {isa = PBXBuildFile; fileRef = CBE9062825A34DAB0045B965 /* BSGStorageMigratorV0V1.h */; };
@@ -1333,7 +1333,7 @@
 		CBCAF6F925A457F90095771F /* BSGFileLocations.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BSGFileLocations.m; sourceTree = "<group>"; };
 		CBCF77A125010648004AF22A /* BSGJSONSerialization.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BSGJSONSerialization.h; sourceTree = "<group>"; };
 		CBCF77A225010648004AF22A /* BSGJSONSerialization.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BSGJSONSerialization.m; sourceTree = "<group>"; };
-		CBCF77AA250142E0004AF22A /* BSGJSONSerializerTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BSGJSONSerializerTest.m; sourceTree = "<group>"; };
+		CBCF77AA250142E0004AF22A /* BSGJSONSerializationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BSGJSONSerializationTests.m; sourceTree = "<group>"; };
 		CBE9062825A34DAB0045B965 /* BSGStorageMigratorV0V1.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BSGStorageMigratorV0V1.h; sourceTree = "<group>"; };
 		CBE9062925A34DAB0045B965 /* BSGStorageMigratorV0V1.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BSGStorageMigratorV0V1.m; sourceTree = "<group>"; };
 		E701FA9E2490EF4A008D842F /* BugsnagApiValidationTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BugsnagApiValidationTest.m; sourceTree = "<group>"; };
@@ -1664,7 +1664,7 @@
 			children = (
 				00896A3F2486DBDD00DC48C2 /* BSGConfigurationBuilderTests.m */,
 				008966C62486D43600DC48C2 /* BSGConnectivityTest.m */,
-				CBCF77AA250142E0004AF22A /* BSGJSONSerializerTest.m */,
+				CBCF77AA250142E0004AF22A /* BSGJSONSerializationTests.m */,
 				008966C82486D43600DC48C2 /* BSGOutOfMemoryTests.m */,
 				CB6419AA25A73E8C00613D25 /* BSGStorageMigratorTests.m */,
 				CB9103632502320A00E9D1E2 /* BugsnagApiClientTest.m */,
@@ -2581,7 +2581,7 @@
 				008967722486D43700DC48C2 /* KSSysCtl_Tests.m in Sources */,
 				0089676C2486D43700DC48C2 /* BugsnagTestsDummyClass.m in Sources */,
 				008966EB2486D43700DC48C2 /* BugsnagDeviceTest.m in Sources */,
-				CBCF77AB250142E0004AF22A /* BSGJSONSerializerTest.m in Sources */,
+				CBCF77AB250142E0004AF22A /* BSGJSONSerializationTests.m in Sources */,
 				008967A22486D43700DC48C2 /* KSCrashSentry_Signal_Tests.m in Sources */,
 				008967272486D43700DC48C2 /* BugsnagStackframeTest.m in Sources */,
 				008967302486D43700DC48C2 /* BugsnagStateEventTest.m in Sources */,
@@ -2748,7 +2748,7 @@
 				008967342486D43700DC48C2 /* BugsnagClientTests.m in Sources */,
 				008967222486D43700DC48C2 /* BugsnagErrorReportSinkTests.m in Sources */,
 				CB10E540250BA8DF00AF5824 /* BugsnagKVStoreTest.m in Sources */,
-				CBCF77AC250142E0004AF22A /* BSGJSONSerializerTest.m in Sources */,
+				CBCF77AC250142E0004AF22A /* BSGJSONSerializationTests.m in Sources */,
 				0089679A2486D43700DC48C2 /* FileBasedTestCase.m in Sources */,
 				008967912486D43700DC48C2 /* KSJSONCodec_Tests.m in Sources */,
 				008967732486D43700DC48C2 /* KSSysCtl_Tests.m in Sources */,
@@ -2955,7 +2955,7 @@
 				008967A42486D43700DC48C2 /* KSCrashSentry_Signal_Tests.m in Sources */,
 				E701FAB12490EFE8008D842F /* ConfigurationApiValidationTest.m in Sources */,
 				0089677D2486D43700DC48C2 /* RFC3339DateTool_Tests.m in Sources */,
-				CBCF77AD250142E0004AF22A /* BSGJSONSerializerTest.m in Sources */,
+				CBCF77AD250142E0004AF22A /* BSGJSONSerializationTests.m in Sources */,
 				0163BF5B25823D8D008DC28B /* NotificationBreadcrumbTests.m in Sources */,
 				008967562486D43700DC48C2 /* BugsnagOnCrashTest.m in Sources */,
 				008967A12486D43700DC48C2 /* KSCrashSentry_Tests.m in Sources */,

diff --git a/Bugsnag/Helpers/BSGJSONSerialization.m b/Bugsnag/Helpers/BSGJSONSerialization.m
@@ -12,12 +12,9 @@
 @implementation BSGJSONSerialization
 
 static NSError* wrapException(NSException* exception) {
-    NSDictionary *userInfo = @{
-        NSLocalizedDescriptionKey: exception.description,
-        NSUnderlyingErrorKey: exception,
-    };
-
-    return [NSError errorWithDomain:@"BSGJSONSerializationErrorDomain" code:1 userInfo:userInfo];
+    return [NSError errorWithDomain:@"BSGJSONSerializationErrorDomain" code:1 userInfo:@{
+        NSLocalizedDescriptionKey: [NSString stringWithFormat:@"%@: %@", exception.name, exception.reason]
+    }];
 }
 
 + (BOOL)isValidJSONObject:(id)obj {

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,8 @@ Changelog
 
 * Fix compiler warnings when importing Bugsnag from Objective-C sources that do not use ARC.
   [#985](https://github.com/bugsnag/bugsnag-cocoa/pull/985)
+* Fix a rare crash that could occur in the event of JSON parsing failures.
+  [#987](https://github.com/bugsnag/bugsnag-cocoa/pull/987)
 
 ## 6.6.0 (2021-01-20)
 

diff --git a/Tests/BSGJSONSerializerTest.m → Tests/BSGJSONSerializationTests.m b/Tests/BSGJSONSerializerTest.m → Tests/BSGJSONSerializationTests.m
@@ -1,19 +1,19 @@
 //
-//  BSGJSONSerializerTest.m
+//  BSGJSONSerializationTests.m
 //  Bugsnag
 //
 //  Created by Karl Stenerud on 03.09.20.
 //  Copyright © 2020 Bugsnag Inc. All rights reserved.
 //
 
 #import <XCTest/XCTest.h>
-#import "BSGJSONSerialization.h"
 
-@interface BSGJSONSerializerTest : XCTestCase
+#import "BSGJSONSerialization.h"
 
+@interface BSGJSONSerializationTests : XCTestCase
 @end
 
-@implementation BSGJSONSerializerTest
+@implementation BSGJSONSerializationTests
 
 - (void)testBadJSONKey {
     id badDict = @{@123: @"string"};
@@ -76,4 +76,15 @@ - (void)testJSONFileSerialization {
     XCTAssertNotNil(error);
 }
 
+- (void)testExceptionHandling {
+    NSError *error = nil;
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wnonnull"
+    XCTAssertNil([BSGJSONSerialization JSONObjectWithData:nil options:0 error:&error]);
+#pragma clang diagnostic pop
+    XCTAssertNotNil(error);
+    id underlyingError = error.userInfo[NSUnderlyingErrorKey];
+    XCTAssert(!underlyingError || [underlyingError isKindOfClass:[NSError class]], @"The value of %@ should be an NSError", NSUnderlyingErrorKey);
+}
+
 @end