fix: Serialization with invalid objects (#2858)

Serialization may fail and crash if there is an invalid objects in the data.

Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Crash when serializing invalid objects (#2858)
+
 ## 8.4.0
 
 ### Features
@@ -11,7 +17,7 @@
 ### Fixes
 
 - Correctly track and send GPU frame render data in profiles (#2823)
-- Xcode 14.3 compiling issue regarding functions declaration with no prototype (#2852)
+- Xcode 14.3 compiling issue regarding functions declaration with no prototype (#2852) 
 
 ## 8.3.3
 

Sources/Sentry/SentryEnvelope.m

@@ -63,10 +63,9 @@ - (instancetype)initWithHeader:(SentryEnvelopeItemHeader *)header data:(NSData *
 
 - (instancetype)initWithEvent:(SentryEvent *)event
 {
-    NSError *error;
-    NSData *json = [SentrySerialization dataWithJSONObject:[event serialize] error:&error];
+    NSData *json = [SentrySerialization dataWithJSONObject:[event serialize]];
 
-    if (nil != error) {
+    if (nil == json) {
         // We don't know what caused the serialization to fail.
         SentryEvent *errorEvent = [[SentryEvent alloc] initWithLevel:kSentryLevelWarning];
 
@@ -83,7 +82,7 @@ - (instancetype)initWithEvent:(SentryEvent *)event
 
         // We accept the risk that this simple serialization fails. Therefore we ignore the
         // error on purpose.
-        json = [SentrySerialization dataWithJSONObject:[errorEvent serialize] error:nil];
+        json = [SentrySerialization dataWithJSONObject:[errorEvent serialize]];
     }
 
     // event.type can be nil and the server infers error if there's a stack trace, otherwise

Sources/Sentry/SentryFileManager.m

@@ -318,7 +318,7 @@ - (void)storeCrashedSession:(SentrySession *)session
 
 - (void)storeSession:(SentrySession *)session sessionFilePath:(NSString *)sessionFilePath
 {
-    NSData *sessionData = [SentrySerialization dataWithSession:session error:nil];
+    NSData *sessionData = [SentrySerialization dataWithSession:session];
     SENTRY_LOG_DEBUG(@"Writing session: %@", sessionFilePath);
     @synchronized(self.currentSessionFilePath) {
         if (![self writeData:sessionData toPath:sessionFilePath]) {
@@ -444,20 +444,18 @@ - (BOOL)writeData:(NSData *)data toPath:(NSString *)path
 
 - (NSString *)storeDictionary:(NSDictionary *)dictionary toPath:(NSString *)path
 {
-    NSData *saveData = [SentrySerialization dataWithJSONObject:dictionary error:nil];
+    NSData *saveData = [SentrySerialization dataWithJSONObject:dictionary];
     return nil != saveData ? [self storeData:saveData toUniqueJSONPath:path]
                            : path; // TODO: Should we return null instead? Whoever is using this
                                    // return value is being tricked.
 }
 
 - (void)storeAppState:(SentryAppState *)appState
 {
-    NSError *error = nil;
-    NSData *data = [SentrySerialization dataWithJSONObject:[appState serialize] error:&error];
+    NSData *data = [SentrySerialization dataWithJSONObject:[appState serialize]];
 
-    if (error != nil) {
-        SENTRY_LOG_ERROR(
-            @"Failed to store app state, because of an error in serialization: %@", error);
+    if (data == nil) {
+        SENTRY_LOG_ERROR(@"Failed to store app state, because of an error in serialization");
         return;
     }
 

Sources/Sentry/SentryNSURLRequest.m

@@ -29,14 +29,9 @@ - (_Nullable instancetype)initStoreRequestWithDsn:(SentryDsn *)dsn
                                  didFailWithError:(NSError *_Nullable *_Nullable)error
 {
     NSDictionary *serialized = [event serialize];
-    NSData *jsonData = [SentrySerialization dataWithJSONObject:serialized error:error];
+    NSData *jsonData = [SentrySerialization dataWithJSONObject:serialized];
     if (nil == jsonData) {
-        if (error) {
-            // TODO: We're possibly overriding an error set by the actual
-            // code that failed ^
-            *error = NSErrorFromSentryError(
-                kSentryErrorJsonConversionError, @"Event cannot be converted to JSON");
-        }
+        SENTRY_LOG_ERROR(@"Event cannot be converted to JSON");
         return nil;
     }
 

Sources/Sentry/SentryProfiler.mm

@@ -742,10 +742,9 @@ + (NSDictionary *)serializeInfoForTransaction:(SentryTransaction *)transaction
 + (SentryEnvelopeItem *)envelopeItemForProfileData:(NSMutableDictionary<NSString *, id> *)profile
                                          profileID:(SentryId *)profileID
 {
-    NSError *error = nil;
-    const auto JSONData = [SentrySerialization dataWithJSONObject:profile error:&error];
+    const auto JSONData = [SentrySerialization dataWithJSONObject:profile];
     if (JSONData == nil) {
-        SENTRY_LOG_DEBUG(@"Failed to encode profile to JSON: %@", error);
+        SENTRY_LOG_DEBUG(@"Failed to encode profile to JSON.");
         return nil;
     }
 

Sources/Sentry/SentrySerialization.m

@@ -16,35 +16,17 @@
 @implementation SentrySerialization
 
 + (NSData *_Nullable)dataWithJSONObject:(NSDictionary *)dictionary
-                                  error:(NSError *_Nullable *_Nullable)error
 {
-// We'll do this whether we're handling an exception or library error
-#define SENTRY_HANDLE_ERROR(__sentry_error)                                                        \
-    SENTRY_LOG_ERROR(@"Invalid JSON: %@", __sentry_error);                                         \
-    *error = __sentry_error;                                                                       \
-    return nil;
-
-    NSData *data = nil;
-
-#if defined(DEBUG) || defined(TEST) || defined(TESTCI)
-    @try {
-#else
-    if ([NSJSONSerialization isValidJSONObject:dictionary]) {
-#endif
-        data = [NSJSONSerialization dataWithJSONObject:dictionary options:0 error:error];
-#if defined(DEBUG) || defined(TEST) || defined(TESTCI)
-    } @catch (NSException *exception) {
-        if (error) {
-            SENTRY_HANDLE_ERROR(NSErrorFromSentryErrorWithException(
-                kSentryErrorJsonConversionError, @"Event cannot be converted to JSON", exception));
-        }
+    if (![NSJSONSerialization isValidJSONObject:dictionary]) {
+        SENTRY_LOG_ERROR(@"Dictionary is not a valid JSON object.");
+        return nil;
     }
-#else
-    } else if (error) {
-        SENTRY_HANDLE_ERROR(NSErrorFromSentryErrorWithUnderlyingError(
-            kSentryErrorJsonConversionError, @"Event cannot be converted to JSON", *error));
+
+    NSError *error = nil;
+    NSData *data = [NSJSONSerialization dataWithJSONObject:dictionary options:0 error:&error];
+    if (error) {
+        SENTRY_LOG_ERROR(@"Internal error while serializing JSON: %@", error);
     }
-#endif
 
     return data;
 }
@@ -69,13 +51,9 @@ + (NSData *_Nullable)dataWithEnvelope:(SentryEnvelope *)envelope
         [serializedData setValue:[traceContext serialize] forKey:@"trace"];
     }
 
-    NSData *header = [SentrySerialization dataWithJSONObject:serializedData error:error];
+    NSData *header = [SentrySerialization dataWithJSONObject:serializedData];
     if (nil == header) {
         SENTRY_LOG_ERROR(@"Envelope header cannot be converted to JSON.");
-        if (error) {
-            *error = NSErrorFromSentryError(
-                kSentryErrorJsonConversionError, @"Envelope header cannot be converted to JSON");
-        }
         return nil;
     }
     [envelopeData appendData:header];
@@ -84,14 +62,9 @@ + (NSData *_Nullable)dataWithEnvelope:(SentryEnvelope *)envelope
         [envelopeData appendData:[@"\n" dataUsingEncoding:NSUTF8StringEncoding]];
         NSDictionary *serializedItemHeaderData = [envelope.items[i].header serialize];
 
-        NSData *itemHeader = [SentrySerialization dataWithJSONObject:serializedItemHeaderData
-                                                               error:error];
+        NSData *itemHeader = [SentrySerialization dataWithJSONObject:serializedItemHeaderData];
         if (nil == itemHeader) {
             SENTRY_LOG_ERROR(@"Envelope item header cannot be converted to JSON.");
-            if (error) {
-                *error = NSErrorFromSentryError(kSentryErrorJsonConversionError,
-                    @"Envelope item header cannot be converted to JSON");
-            }
             return nil;
         }
         [envelopeData appendData:itemHeader];
@@ -305,9 +278,8 @@ + (SentryEnvelope *_Nullable)envelopeWithData:(NSData *)data
 }
 
 + (NSData *_Nullable)dataWithSession:(SentrySession *)session
-                               error:(NSError *_Nullable *_Nullable)error
 {
-    return [self dataWithJSONObject:[session serialize] error:error];
+    return [self dataWithJSONObject:[session serialize]];
 }
 
 + (SentrySession *_Nullable)sessionWithData:(NSData *)sessionData

Sources/Sentry/include/SentrySerialization.h

@@ -8,11 +8,9 @@ static int const SENTRY_BAGGAGE_MAX_SIZE = 8192;
 
 @interface SentrySerialization : NSObject
 
-+ (NSData *_Nullable)dataWithJSONObject:(NSDictionary *)dictionary
-                                  error:(NSError *_Nullable *_Nullable)error;
++ (NSData *_Nullable)dataWithJSONObject:(NSDictionary *)dictionary;
 
-+ (NSData *_Nullable)dataWithSession:(SentrySession *)session
-                               error:(NSError *_Nullable *_Nullable)error;
++ (NSData *_Nullable)dataWithSession:(SentrySession *)session;
 
 + (NSDictionary<NSString *, NSString *> *)decodeBaggage:(NSString *)baggage;
 + (NSString *)baggageEncodedDictionary:(NSDictionary *)dictionary;

Tests/SentryTests/Helper/SentrySerializationTests.swift

@@ -12,18 +12,7 @@ class SentrySerializationTests: XCTestCase {
             "valid object": "hi, i'm a valid object",
             "invalid object": NSDate()
         ]
-
-        var data: Data?
-        let exp = expectation(description: "Should throw error")
-        do {
-            data = try SentrySerialization.data(withJSONObject: json)
-        } catch {
-            exp.fulfill()
-            //Depending of the iOS version, the underlying type of NSDate may change.
-            //Knowing that we have an error is enough.
-            XCTAssertTrue(error.localizedDescription.starts(with: "Event cannot be converted to JSON (Invalid type in JSON write"))
-        }
-        waitForExpectations(timeout: 1)
+        let data = SentrySerialization.data(withJSONObject: json)
         XCTAssertNil(data)
     }
 
@@ -198,17 +187,17 @@ class SentrySerializationTests: XCTestCase {
         let dict = SentrySession(releaseName: "1.0.0").serialize()
         let session = SentrySession(jsonObject: dict)!
 
-        let data = try SentrySerialization.data(with: session)
+        let data = SentrySerialization.data(with: session)
 
-        XCTAssertNotNil(SentrySerialization.session(with: data))
+        XCTAssertNotNil(SentrySerialization.session(with: data!))
     }
 
     func testSerializeSessionWithNoReleaseName() throws {
         var dict = SentrySession(releaseName: "1.0.0").serialize()
         dict["attrs"] = nil // Remove release name
         let session = SentrySession(jsonObject: dict)!
 
-        let data = try SentrySerialization.data(with: session)
+        let data = SentrySerialization.data(with: session)!
 
         XCTAssertNil(SentrySerialization.session(with: data))
     }
@@ -217,15 +206,15 @@ class SentrySerializationTests: XCTestCase {
         let dict = SentrySession(releaseName: "").serialize()
         let session = SentrySession(jsonObject: dict)!
 
-        let data = try SentrySerialization.data(with: session)
+        let data = SentrySerialization.data(with: session)!
 
         XCTAssertNil(SentrySerialization.session(with: data))
     }
 
     func testSerializeSessionWithGarbageInDict() throws {
         var dict = SentrySession(releaseName: "").serialize()
         dict["started"] = "20"
-        let data = try SentrySerialization.data(withJSONObject: dict)
+        let data = SentrySerialization.data(withJSONObject: dict)!
 
         XCTAssertNil(SentrySerialization.session(with: data))
     }
@@ -252,7 +241,7 @@ class SentrySerializationTests: XCTestCase {
 
     func testAppStateWithValidData_ReturnsValidAppState() throws {
         let appState = TestData.appState
-        let appStateData = try SentrySerialization.data(withJSONObject: appState.serialize())
+        let appStateData = SentrySerialization.data(withJSONObject: appState.serialize())!
 
         let actual = SentrySerialization.appState(with: appStateData)
 

Tests/SentryTests/Protocol/SentryEnvelopeTests.swift

@@ -177,7 +177,7 @@ class SentryEnvelopeTests: XCTestCase {
         let event = fixture.event
         let envelope = SentryEnvelope(event: event)
 
-        let expectedData = try SentrySerialization.data(withJSONObject: event.serialize())
+        let expectedData = SentrySerialization.data(withJSONObject: event.serialize())!
 
         XCTAssertEqual(event.eventId, envelope.header.eventId)
         XCTAssertEqual(1, envelope.items.count)
@@ -222,7 +222,7 @@ class SentryEnvelopeTests: XCTestCase {
         XCTAssertEqual("user_report", item?.header.type)
         XCTAssertNotNil(item?.data)
 
-        let expectedData = try SentrySerialization.data(withJSONObject: userFeedback.serialize())
+        let expectedData = SentrySerialization.data(withJSONObject: userFeedback.serialize())!
 
         let actual = String(data: item?.data ?? Data(), encoding: .utf8)?.sorted()
         let expected = String(data: expectedData, encoding: .utf8)?.sorted()