fix: crash when converting invalid JSON object (#68)

Co-authored-by: xiaoweii <[email protected]>
awslabs · Jun 4, 2024 · 890b347 · 890b347
1 parent be5d15c
commit 890b347
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 2 deletions.
diff --git a/Sources/Clickstream/Dependency/Clickstream/Analytics/EventRecorder.swift b/Sources/Clickstream/Dependency/Clickstream/Analytics/EventRecorder.swift
@@ -45,6 +45,7 @@ class EventRecorder: AnalyticsEventRecording {
     func save(_ event: ClickstreamEvent) throws {
         let eventObject = event.toJsonObject()
         let eventJson = eventObject.toJsonString()
+        if eventJson == "" { return }
         let eventSize = eventJson.count
         let storageEvent = StorageEvent(eventJson: eventJson, eventSize: Int64(eventSize))
         try dbUtil.saveEvent(storageEvent)

diff --git a/Sources/Clickstream/Support/Extension/JsonObject+ToJsonString.swift b/Sources/Clickstream/Support/Extension/JsonObject+ToJsonString.swift
@@ -9,11 +9,15 @@ import Foundation
 
 extension JsonObject {
     func toJsonString() -> String {
+        if !JSONSerialization.isValidJSONObject(self) {
+            log.error("Invalid JSON object: \(self)")
+            return ""
+        }
         do {
             let jsonData = try JSONSerialization.data(withJSONObject: self, options: [.sortedKeys])
             return String(data: jsonData, encoding: .utf8) ?? ""
         } catch {
-            print("Error serializing dictionary to JSON: \(error.localizedDescription)")
+            log.error("Error serializing dictionary to JSON: \(error.localizedDescription)")
         }
         return ""
     }
@@ -23,8 +27,10 @@ extension JsonObject {
             let jsonData = try JSONSerialization.data(withJSONObject: self, options: [.sortedKeys, .prettyPrinted])
             return String(data: jsonData, encoding: .utf8) ?? ""
         } catch {
-            print("Error serializing dictionary to JSON: \(error.localizedDescription)")
+            log.error("Error serializing dictionary to JSON: \(error.localizedDescription)")
         }
         return ""
     }
 }
+
+extension JsonObject: ClickstreamLogger {}
diff --git a/Tests/ClickstreamTests/Clickstream/EventRecorderTest.swift b/Tests/ClickstreamTests/Clickstream/EventRecorderTest.swift
@@ -183,6 +183,12 @@ class EventRecorderTest: XCTestCase {
         XCTAssertEqual("carl", (userAttributes["_user_name"] as! JsonObject)["value"] as! String)
     }
 
+    func testRecordEventWithInvalidAttribute() throws {
+        clickstreamEvent.addGlobalAttribute(Decimal.nan, forKey: "invalidDecimal")
+        try eventRecorder.save(clickstreamEvent)
+        XCTAssertEqual(0, try dbUtil.getEventCount())
+    }
+
     func testSaveMultiEvent() throws {
         for _ in 0 ..< 5 {
             try eventRecorder.save(clickstreamEvent)

diff --git a/Tests/ClickstreamTests/Util/ToJsonStringUtilTest.swift b/Tests/ClickstreamTests/Util/ToJsonStringUtilTest.swift
@@ -0,0 +1,46 @@
+//
+// Copyright Amazon.com Inc. or its affiliates.
+// All Rights Reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+@testable import Clickstream
+import XCTest
+
+class ToJsonStringUtilTest: XCTestCase {
+    func testNullObjectToJsonString() {
+        let attr: JsonObject = [
+            "user": {}
+        ]
+        XCTAssertTrue(attr.toJsonString() == "")
+    }
+
+    func testNSObjectToJsonString() {
+        let attr: JsonObject = [
+            "user": NSObject()
+        ]
+        XCTAssertTrue(attr.toJsonString() == "")
+    }
+
+    func testNanObjectToJsonString() {
+        let attr: JsonObject = [
+            "doubleKey": Double.nan
+        ]
+        XCTAssertTrue(attr.toJsonString() == "")
+    }
+
+    func testInfiniteObjectToJsonString() {
+        let attr: JsonObject = [
+            "doubleKey": Double.infinity
+        ]
+        XCTAssertTrue(attr.toJsonString() == "")
+    }
+
+    func testDateObjectToJsonString() {
+        let attr: JsonObject = [
+            "dateKey": Date()
+        ]
+        XCTAssertTrue(attr.toJsonString() == "")
+    }
+}