segmentio / analytics-ios

The hassle-free way to integrate analytics into any iOS application.
https://segment.com/libraries/ios
MIT License
402 stars 334 forks source link

NSDictionary's serializableDeepCopy does not catch Infinite Double/Float values, causes crash #1045

Open dfed opened 2 years ago

dfed commented 2 years ago

This repo does a good job of transforming Dictionary types from non-JSON-compatible types to JSON-compatible types prior to sending analytics dictionaries to your backend via the serializableDeepCopy method.

However, if an infinite Double or Float value (created by dividing a Double or Float by zero) slips through the serializableDeepCopy method. The result of sending an infinite value is that this repo crashes with the error SIGABRT (#0): Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'Invalid number value (infinite) in JSON write'. Worse yet, this crash happens on Segment's own queue, making it difficult for teams to find the source of the corrupted log. Here's a screen grab from the stack trace showing Segment on its own queue:

image

Ideally, this repo would detect an Infinite value as part of -[NSDictionary serializableDeepCopy:] and either crash (since this method is called synchronously from -[SEGAnalytics track:properties:options:], crashing here would yield a stack trace can help identify the source of the bad log) or remove/modify the infinite value.

Let me know which approach seems reasonable to you – I'd be happy to create a PR with a fix here.

ankittlp commented 2 years ago

Any solution ?

dfed commented 2 years ago

@ankittlp we ended up implementing data validation prior to calling into Segment to help find invalid data issues in our app. This issue is tracking adding that data validation to the Segment API.

ankittlp commented 2 years ago

@ankittlp we ended up implementing data validation prior to calling into Segment to help find invalid data issues in our app. This issue is tracking adding that data validation to the Segment API.

I am seeing this https://github.com/segmentio/analytics-ios/issues/1043 which brings me to the same function of deepCopy. Not sure if any invalid data is passed. As this is working fine on the prior iOS version < iOS 16.

dfed commented 2 years ago

Interesting! Though that issue doesn't seem directly related to this one.