From bf8f908c541b3221828ff5706d8c54fdfabd0d2a Mon Sep 17 00:00:00 2001 From: Brandon Sneed Date: Tue, 28 Nov 2023 09:26:10 -0800 Subject: [PATCH 1/8] Add JSONSafeEncoder for NaN/Infinity handling in JSON --- Package.resolved | 33 ++++++++++++-------- Package.swift | 4 ++- Package@swift-5.9.swift | 6 ++-- Sources/Segment/ObjC/ObjCAnalytics.swift | 3 +- Sources/Segment/ObjC/ObjCConfiguration.swift | 3 +- Sources/Segment/Utilities/JSON.swift | 6 ++-- Sources/Segment/Utilities/Utils.swift | 4 +++ Sources/Segment/Utilities/iso8601.swift | 8 ++--- Tests/Segment-Tests/JSON_Tests.swift | 9 +++--- 9 files changed, 47 insertions(+), 29 deletions(-) diff --git a/Package.resolved b/Package.resolved index 148e0e79..4908cb84 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,16 +1,23 @@ { - "object": { - "pins": [ - { - "package": "Sovran", - "repositoryURL": "https://github.com/segmentio/Sovran-Swift.git", - "state": { - "branch": null, - "revision": "64f3b5150c282a34af4578188dce2fd597e600e3", - "version": "1.1.0" - } + "pins" : [ + { + "identity" : "jsonsafeencoder-swift", + "kind" : "remoteSourceControl", + "location" : "https://github.com/segmentio/jsonsafeencoder-swift.git", + "state" : { + "revision" : "75ad40f07d4e0b938e3afb80811244d6b7acd4ba", + "version" : "1.0.0" } - ] - }, - "version": 1 + }, + { + "identity" : "sovran-swift", + "kind" : "remoteSourceControl", + "location" : "https://github.com/segmentio/Sovran-Swift.git", + "state" : { + "revision" : "64f3b5150c282a34af4578188dce2fd597e600e3", + "version" : "1.1.0" + } + } + ], + "version" : 2 } diff --git a/Package.swift b/Package.swift index 61ff3e2b..282e584b 100644 --- a/Package.swift +++ b/Package.swift @@ -20,7 +20,8 @@ let package = Package( dependencies: [ // Dependencies declare other packages that this package depends on. // .package(url: /* package url */, from: "1.0.0"), - .package(url: "https://github.com/segmentio/sovran-swift.git", from: "1.1.0") + .package(url: "https://github.com/segmentio/sovran-swift.git", from: "1.1.0"), + .package(url: "https://github.com/segmentio/jsonsafeencoder-swift.git", from: "1.0.0") ], targets: [ // Targets are the basic building blocks of a package. A target can define a module or a test suite. @@ -29,6 +30,7 @@ let package = Package( name: "Segment", dependencies: [ .product(name: "Sovran", package: "sovran-swift") + .product(name: "JSONSafeEncoder", package: "jsonsafeencoder-swift") ], exclude: ["PrivacyInfo.xcprivacy"]), .testTarget( diff --git a/Package@swift-5.9.swift b/Package@swift-5.9.swift index a1aec1a7..a088a180 100644 --- a/Package@swift-5.9.swift +++ b/Package@swift-5.9.swift @@ -21,7 +21,8 @@ let package = Package( dependencies: [ // Dependencies declare other packages that this package depends on. // .package(url: /* package url */, from: "1.0.0"), - .package(url: "https://github.com/segmentio/sovran-swift.git", from: "1.1.0") + .package(url: "https://github.com/segmentio/sovran-swift.git", from: "1.1.0"), + .package(url: "https://github.com/segmentio/jsonsafeencoder-swift.git", from: "1.0.0") ], targets: [ // Targets are the basic building blocks of a package. A target can define a module or a test suite. @@ -29,7 +30,8 @@ let package = Package( .target( name: "Segment", dependencies: [ - .product(name: "Sovran", package: "sovran-swift") + .product(name: "Sovran", package: "sovran-swift"), + .product(name: "JSONSafeEncoder", package: "jsonsafeencoder-swift") ], exclude: ["PrivacyInfo.xcprivacy"]), .testTarget( diff --git a/Sources/Segment/ObjC/ObjCAnalytics.swift b/Sources/Segment/ObjC/ObjCAnalytics.swift index 8cfc0f54..93f32d4c 100644 --- a/Sources/Segment/ObjC/ObjCAnalytics.swift +++ b/Sources/Segment/ObjC/ObjCAnalytics.swift @@ -8,6 +8,7 @@ #if !os(Linux) import Foundation +import JSONSafeEncoder // MARK: - ObjC Compatibility @@ -164,7 +165,7 @@ extension ObjCAnalytics { var result: [String: Any]? = nil if let system: System = analytics.store.currentState() { do { - let encoder = JSONEncoder.default + let encoder = JSONSafeEncoder.default let json = try encoder.encode(system.settings) if let r = try JSONSerialization.jsonObject(with: json) as? [String: Any] { result = r diff --git a/Sources/Segment/ObjC/ObjCConfiguration.swift b/Sources/Segment/ObjC/ObjCConfiguration.swift index f07324f6..261e9da6 100644 --- a/Sources/Segment/ObjC/ObjCConfiguration.swift +++ b/Sources/Segment/ObjC/ObjCConfiguration.swift @@ -8,6 +8,7 @@ #if !os(Linux) import Foundation +import JSONSafeEncoder @objc(SEGConfiguration) public class ObjCConfiguration: NSObject { @@ -75,7 +76,7 @@ public class ObjCConfiguration: NSObject { get { var result = [String: Any]() do { - let encoder = JSONEncoder.default + let encoder = JSONSafeEncoder.default let json = try encoder.encode(configuration.values.defaultSettings) if let r = try JSONSerialization.jsonObject(with: json) as? [String: Any] { result = r diff --git a/Sources/Segment/Utilities/JSON.swift b/Sources/Segment/Utilities/JSON.swift index 5ef8f224..86924d37 100644 --- a/Sources/Segment/Utilities/JSON.swift +++ b/Sources/Segment/Utilities/JSON.swift @@ -6,7 +6,7 @@ // import Foundation - +import JSONSafeEncoder // MARK: - JSON Definition @@ -35,7 +35,7 @@ public enum JSON: Equatable { // For Value types public init(with value: T) throws { - let encoder = JSONEncoder.default + let encoder = JSONSafeEncoder.default let json = try encoder.encode(value) let output = try JSONSerialization.jsonObject(with: json) try self.init(output) @@ -136,7 +136,7 @@ extension Encodable { public func toString(pretty: Bool) -> String { var returnString = "" do { - let encoder = JSONEncoder.default + let encoder = JSONSafeEncoder.default if pretty { encoder.outputFormatting = .prettyPrinted } diff --git a/Sources/Segment/Utilities/Utils.swift b/Sources/Segment/Utilities/Utils.swift index e317738e..db7612ea 100644 --- a/Sources/Segment/Utilities/Utils.swift +++ b/Sources/Segment/Utilities/Utils.swift @@ -69,6 +69,8 @@ extension Optional: Flattenable { } } +/* for dev testing only +#if DEBUG class TrackingDispatchGroup: CustomStringConvertible { internal let group = DispatchGroup() @@ -102,3 +104,5 @@ class TrackingDispatchGroup: CustomStringConvertible { group.notify(qos: qos, flags: flags, queue: queue, execute: work) } } +#endif +*/ diff --git a/Sources/Segment/Utilities/iso8601.swift b/Sources/Segment/Utilities/iso8601.swift index 1a098ce8..93069f50 100644 --- a/Sources/Segment/Utilities/iso8601.swift +++ b/Sources/Segment/Utilities/iso8601.swift @@ -6,6 +6,7 @@ // import Foundation +import JSONSafeEncoder enum SegmentISO8601DateFormatter { static let shared: ISO8601DateFormatter = { @@ -16,7 +17,6 @@ enum SegmentISO8601DateFormatter { } internal extension Date { - // TODO: support nanoseconds func iso8601() -> String { return SegmentISO8601DateFormatter.shared.string(from: self) } @@ -47,9 +47,9 @@ extension JSONDecoder { } } -extension JSONEncoder { - static var `default`: JSONEncoder { - let e = JSONEncoder() +extension JSONSafeEncoder { + static var `default`: JSONSafeEncoder { + let e = JSONSafeEncoder() e.dateEncodingStrategy = .formatted(DateFormatter.iso8601) return e } diff --git a/Tests/Segment-Tests/JSON_Tests.swift b/Tests/Segment-Tests/JSON_Tests.swift index ecfe41d3..c649fcfc 100644 --- a/Tests/Segment-Tests/JSON_Tests.swift +++ b/Tests/Segment-Tests/JSON_Tests.swift @@ -6,6 +6,7 @@ // import XCTest +import JSONSafeEncoder @testable import Segment struct Personal: Codable { @@ -39,7 +40,7 @@ class JSONTests: XCTestCase { let traits = try? JSON(["email": "blah@blah.com"]) let userInfo = UserInfo(anonymousId: "1234", userId: "brandon", traits: traits, referrer: nil) - let encoder = JSONEncoder.default + let encoder = JSONSafeEncoder.default encoder.outputFormatting = .prettyPrinted do { @@ -60,7 +61,7 @@ class JSONTests: XCTestCase { let test = TestStruct(myDate: now) let object = try JSON(with: test) - let encoder = JSONEncoder.default + let encoder = JSONSafeEncoder.default encoder.outputFormatting = .prettyPrinted do { @@ -91,7 +92,7 @@ class JSONTests: XCTestCase { func testJSONNil() throws { let traits = try JSON(["type": NSNull(), "preferences": ["bwack"], "key": nil] as [String : Any?]) - let encoder = JSONEncoder.default + let encoder = JSONSafeEncoder.default encoder.outputFormatting = .prettyPrinted do { @@ -109,7 +110,7 @@ class JSONTests: XCTestCase { let test = TestStruct(blah: "hello") let object = try JSON(with: test) - let encoder = JSONEncoder.default + let encoder = JSONSafeEncoder.default encoder.outputFormatting = .prettyPrinted do { From 4d5cff9565985e073ad32b5f89de662c944ee938 Mon Sep 17 00:00:00 2001 From: Brandon Sneed Date: Tue, 28 Nov 2023 09:41:19 -0800 Subject: [PATCH 2/8] Make it all user-settable. --- Sources/Segment/Configuration.swift | 13 ++++++++++++- Sources/Segment/Utilities/JSON.swift | 19 +++++++++++++++++++ Sources/Segment/Utilities/iso8601.swift | 16 ---------------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/Sources/Segment/Configuration.swift b/Sources/Segment/Configuration.swift index f43512c7..b2650d95 100644 --- a/Sources/Segment/Configuration.swift +++ b/Sources/Segment/Configuration.swift @@ -6,6 +6,7 @@ // import Foundation +import JSONSafeEncoder #if os(Linux) import FoundationNetworking #endif @@ -37,10 +38,10 @@ public class Configuration { var requestFactory: ((URLRequest) -> URLRequest)? = nil var errorHandler: ((Error) -> Void)? = nil var flushPolicies: [FlushPolicy] = [CountBasedFlushPolicy(), IntervalBasedFlushPolicy()] - var operatingMode: OperatingMode = .asynchronous var flushQueue: DispatchQueue = OperatingMode.defaultQueue var userAgent: String? = nil + var jsonNonConformingNumberStrategy: JSONSafeEncoder.NonConformingFloatEncodingStrategy = .zero } internal var values: Values @@ -216,11 +217,21 @@ public extension Configuration { return self } + /// Specify a custom UserAgent string. This bypasses the OS dependent check entirely. @discardableResult func userAgent(_ userAgent: String) -> Configuration { values.userAgent = userAgent return self } + + /// This option specifies how NaN/Infinity are handled when encoding JSON. + /// The default is .zero. See JSONSafeEncoder.NonConformingFloatEncodingStrategy for more informatino. + @discardableResult + func jsonNonConformingNumberStrategy(_ strategy: JSONSafeEncoder.NonConformingFloatEncodingStrategy) -> Configuration { + values.jsonNonConformingNumberStrategy = strategy + JSON.jsonNonConformingNumberStrategy = values.jsonNonConformingNumberStrategy + return self + } } extension Analytics { diff --git a/Sources/Segment/Utilities/JSON.swift b/Sources/Segment/Utilities/JSON.swift index 86924d37..8869854f 100644 --- a/Sources/Segment/Utilities/JSON.swift +++ b/Sources/Segment/Utilities/JSON.swift @@ -8,6 +8,23 @@ import Foundation import JSONSafeEncoder +extension JSONDecoder { + static var `default`: JSONDecoder { + let d = JSONDecoder() + d.dateDecodingStrategy = .formatted(DateFormatter.iso8601) + return d + } +} + +extension JSONSafeEncoder { + static var `default`: JSONSafeEncoder { + let e = JSONSafeEncoder() + e.dateEncodingStrategy = .formatted(DateFormatter.iso8601) + e.nonConformingFloatEncodingStrategy = JSON.jsonNonConformingNumberStrategy + return e + } +} + // MARK: - JSON Definition public enum JSON: Equatable { @@ -18,6 +35,8 @@ public enum JSON: Equatable { case array([JSON]) case object([String: JSON]) + static var jsonNonConformingNumberStrategy: JSONSafeEncoder.NonConformingFloatEncodingStrategy = .zero + internal enum JSONError: Error { case unknown case nonJSONType(type: String) diff --git a/Sources/Segment/Utilities/iso8601.swift b/Sources/Segment/Utilities/iso8601.swift index 93069f50..d99830e3 100644 --- a/Sources/Segment/Utilities/iso8601.swift +++ b/Sources/Segment/Utilities/iso8601.swift @@ -38,19 +38,3 @@ extension DateFormatter { return formatter }() } - -extension JSONDecoder { - static var `default`: JSONDecoder { - let d = JSONDecoder() - d.dateDecodingStrategy = .formatted(DateFormatter.iso8601) - return d - } -} - -extension JSONSafeEncoder { - static var `default`: JSONSafeEncoder { - let e = JSONSafeEncoder() - e.dateEncodingStrategy = .formatted(DateFormatter.iso8601) - return e - } -} From bc06a340eac3c09771725288f12ef3562158cc10 Mon Sep 17 00:00:00 2001 From: Brandon Sneed Date: Tue, 28 Nov 2023 09:59:00 -0800 Subject: [PATCH 3/8] Add tests --- Tests/Segment-Tests/Analytics_Tests.swift | 34 +++++++++++ Tests/Segment-Tests/JSON_Tests.swift | 73 +++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/Tests/Segment-Tests/Analytics_Tests.swift b/Tests/Segment-Tests/Analytics_Tests.swift index 5c8796e0..e3839c1d 100644 --- a/Tests/Segment-Tests/Analytics_Tests.swift +++ b/Tests/Segment-Tests/Analytics_Tests.swift @@ -823,4 +823,38 @@ final class Analytics_Tests: XCTestCase { XCTAssertEqual(ziggysFound!.count, 3) XCTAssertEqual(goobersFound!.count, 2) } + + func testJSONNaNDefaultHandlingZero() throws { + let analytics = Analytics(configuration: Configuration(writeKey: "test")) + let outputReader = OutputReaderPlugin() + analytics.add(plugin: outputReader) + + waitUntilStarted(analytics: analytics) + + analytics.track(name: "test track", properties: ["TestNaN": Double.nan]) + + let trackEvent: TrackEvent? = outputReader.lastEvent as? TrackEvent + XCTAssertTrue(trackEvent?.event == "test track") + XCTAssertTrue(trackEvent?.type == "track") + let d: Double? = trackEvent?.properties?.value(forKeyPath: "TestNaN") + XCTAssertTrue(d! == 0) + } + + func testJSONNaNDefaultHandlingNull() throws { + let analytics = Analytics(configuration: Configuration(writeKey: "test") + .jsonNonConformingNumberStrategy(.null) + ) + let outputReader = OutputReaderPlugin() + analytics.add(plugin: outputReader) + + waitUntilStarted(analytics: analytics) + + analytics.track(name: "test track", properties: ["TestNaN": Double.nan]) + + let trackEvent: TrackEvent? = outputReader.lastEvent as? TrackEvent + XCTAssertTrue(trackEvent?.event == "test track") + XCTAssertTrue(trackEvent?.type == "track") + let d: Double? = trackEvent?.properties?.value(forKeyPath: "TestNaN") + XCTAssertNil(d) + } } diff --git a/Tests/Segment-Tests/JSON_Tests.swift b/Tests/Segment-Tests/JSON_Tests.swift index c649fcfc..56ee093c 100644 --- a/Tests/Segment-Tests/JSON_Tests.swift +++ b/Tests/Segment-Tests/JSON_Tests.swift @@ -324,4 +324,77 @@ class JSONTests: XCTestCase { XCTAssertThrowsError(try json?.remove(key: "merchant")) } + func testJSONNaNZero() throws { + struct TestStruct: Codable { + let str: String + let decimal: Double + let nando: Double + } + let nan = NSNumber.FloatLiteralType(nan: 1, signaling: true) + + JSON.jsonNonConformingNumberStrategy = .zero + + do { + let o = try JSON(nan) + XCTAssertNotNil(o) + } catch { + print(error) + XCTFail() + } + + let test = TestStruct( + str: "hello", + decimal: 333.9999, + nando: nan + ) + + do { + let o = try JSON(with: test) + XCTAssertNotNil(o) + + let t: TestStruct? = o.codableValue() + XCTAssertNotNil(t) + XCTAssertTrue(t!.nando == 0) + } catch { + print(error) + XCTFail() + } + } + + func testJSONNaNNull() throws { + struct TestStruct: Codable { + let str: String + let decimal: Double + let nando: Double? + } + let nan = NSNumber.FloatLiteralType(nan: 1, signaling: true) + + JSON.jsonNonConformingNumberStrategy = .null + + do { + let o = try JSON(nan) + XCTAssertNotNil(o) + } catch { + print(error) + XCTFail() + } + + let test = TestStruct( + str: "hello", + decimal: 333.9999, + nando: nan + ) + + do { + let o = try JSON(with: test) + XCTAssertNotNil(o) + + let t: TestStruct? = o.codableValue() + XCTAssertNotNil(t) + XCTAssertNil(t!.nando) + } catch { + print(error) + XCTFail() + } + } } From 5a1f8f216b387426f0099253c5b511029b41fff5 Mon Sep 17 00:00:00 2001 From: Brandon Sneed Date: Tue, 28 Nov 2023 10:04:40 -0800 Subject: [PATCH 4/8] Fix missing comma. --- Package.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Package.swift b/Package.swift index 282e584b..eafeb95d 100644 --- a/Package.swift +++ b/Package.swift @@ -29,7 +29,7 @@ let package = Package( .target( name: "Segment", dependencies: [ - .product(name: "Sovran", package: "sovran-swift") + .product(name: "Sovran", package: "sovran-swift"), .product(name: "JSONSafeEncoder", package: "jsonsafeencoder-swift") ], exclude: ["PrivacyInfo.xcprivacy"]), From 1a0cd799859e035cc38c00d6240bab7d07f84d7d Mon Sep 17 00:00:00 2001 From: Brandon Sneed Date: Tue, 28 Nov 2023 10:10:23 -0800 Subject: [PATCH 5/8] Specified value for zero test --- Tests/Segment-Tests/Analytics_Tests.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Tests/Segment-Tests/Analytics_Tests.swift b/Tests/Segment-Tests/Analytics_Tests.swift index e3839c1d..1f190b86 100644 --- a/Tests/Segment-Tests/Analytics_Tests.swift +++ b/Tests/Segment-Tests/Analytics_Tests.swift @@ -825,7 +825,9 @@ final class Analytics_Tests: XCTestCase { } func testJSONNaNDefaultHandlingZero() throws { - let analytics = Analytics(configuration: Configuration(writeKey: "test")) + let analytics = Analytics(configuration: Configuration(writeKey: "test") + .jsonNonConformingNumberStrategy(.zero) + ) let outputReader = OutputReaderPlugin() analytics.add(plugin: outputReader) From e7da180d0368afaa76266cf54eabfd096b1169ba Mon Sep 17 00:00:00 2001 From: Brandon Sneed Date: Tue, 28 Nov 2023 10:15:29 -0800 Subject: [PATCH 6/8] Set configuration value to JSON's static prop. --- Sources/Segment/Configuration.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Segment/Configuration.swift b/Sources/Segment/Configuration.swift index b2650d95..e77591cf 100644 --- a/Sources/Segment/Configuration.swift +++ b/Sources/Segment/Configuration.swift @@ -51,6 +51,7 @@ public class Configuration { /// - Parameter writeKey: Your Segment write key value public init(writeKey: String) { self.values = Values(writeKey: writeKey) + JSON.jsonNonConformingNumberStrategy = self.values.jsonNonConformingNumberStrategy // enable segment destination by default var settings = Settings(writeKey: writeKey) settings.integrations = try? JSON([ From 7c783d24fe6d91a69289190ff1f61a7285603053 Mon Sep 17 00:00:00 2001 From: Brandon Sneed Date: Tue, 28 Nov 2023 10:16:07 -0800 Subject: [PATCH 7/8] Updated tests --- Tests/Segment-Tests/Analytics_Tests.swift | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Tests/Segment-Tests/Analytics_Tests.swift b/Tests/Segment-Tests/Analytics_Tests.swift index 1f190b86..7fc373ad 100644 --- a/Tests/Segment-Tests/Analytics_Tests.swift +++ b/Tests/Segment-Tests/Analytics_Tests.swift @@ -825,9 +825,8 @@ final class Analytics_Tests: XCTestCase { } func testJSONNaNDefaultHandlingZero() throws { - let analytics = Analytics(configuration: Configuration(writeKey: "test") - .jsonNonConformingNumberStrategy(.zero) - ) + // notice we didn't set the nan handling option. zero is the default. + let analytics = Analytics(configuration: Configuration(writeKey: "test")) let outputReader = OutputReaderPlugin() analytics.add(plugin: outputReader) @@ -842,7 +841,7 @@ final class Analytics_Tests: XCTestCase { XCTAssertTrue(d! == 0) } - func testJSONNaNDefaultHandlingNull() throws { + func testJSONNaNHandlingNull() throws { let analytics = Analytics(configuration: Configuration(writeKey: "test") .jsonNonConformingNumberStrategy(.null) ) From a20b1a1701ca4c839eec2a46ce0e1dfeb342426b Mon Sep 17 00:00:00 2001 From: Brandon Sneed Date: Tue, 28 Nov 2023 10:28:38 -0800 Subject: [PATCH 8/8] Fixed example build. --- .../DestinationsExample/ViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Examples/apps/DestinationsExample/DestinationsExample/ViewController.swift b/Examples/apps/DestinationsExample/DestinationsExample/ViewController.swift index a51def09..f9ffda0d 100644 --- a/Examples/apps/DestinationsExample/DestinationsExample/ViewController.swift +++ b/Examples/apps/DestinationsExample/DestinationsExample/ViewController.swift @@ -84,7 +84,7 @@ class ViewController: UIViewController { case .alias: aliasEvent() case .none: - analytics?.log(message: "Failed to establish event type", kind: .error) + analytics?.log(message: "Failed to establish event type") } clearAll()