From a95ba2f0ce1d73afe962b963608d2b414d87b38b Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Mon, 19 Aug 2024 16:28:56 +0200 Subject: [PATCH] fix: Crash during SDK initialization due to corrupted envelope (#4291) Reverting #4219 to further investigation --- CHANGELOG.md | 2 ++ Sources/Sentry/SentryFileManager.m | 31 ++++------------- Sources/Sentry/SentrySerialization.m | 34 ++++++------------- Sources/Sentry/include/SentryFileManager.h | 2 +- Sources/Sentry/include/SentrySerialization.h | 8 ----- .../Helper/SentryFileManagerTests.swift | 9 ++--- .../Networking/SentryHttpTransportTests.swift | 3 +- 7 files changed, 23 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90801a58898..d8319f32351 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixes - Fix `SIGABRT` when modifying scope user (#4274) +- Crash during SDK initialization due to corrupted envelope (#4291) + - Reverts [#4219](https://github.com/getsentry/sentry-cocoa/pull/4219) as potential fix ## 8.34.0 diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index 8ff5ad8f9aa..173f23df51e 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -310,40 +310,21 @@ - (void)removeFileAtPath:(NSString *)path } } -- (void)storeEnvelope:(SentryEnvelope *)envelope +- (NSString *)storeEnvelope:(SentryEnvelope *)envelope { + NSData *envelopeData = [SentrySerialization dataWithEnvelope:envelope]; + @synchronized(self) { NSString *path = [self.envelopesPath stringByAppendingPathComponent:[self uniqueAscendingJsonName]]; - SENTRY_LOG_DEBUG(@"Writing envelope to path: %@", path); - [[NSFileManager defaultManager] createFileAtPath:path contents:nil attributes:nil]; - - NSFileHandle *fileHandle = [NSFileHandle fileHandleForWritingAtPath:path]; - - if (!fileHandle) { - SENTRY_LOG_ERROR(@"Couldn't get NSFileHandle for path: %@", path); - return; - } - - @try { - BOOL success = [SentrySerialization - writeEnvelopeData:envelope - writeData:^(NSData *data) { [fileHandle writeData:data]; }]; - - if (!success) { - SENTRY_LOG_WARN(@"Failed to store envelope."); - [self removeFileAtPath:path]; - } - } @catch (NSException *exception) { - SENTRY_LOG_ERROR(@"Error while writing data: %@ ", exception.description); - [self removeFileAtPath:path]; - } @finally { - [fileHandle closeFile]; + if (![self writeData:envelopeData toPath:path]) { + SENTRY_LOG_WARN(@"Failed to store envelope."); } [self handleEnvelopesLimit]; + return path; } } diff --git a/Sources/Sentry/SentrySerialization.m b/Sources/Sentry/SentrySerialization.m index 8f9fe67a2cb..521992499e1 100644 --- a/Sources/Sentry/SentrySerialization.m +++ b/Sources/Sentry/SentrySerialization.m @@ -32,8 +32,9 @@ + (NSData *_Nullable)dataWithJSONObject:(id)jsonObject return data; } -+ (BOOL)writeEnvelopeData:(SentryEnvelope *)envelope writeData:(SentryDataWriter)writeData ++ (NSData *_Nullable)dataWithEnvelope:(SentryEnvelope *)envelope { + NSMutableData *envelopeData = [[NSMutableData alloc] init]; NSMutableDictionary *serializedData = [NSMutableDictionary new]; if (nil != envelope.header.eventId) { [serializedData setValue:[envelope.header.eventId sentryIdString] forKey:@"event_id"]; @@ -56,40 +57,25 @@ + (BOOL)writeEnvelopeData:(SentryEnvelope *)envelope writeData:(SentryDataWriter NSData *header = [SentrySerialization dataWithJSONObject:serializedData]; if (nil == header) { SENTRY_LOG_ERROR(@"Envelope header cannot be converted to JSON."); - return NO; + return nil; } - writeData(header); + [envelopeData appendData:header]; for (int i = 0; i < envelope.items.count; ++i) { - writeData([@"\n" dataUsingEncoding:NSUTF8StringEncoding]); + [envelopeData appendData:[@"\n" dataUsingEncoding:NSUTF8StringEncoding]]; NSDictionary *serializedItemHeaderData = [envelope.items[i].header serialize]; NSData *itemHeader = [SentrySerialization dataWithJSONObject:serializedItemHeaderData]; if (nil == itemHeader) { SENTRY_LOG_ERROR(@"Envelope item header cannot be converted to JSON."); - return NO; + return nil; } - writeData(itemHeader); - writeData([@"\n" dataUsingEncoding:NSUTF8StringEncoding]); - writeData(envelope.items[i].data); + [envelopeData appendData:itemHeader]; + [envelopeData appendData:[@"\n" dataUsingEncoding:NSUTF8StringEncoding]]; + [envelopeData appendData:envelope.items[i].data]; } - return YES; -} - -+ (NSData *_Nullable)dataWithEnvelope:(SentryEnvelope *)envelope -{ - NSMutableData *envelopeData = [[NSMutableData alloc] init]; - - BOOL result = - [SentrySerialization writeEnvelopeData:envelope - writeData:^(NSData *data) { [envelopeData appendData:data]; }]; - - if (result == YES) { - return envelopeData; - } else { - return nil; - } + return envelopeData; } + (SentryEnvelope *_Nullable)envelopeWithData:(NSData *)data diff --git a/Sources/Sentry/include/SentryFileManager.h b/Sources/Sentry/include/SentryFileManager.h index 5c55ca07131..698d9e5d57a 100644 --- a/Sources/Sentry/include/SentryFileManager.h +++ b/Sources/Sentry/include/SentryFileManager.h @@ -38,7 +38,7 @@ SENTRY_NO_INIT - (void)setDelegate:(id)delegate; -- (void)storeEnvelope:(SentryEnvelope *)envelope; +- (NSString *)storeEnvelope:(SentryEnvelope *)envelope; - (void)storeCurrentSession:(SentrySession *)session; - (void)storeCrashedSession:(SentrySession *)session; diff --git a/Sources/Sentry/include/SentrySerialization.h b/Sources/Sentry/include/SentrySerialization.h index dcd171a8d5f..ba9b6753536 100644 --- a/Sources/Sentry/include/SentrySerialization.h +++ b/Sources/Sentry/include/SentrySerialization.h @@ -4,8 +4,6 @@ NS_ASSUME_NONNULL_BEGIN -typedef void (^SentryDataWriter)(NSData *data); - @interface SentrySerialization : NSObject + (NSData *_Nullable)dataWithJSONObject:(id)jsonObject; @@ -14,12 +12,6 @@ typedef void (^SentryDataWriter)(NSData *data); + (SentrySession *_Nullable)sessionWithData:(NSData *)sessionData; -+ (BOOL)writeEnvelopeData:(SentryEnvelope *)envelope writeData:(SentryDataWriter)writeData; - -/** - * For large envelopes, consider using @c writeEnvelopeData, which lets you write the envelope in - * chunks to your desired location, to minimize the memory footprint. - */ + (NSData *_Nullable)dataWithEnvelope:(SentryEnvelope *)envelope; + (NSData *)dataWithReplayRecording:(SentryReplayRecording *)replayRecording; diff --git a/Tests/SentryTests/Helper/SentryFileManagerTests.swift b/Tests/SentryTests/Helper/SentryFileManagerTests.swift index 46c58b06d4e..51c3a38637e 100644 --- a/Tests/SentryTests/Helper/SentryFileManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryFileManagerTests.swift @@ -220,8 +220,7 @@ class SentryFileManagerTests: XCTestCase { func testDontDeleteYoungEnvelopes() throws { let envelope = TestConstants.envelope - sut.store(envelope) - let path = try XCTUnwrap(sut.getAllEnvelopes().last?.path) + let path = sut.store(envelope) let timeIntervalSince1970 = fixture.currentDateProvider.date().timeIntervalSince1970 - (90 * 24 * 60 * 60) let date = Date(timeIntervalSince1970: timeIntervalSince1970) @@ -411,8 +410,7 @@ class SentryFileManagerTests: XCTestCase { } // Trying to load the file content of a directory is going to return nil for the envelope. - sut.store(TestConstants.envelope) - let envelopePath = try XCTUnwrap(sut.getAllEnvelopes().last?.path) + let envelopePath = sut.store(TestConstants.envelope) let fileManager = FileManager.default try! fileManager.removeItem(atPath: envelopePath) try! fileManager.createDirectory(atPath: envelopePath, withIntermediateDirectories: false, attributes: nil) @@ -856,8 +854,7 @@ private extension SentryFileManagerTests { func givenOldEnvelopes() throws { let envelope = TestConstants.envelope - sut.store(envelope) - let path = try XCTUnwrap(sut.getAllEnvelopes().last?.path) + let path = sut.store(envelope) let timeIntervalSince1970 = fixture.currentDateProvider.date().timeIntervalSince1970 - (90 * 24 * 60 * 60) let date = Date(timeIntervalSince1970: timeIntervalSince1970 - 1) diff --git a/Tests/SentryTests/Networking/SentryHttpTransportTests.swift b/Tests/SentryTests/Networking/SentryHttpTransportTests.swift index 527a1fd92f1..eb3dbdeb2e4 100644 --- a/Tests/SentryTests/Networking/SentryHttpTransportTests.swift +++ b/Tests/SentryTests/Networking/SentryHttpTransportTests.swift @@ -470,8 +470,7 @@ class SentryHttpTransportTests: XCTestCase { } func testAllCachedEnvelopesCantDeserializeEnvelope() throws { - fixture.fileManager.store(TestConstants.envelope) - let path = try XCTUnwrap(fixture.fileManager.getAllEnvelopes().last?.path) + let path = fixture.fileManager.store(TestConstants.envelope) let faultyEnvelope = Data([0x70, 0xa3, 0x10, 0x45]) try faultyEnvelope.write(to: URL(fileURLWithPath: path))