From 0a221d85095cf3ce4585e816faa5396bec57e4a6 Mon Sep 17 00:00:00 2001 From: Nick Dowell Date: Fri, 29 Jan 2021 14:18:07 +0000 Subject: [PATCH] Fix malformed NSError from BSGJSONSerialization --- Bugsnag.xcodeproj/project.pbxproj | 16 ++++++++-------- Bugsnag/Helpers/BSGJSONSerialization.m | 9 +++------ CHANGELOG.md | 2 ++ ...izerTest.m => BSGJSONSerializationTests.m} | 19 +++++++++++++++---- 4 files changed, 28 insertions(+), 18 deletions(-) rename Tests/{BSGJSONSerializerTest.m => BSGJSONSerializationTests.m} (79%) diff --git a/Bugsnag.xcodeproj/project.pbxproj b/Bugsnag.xcodeproj/project.pbxproj index ca7935eb8..9a2dd240e 100644 --- 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 = ""; }; CBCF77A125010648004AF22A /* BSGJSONSerialization.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BSGJSONSerialization.h; sourceTree = ""; }; CBCF77A225010648004AF22A /* BSGJSONSerialization.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BSGJSONSerialization.m; sourceTree = ""; }; - CBCF77AA250142E0004AF22A /* BSGJSONSerializerTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BSGJSONSerializerTest.m; sourceTree = ""; }; + CBCF77AA250142E0004AF22A /* BSGJSONSerializationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BSGJSONSerializationTests.m; sourceTree = ""; }; CBE9062825A34DAB0045B965 /* BSGStorageMigratorV0V1.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BSGStorageMigratorV0V1.h; sourceTree = ""; }; CBE9062925A34DAB0045B965 /* BSGStorageMigratorV0V1.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BSGStorageMigratorV0V1.m; sourceTree = ""; }; E701FA9E2490EF4A008D842F /* BugsnagApiValidationTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BugsnagApiValidationTest.m; sourceTree = ""; }; @@ -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 index c536210e6..c344a0764 100644 --- 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 index b86c2fc6b..40d7a5bce 100644 --- 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 b/Tests/BSGJSONSerializationTests.m similarity index 79% rename from Tests/BSGJSONSerializerTest.m rename to Tests/BSGJSONSerializationTests.m index 1ea777564..5d9ca0881 100644 --- a/Tests/BSGJSONSerializerTest.m +++ b/Tests/BSGJSONSerializationTests.m @@ -1,5 +1,5 @@ // -// BSGJSONSerializerTest.m +// BSGJSONSerializationTests.m // Bugsnag // // Created by Karl Stenerud on 03.09.20. @@ -7,13 +7,13 @@ // #import -#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