From 8ce3854ab0a3870ddcebcfb667ce02df3629d7b2 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Fri, 27 Jan 2023 11:33:32 +0100 Subject: [PATCH] Fix crash introduced with uint64 support We forgot to add onUIntegerElement to SentryCrashReportFixer. So the SDK crashed while reading the crash report on the app start. This is fixed now by adding onUIntegerElement. Furthermore, I validated that all SentryCrashJSONDecodeCallbacks now have a mapping to onUIntegerElement. --- CHANGELOG.md | 2 +- .../Monitors/SentryCrashMonitor_AppState.c | 8 +++ .../Recording/SentryCrashReportFixer.c | 8 +++ .../CrashState_unsupported_fields.json | 8 +++ Tests/Resources/processed.json | 4 +- Tests/Resources/raw.json | 4 +- .../SentryCrashMonitor_AppState_Tests.m | 55 +++++++++++++------ 7 files changed, 66 insertions(+), 23 deletions(-) create mode 100644 Tests/Resources/CrashState_unsupported_fields.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 7185042e4f1..c406ffcb217 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ ### Fixes -- Support uint64 in crash reports (#2631, #2642) +- Support uint64 in crash reports (#2631, #2642, #2663) - Always fetch view hierarchy on the main thread (#2629) - Carthage Xcode 14 compatibility issue (#2636) - Crash in CppException Monitor (#2639) diff --git a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_AppState.c b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_AppState.c index 795fd320235..3d6335baf90 100644 --- a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_AppState.c +++ b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_AppState.c @@ -137,6 +137,13 @@ onIntegerElement(const char *const name, const int64_t value, void *const userDa return onFloatingPointElement(name, value, userData); } +static int +onUIntegerElement( + __unused const char *const name, __unused const uint64_t value, __unused void *const userData) +{ + return SentryCrashJSON_OK; +} + static int onNullElement(__unused const char *const name, __unused void *const userData) { @@ -234,6 +241,7 @@ loadState(const char *const path) callbacks.onEndData = onEndData; callbacks.onFloatingPointElement = onFloatingPointElement; callbacks.onIntegerElement = onIntegerElement; + callbacks.onUIntegerElement = onUIntegerElement; callbacks.onNullElement = onNullElement; callbacks.onStringElement = onStringElement; diff --git a/Sources/SentryCrash/Recording/SentryCrashReportFixer.c b/Sources/SentryCrash/Recording/SentryCrashReportFixer.c index f7151d7d0e8..0a1fe65494a 100644 --- a/Sources/SentryCrash/Recording/SentryCrashReportFixer.c +++ b/Sources/SentryCrash/Recording/SentryCrashReportFixer.c @@ -142,6 +142,13 @@ onIntegerElement(const char *const name, const int64_t value, void *const userDa return result; } +static int +onUIntegerElement(const char *const name, const uint64_t value, void *const userData) +{ + FixupContext *context = (FixupContext *)userData; + return sentrycrashjson_addUIntegerElement(context->encodeContext, name, value); +} + static int onNullElement(const char *const name, void *const userData) { @@ -230,6 +237,7 @@ sentrycrashcrf_fixupCrashReport(const char *crashReport) .onEndData = onEndData, .onFloatingPointElement = onFloatingPointElement, .onIntegerElement = onIntegerElement, + .onUIntegerElement = onUIntegerElement, .onNullElement = onNullElement, .onStringElement = onStringElement, }; diff --git a/Tests/Resources/CrashState_unsupported_fields.json b/Tests/Resources/CrashState_unsupported_fields.json new file mode 100644 index 00000000000..7518c4527c9 --- /dev/null +++ b/Tests/Resources/CrashState_unsupported_fields.json @@ -0,0 +1,8 @@ +{ + "version": 9223372036854775808, + "crashedLastLaunch": "false", + "activeDurationSinceLastCrash": null, + "backgroundDurationSinceLastCrash": { + "not": "supported" + }, +} diff --git a/Tests/Resources/processed.json b/Tests/Resources/processed.json index 9bfb7c1c137..e05c3cb9ff3 100644 --- a/Tests/Resources/processed.json +++ b/Tests/Resources/processed.json @@ -2544,8 +2544,8 @@ "cs": 11, "ds": 35, "es": 35, - "fs": 35, - "gs": 15 + "fs": 9223372036854775807, + "gs": 9223372036854775808 } }, "index": 5, diff --git a/Tests/Resources/raw.json b/Tests/Resources/raw.json index 6f8581935ed..0552e03b4df 100644 --- a/Tests/Resources/raw.json +++ b/Tests/Resources/raw.json @@ -2544,8 +2544,8 @@ "cs": 11, "ds": 35, "es": 35, - "fs": 35, - "gs": 15 + "fs": 9223372036854775807, + "gs": 9223372036854775808 } }, "index": 5, diff --git a/Tests/SentryTests/SentryCrash/SentryCrashMonitor_AppState_Tests.m b/Tests/SentryTests/SentryCrash/SentryCrashMonitor_AppState_Tests.m index 0948e1fb050..584634bec2b 100644 --- a/Tests/SentryTests/SentryCrash/SentryCrashMonitor_AppState_Tests.m +++ b/Tests/SentryTests/SentryCrash/SentryCrashMonitor_AppState_Tests.m @@ -155,26 +155,10 @@ - (void)testInitWithWrongCrashState [jsonData writeToFile:stateFile atomically:true]; [self initializeCrashState]; - SentryCrash_AppState context = *sentrycrashstate_currentState(); - - XCTAssertTrue(context.applicationIsInForeground); - XCTAssertFalse(context.applicationIsActive); - - XCTAssertEqual(context.activeDurationSinceLastCrash, 0.0); - XCTAssertEqual(context.backgroundDurationSinceLastCrash, 0.0); - XCTAssertEqual(context.launchesSinceLastCrash, 1); - XCTAssertEqual(context.sessionsSinceLastCrash, 1); - - XCTAssertEqual(context.activeDurationSinceLaunch, 0.0); - XCTAssertEqual(context.backgroundDurationSinceLaunch, 0.0); - XCTAssertEqual(context.sessionsSinceLaunch, 1); - - XCTAssertFalse(context.crashedThisLaunch); - XCTAssertFalse(context.crashedLastLaunch); - XCTAssertEqual(context.durationFromCrashStateInitToLastCrash, 0.0); + [self assertDefaultCrashState]; [self initializeCrashState]; - context = *sentrycrashstate_currentState(); + SentryCrash_AppState context = *sentrycrashstate_currentState(); XCTAssertEqual(context.launchesSinceLastCrash, 2); XCTAssertEqual(context.sessionsSinceLastCrash, 2); @@ -186,6 +170,20 @@ - (void)testInitWithWrongCrashState XCTAssertEqual(context.sessionsSinceLastCrash, 1); } +- (void)testInitWithUnsupportedFields +{ + NSString *stateFile = [self.tempPath stringByAppendingPathComponent:@"state.json"]; + NSString *jsonPath = [[NSBundle bundleForClass:self.class] + pathForResource:@"Resources/CrashState_unsupported_fields" + ofType:@"json"]; + NSData *jsonData = [NSData dataWithContentsOfURL:[NSURL fileURLWithPath:jsonPath]]; + [jsonData writeToFile:stateFile atomically:true]; + + [self initializeCrashState]; + + [self assertDefaultCrashState]; +} + - (void)testInitWithCrashStateLegacy { NSString *stateFile = [self.tempPath stringByAppendingPathComponent:@"state.json"]; @@ -714,4 +712,25 @@ - (void)testActDeactBGFGCrash XCTAssertLessThan(context.durationFromCrashStateInitToLastCrash, 1.0); } +- (void)assertDefaultCrashState +{ + SentryCrash_AppState context = *sentrycrashstate_currentState(); + + XCTAssertTrue(context.applicationIsInForeground); + XCTAssertFalse(context.applicationIsActive); + + XCTAssertEqual(context.activeDurationSinceLastCrash, 0.0); + XCTAssertEqual(context.backgroundDurationSinceLastCrash, 0.0); + XCTAssertEqual(context.launchesSinceLastCrash, 1); + XCTAssertEqual(context.sessionsSinceLastCrash, 1); + + XCTAssertEqual(context.activeDurationSinceLaunch, 0.0); + XCTAssertEqual(context.backgroundDurationSinceLaunch, 0.0); + XCTAssertEqual(context.sessionsSinceLaunch, 1); + + XCTAssertFalse(context.crashedThisLaunch); + XCTAssertFalse(context.crashedLastLaunch); + XCTAssertEqual(context.durationFromCrashStateInitToLastCrash, 0.0); +} + @end