fix: Crash during SDK initialization due to corrupted envelope (#4291)

Reverting #4219 to further investigation
getsentry · Aug 19, 2024 · a95ba2f · a95ba2f
1 parent 8ef07fb
commit a95ba2f
Show file tree

Hide file tree

Showing 7 changed files with 23 additions and 66 deletions.
diff --git 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
@@ -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
@@ -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
@@ -38,7 +38,7 @@ SENTRY_NO_INIT
 
 - (void)setDelegate:(id<SentryFileManagerDelegate>)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
@@ -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
@@ -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
@@ -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))