Skip to content

Commit 08fc5f9

Browse files
authored
ref: Update SentryCrashReportConverter for improved nullability handling (#6208)
- Renamed SentryKSCrashReportConverterTests to SentryCrashReportConverterTests for consistency. - Enhanced nullability checks in SentryCrashReportConverter to prevent potential crashes. - Added new test cases to validate handling of various edge cases in crash report conversion.
1 parent 15c8f13 commit 08fc5f9

File tree

8 files changed

+327
-39
lines changed

8 files changed

+327
-39
lines changed

Sentry.xcodeproj/project.pbxproj

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@
155155
6304360B1EC0595B00C4D3FA /* SentryNSDataUtils.m in Sources */ = {isa = PBXBuildFile; fileRef = 630436091EC0595B00C4D3FA /* SentryNSDataUtils.m */; };
156156
630436101EC0600A00C4D3FA /* SentrySerializable.h in Headers */ = {isa = PBXBuildFile; fileRef = 6304360F1EC0600A00C4D3FA /* SentrySerializable.h */; settings = {ATTRIBUTES = (Public, ); }; };
157157
630436161EC0AD3100C4D3FA /* SentryNSDataCompressionTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 630436151EC0AD3100C4D3FA /* SentryNSDataCompressionTests.m */; };
158-
630C01941EC3402C00C52CEF /* SentryKSCrashReportConverterTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 630C01931EC3402C00C52CEF /* SentryKSCrashReportConverterTests.m */; };
158+
630C01941EC3402C00C52CEF /* SentryCrashReportConverterTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 630C01931EC3402C00C52CEF /* SentryCrashReportConverterTests.m */; };
159159
630C01961EC341D600C52CEF /* Resources in Resources */ = {isa = PBXBuildFile; fileRef = 630C01951EC341D600C52CEF /* Resources */; };
160160
631501BB1EE6F30B00512C5B /* SentrySwizzleTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 631501BA1EE6F30B00512C5B /* SentrySwizzleTests.m */; };
161161
631E6D331EBC679C00712345 /* SentryQueueableRequestManager.h in Headers */ = {isa = PBXBuildFile; fileRef = 631E6D311EBC679C00712345 /* SentryQueueableRequestManager.h */; };
@@ -1391,7 +1391,7 @@
13911391
6304360D1EC05CEF00C4D3FA /* libz.tbd */ = {isa = PBXFileReference; lastKnownFileType = "sourcecode.text-based-dylib-definition"; name = libz.tbd; path = Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/lib/libz.tbd; sourceTree = DEVELOPER_DIR; };
13921392
6304360F1EC0600A00C4D3FA /* SentrySerializable.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SentrySerializable.h; path = Public/SentrySerializable.h; sourceTree = "<group>"; };
13931393
630436151EC0AD3100C4D3FA /* SentryNSDataCompressionTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = SentryNSDataCompressionTests.m; sourceTree = "<group>"; };
1394-
630C01931EC3402C00C52CEF /* SentryKSCrashReportConverterTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = SentryKSCrashReportConverterTests.m; sourceTree = "<group>"; };
1394+
630C01931EC3402C00C52CEF /* SentryCrashReportConverterTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = SentryCrashReportConverterTests.m; sourceTree = "<group>"; };
13951395
630C01951EC341D600C52CEF /* Resources */ = {isa = PBXFileReference; lastKnownFileType = folder; path = Resources; sourceTree = "<group>"; };
13961396
631501BA1EE6F30B00512C5B /* SentrySwizzleTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = SentrySwizzleTests.m; sourceTree = "<group>"; };
13971397
631E6D311EBC679C00712345 /* SentryQueueableRequestManager.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SentryQueueableRequestManager.h; path = include/SentryQueueableRequestManager.h; sourceTree = "<group>"; };
@@ -2974,7 +2974,7 @@
29742974
8431D4572BE175A1009EAEC1 /* SentryContinuousProfiler+Test.h */,
29752975
639889D21EDF06C100EA7442 /* SentryTests-Bridging-Header.h */,
29762976
63B819131EC352A7002FDF4C /* SentryInterfacesTests.m */,
2977-
630C01931EC3402C00C52CEF /* SentryKSCrashReportConverterTests.m */,
2977+
630C01931EC3402C00C52CEF /* SentryCrashReportConverterTests.m */,
29782978
630436151EC0AD3100C4D3FA /* SentryNSDataCompressionTests.m */,
29792979
63EED6C22237989300E02400 /* SentryOptionsTest.m */,
29802980
7B569DFF2590EEF600B653FC /* SentryScope+Equality.m */,
@@ -6150,7 +6150,7 @@
61506150
7BBD18A2244EE2FD00427C76 /* TestResponseFactory.swift in Sources */,
61516151
628B89022D841D7F004B6F2A /* SentryDateUtilsTests.swift in Sources */,
61526152
D808FB8B281BCE96009A2A33 /* TestSentrySwizzleWrapper.swift in Sources */,
6153-
630C01941EC3402C00C52CEF /* SentryKSCrashReportConverterTests.m in Sources */,
6153+
630C01941EC3402C00C52CEF /* SentryCrashReportConverterTests.m in Sources */,
61546154
7B59398424AB481B0003AAD2 /* NotificationCenterTestCase.swift in Sources */,
61556155
7B0A542E2521C62400A71716 /* SentryFrameRemoverTests.swift in Sources */,
61566156
7BE912B12721C76000E49E62 /* SentryPerformanceTrackingIntegrationTests.swift in Sources */,

Sources/Sentry/Public/SentryThread.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,19 @@ NS_ASSUME_NONNULL_BEGIN
2020

2121
SENTRY_NO_INIT
2222

23+
#if SDK_V9
24+
/**
25+
* Number of the thread.
26+
*
27+
* Can be nil for threads in recrash reports where the thread index information is not available.
28+
*/
29+
@property (nullable, nonatomic, copy) NSNumber *threadId;
30+
#else
2331
/**
2432
* Number of the thread
2533
*/
2634
@property (nonatomic, copy) NSNumber *threadId;
35+
#endif // SDK_V9
2736

2837
/**
2938
* Name (if available) of the thread
@@ -50,12 +59,21 @@ SENTRY_NO_INIT
5059
*/
5160
@property (nullable, nonatomic, copy) NSNumber *isMain;
5261

62+
#if SDK_V9
63+
/**
64+
* Initializes a SentryThread with its id
65+
* @param threadId NSNumber or nil if thread index is not available (e.g., recrash reports)
66+
* @return SentryThread
67+
*/
68+
- (instancetype)initWithThreadId:(nullable NSNumber *)threadId;
69+
#else
5370
/**
5471
* Initializes a SentryThread with its id
5572
* @param threadId NSNumber
5673
* @return SentryThread
5774
*/
5875
- (instancetype)initWithThreadId:(NSNumber *)threadId;
76+
#endif // SDK_V9
5977

6078
@end
6179

Sources/Sentry/SentryANRTrackingIntegration.m

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#import "SentryEvent.h"
66
#import "SentryException.h"
77
#import "SentryHub+Private.h"
8+
#import "SentryInternalDefines.h"
89
#import "SentryLogC.h"
910
#import "SentryMechanism.h"
1011
#import "SentrySDK+Private.h"
@@ -154,7 +155,8 @@ - (void)anrDetectedWithType:(enum SentryANRType)type
154155
// recover the debug images. The client would also attach the debug images when directly
155156
// capturing the app hang event. Still, we attach them already now to ensure all app hang events
156157
// have debug images cause it's easy to mess this up in the future.
157-
event.debugMeta = [self.debugImageProvider getDebugImagesFromCacheForThreads:event.threads];
158+
event.debugMeta = [self.debugImageProvider
159+
getDebugImagesFromCacheForThreads:SENTRY_UNWRAP_NULLABLE(NSArray, event.threads)];
158160

159161
#if SENTRY_HAS_UIKIT
160162
# if SDK_V9

Sources/Sentry/SentryClient.m

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -765,8 +765,9 @@ - (SentryEvent *_Nullable)prepareEvent:(SentryEvent *)event
765765
BOOL debugMetaNotAttached = !(nil != event.debugMeta && event.debugMeta.count > 0);
766766
if (!isFatalEvent && shouldAttachStacktrace && debugMetaNotAttached
767767
&& event.threads != nil) {
768-
event.debugMeta =
769-
[self.debugImageProvider getDebugImagesFromCacheForThreads:event.threads];
768+
event.debugMeta = [self.debugImageProvider
769+
getDebugImagesFromCacheForThreads:SENTRY_UNWRAP_NULLABLE(
770+
NSArray<SentryThread *>, event.threads)];
770771
}
771772
}
772773

Sources/Sentry/SentryCrashReportConverter.m

Lines changed: 59 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ - (instancetype)initWithReport:(NSDictionary *)report inAppLogic:(SentryInAppLog
5151
// here. For more details please check out SentryCrashScopeObserver.
5252
NSMutableDictionary *userContextMerged =
5353
[[NSMutableDictionary alloc] initWithDictionary:userContextUnMerged];
54-
[userContextMerged addEntriesFromDictionary:report[@"sentry_sdk_scope"]];
54+
[userContextMerged addEntriesFromDictionary:report[@"sentry_sdk_scope"] ?: @{}];
5555
[userContextMerged removeObjectForKey:@"sentry_sdk_scope"];
5656
self.userContext = userContextMerged;
5757

@@ -108,8 +108,9 @@ - (SentryEvent *_Nullable)convertReportToEvent
108108
if ([self.report[@"report"][@"timestamp"] isKindOfClass:NSNumber.class]) {
109109
event.timestamp = [NSDate
110110
dateWithTimeIntervalSince1970:[self.report[@"report"][@"timestamp"] integerValue]];
111-
} else {
112-
event.timestamp = sentry_fromIso8601String(self.report[@"report"][@"timestamp"]);
111+
} else if ([self.report[@"report"][@"timestamp"] isKindOfClass:NSString.class]) {
112+
event.timestamp = sentry_fromIso8601String(
113+
SENTRY_UNWRAP_NULLABLE(NSString, self.report[@"report"][@"timestamp"]));
113114
}
114115
event.threads = [self convertThreads];
115116
event.debugMeta = [self debugMetaForThreads:event.threads];
@@ -119,7 +120,7 @@ - (SentryEvent *_Nullable)convertReportToEvent
119120
event.environment = self.userContext[@"environment"];
120121

121122
NSMutableDictionary *mutableContext =
122-
[[NSMutableDictionary alloc] initWithDictionary:self.userContext[@"context"]];
123+
[[NSMutableDictionary alloc] initWithDictionary:self.userContext[@"context"] ?: @{}];
123124
if (self.userContext[@"traceContext"]) {
124125
mutableContext[@"trace"] = self.userContext[@"traceContext"];
125126
}
@@ -188,11 +189,16 @@ - (SentryUser *_Nullable)convertUser
188189
for (NSDictionary *storedCrumb in storedBreadcrumbs) {
189190
SentryBreadcrumb *crumb = [[SentryBreadcrumb alloc]
190191
initWithLevel:[self sentryLevelFromString:storedCrumb[@"level"]]
191-
category:storedCrumb[@"category"]];
192+
category:storedCrumb[@"category"]
193+
?: @"default"]; // The default value is the same as the one in
194+
// SentryBreadcrumb.init
192195
crumb.message = storedCrumb[@"message"];
193196
crumb.type = storedCrumb[@"type"];
194197
crumb.origin = storedCrumb[@"origin"];
195-
crumb.timestamp = sentry_fromIso8601String(storedCrumb[@"timestamp"]);
198+
if ([storedCrumb[@"timestamp"] isKindOfClass:NSString.class]) {
199+
crumb.timestamp = sentry_fromIso8601String(
200+
SENTRY_UNWRAP_NULLABLE(NSString, storedCrumb[@"timestamp"]));
201+
}
196202
crumb.data = storedCrumb[@"data"];
197203
[breadcrumbs addObject:crumb];
198204
}
@@ -248,14 +254,32 @@ - (NSDictionary *)binaryImageForAddress:(uintptr_t)address
248254
return result;
249255
}
250256

257+
/**
258+
* Creates a SentryThread from crash report thread data at the specified index.
259+
*
260+
* This method includes defensive null handling to prevent crashes when processing
261+
* malformed crash reports. The original bug was that invalid thread index types
262+
* could cause crashes when accessing threadId.intValue for isMain calculation.
263+
*/
251264
- (SentryThread *_Nullable)threadAtIndex:(NSInteger)threadIndex
252265
{
253266
if (threadIndex >= [self.threads count]) {
254267
return nil;
255268
}
256269
NSDictionary *threadDictionary = self.threads[threadIndex];
257270

258-
SentryThread *thread = [[SentryThread alloc] initWithThreadId:threadDictionary[@"index"]];
271+
// Thread index validation: We must support nil/missing indexes for backward compatibility
272+
// with recrash reports (see testRecrashReport_WithThreadIsStringInsteadOfDict), but we
273+
// must reject invalid types when present to prevent crashes from calling .intValue on
274+
// non-NSNumber objects. This fixes a bug where malformed crash reports could cause
275+
// crashes in the converter itself when accessing threadId.intValue for isMain calculation.
276+
id threadIndexObj = threadDictionary[@"index"];
277+
if (threadIndexObj != nil && ![threadIndexObj isKindOfClass:[NSNumber class]]) {
278+
SENTRY_LOG_ERROR(@"Thread index is not a number: %@", threadIndexObj);
279+
return nil;
280+
}
281+
SentryThread *thread =
282+
[[SentryThread alloc] initWithThreadId:SENTRY_UNWRAP_NULLABLE(NSNumber, threadIndexObj)];
259283
// We only want to add the stacktrace if this thread hasn't crashed
260284
thread.stacktrace = [self stackTraceForThreadIndex:threadIndex];
261285
if (thread.stacktrace.frames.count == 0) {
@@ -266,7 +290,10 @@ - (SentryThread *_Nullable)threadAtIndex:(NSInteger)threadIndex
266290
thread.current = threadDictionary[@"current_thread"];
267291
thread.name = threadDictionary[@"name"];
268292
// We don't have access to the MachineContextWrapper but we know first thread is always the main
269-
thread.isMain = [NSNumber numberWithBool:thread.threadId.intValue == 0];
293+
// Use null-safe check: threadIndexObj can be nil for recrash reports, and calling intValue on
294+
// a nil NSNumber would return 0, incorrectly marking threads without indexes as main threads.
295+
thread.isMain =
296+
[NSNumber numberWithBool:threadIndexObj != nil && [threadIndexObj intValue] == 0];
270297
if (nil == thread.name) {
271298
thread.name = threadDictionary[@"dispatch_queue"];
272299
}
@@ -357,9 +384,11 @@ - (SentryDebugMeta *)debugMetaFromBinaryImageDictionary:(NSDictionary *)sourceIm
357384

358385
for (SentryThread *thread in threads) {
359386
for (SentryFrame *frame in thread.stacktrace.frames) {
360-
if (frame.imageAddress && ![imageNames containsObject:frame.imageAddress]) {
361-
[imageNames addObject:frame.imageAddress];
387+
NSString *_Nullable nullableImageAddress = frame.imageAddress;
388+
if (nullableImageAddress == nil) {
389+
continue;
362390
}
391+
[imageNames addObject:SENTRY_UNWRAP_NULLABLE(NSString, nullableImageAddress)];
363392
}
364393
}
365394

@@ -399,19 +428,25 @@ - (SentryDebugMeta *)debugMetaFromBinaryImageDictionary:(NSDictionary *)sourceIm
399428
self.exceptionContext[@"mach"][@"exception"],
400429
self.exceptionContext[@"mach"][@"code"],
401430
self.exceptionContext[@"mach"][@"subcode"]]
402-
type:self.exceptionContext[@"mach"][@"exception_name"]];
431+
type:self.exceptionContext[@"mach"][@"exception_name"]
432+
?: @"Mach Exception"]; // The fallback value is best-attempt in case the exception
433+
// name is not available
403434
} else if ([exceptionType isEqualToString:@"signal"]) {
404435
exception =
405436
[[SentryException alloc] initWithValue:[NSString stringWithFormat:@"Signal %@, Code %@",
406437
self.exceptionContext[@"signal"][@"signal"],
407438
self.exceptionContext[@"signal"][@"code"]]
408-
type:self.exceptionContext[@"signal"][@"name"]];
439+
type:self.exceptionContext[@"signal"][@"name"]
440+
?: @"Signal Exception"]; // The fallback value is best-attempt in case the
441+
// exception name is not available
409442
} else if ([exceptionType isEqualToString:@"user"]) {
410443
NSString *exceptionReason =
411444
[NSString stringWithFormat:@"%@", self.exceptionContext[@"reason"]];
412-
exception = [[SentryException alloc]
413-
initWithValue:exceptionReason
414-
type:self.exceptionContext[@"user_reported"][@"name"]];
445+
exception =
446+
[[SentryException alloc] initWithValue:exceptionReason
447+
type:self.exceptionContext[@"user_reported"][@"name"]
448+
?: @"User Reported Exception"]; // The fallback value is best-attempt in case
449+
// the exception name is not available
415450

416451
NSRange match = [exceptionReason rangeOfString:@":"];
417452
if (match.location != NSNotFound) {
@@ -454,7 +489,9 @@ - (SentryException *)parseNSException
454489
}
455490

456491
return [[SentryException alloc] initWithValue:[NSString stringWithFormat:@"%@", reason]
457-
type:self.exceptionContext[@"nsexception"][@"name"]];
492+
type:self.exceptionContext[@"nsexception"][@"name"]
493+
?: @"NSException"]; // The fallback value is best-attempt in case the exception name is
494+
// not available
458495
}
459496

460497
- (void)enhanceValueFromNotableAddresses:(SentryException *)exception
@@ -464,15 +501,15 @@ - (void)enhanceValueFromNotableAddresses:(SentryException *)exception
464501
return;
465502
}
466503
NSDictionary *crashedThread = self.threads[self.crashedThreadIndex];
467-
NSDictionary *notableAddresses = crashedThread[@"notable_addresses"];
504+
NSDictionary *_Nullable notableAddresses = crashedThread[@"notable_addresses"];
468505
NSMutableOrderedSet *reasons = [[NSMutableOrderedSet alloc] init];
469506
if (nil != notableAddresses) {
470507
for (id key in notableAddresses) {
471508
NSDictionary *content = notableAddresses[key];
472509
if ([content[@"type"] isEqualToString:@"string"] && nil != content[@"value"]) {
473510
// if there are less than 3 slashes it shouldn't be a filepath
474511
if ([[content[@"value"] componentsSeparatedByString:@"/"] count] < 3) {
475-
[reasons addObject:content[@"value"]];
512+
[reasons addObject:SENTRY_UNWRAP_NULLABLE(NSString, content[@"value"])];
476513
}
477514
}
478515
}
@@ -497,11 +534,13 @@ - (void)enhanceValueFromCrashInfoMessage:(SentryException *)exception
497534

498535
for (NSDictionary *binaryImage in libSwiftCoreBinaryImages) {
499536
if (binaryImage[@"crash_info_message"] != nil) {
500-
[crashInfoMessages addObject:binaryImage[@"crash_info_message"]];
537+
[crashInfoMessages
538+
addObject:SENTRY_UNWRAP_NULLABLE(NSString, binaryImage[@"crash_info_message"])];
501539
}
502540

503541
if (binaryImage[@"crash_info_message2"] != nil) {
504-
[crashInfoMessages addObject:binaryImage[@"crash_info_message2"]];
542+
[crashInfoMessages
543+
addObject:SENTRY_UNWRAP_NULLABLE(NSString, binaryImage[@"crash_info_message2"])];
505544
}
506545
}
507546

Sources/Sentry/SentryThread.mm

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@
1212

1313
@implementation SentryThread
1414

15+
#if SDK_V9
16+
- (instancetype)initWithThreadId:(nullable NSNumber *)threadId
17+
#else
1518
- (instancetype)initWithThreadId:(NSNumber *)threadId
19+
#endif // SDK_V9
1620
{
1721
self = [super init];
1822
if (self) {

0 commit comments

Comments
 (0)