search

datatheorem / TrustKit

Easy SSL pinning validation and reporting for iOS, macOS, tvOS and watchOS.
MIT License
2.01k stars 362 forks source link

Production crashes #240

Closed ghost closed 3 years ago

ghost commented 3 years ago

Could I ask what is the point of this Assert that crashes the app?

I am tempted to simply remove it, as it is causing crashes for some users out in the real world, but I cannot get my head around why the app cannot function without it?

Later edit: I have forked the repository, and without persisting the cache on disk, it seems that the only downside is having to rerun the validation next time the app starts. In my case, that's fine, as it seems the first time an app runs post install, there are some issues with accessing disk storage and persisting anything. So, I am really not convinced an NSAssert, which seems to crash the app is a good way to handle a routine check. I'm not very familiar with Obj-C, but in Swift, I would suggest using an assertionFailure, which will prevent the app from crashing pointlessly in Production, whilst crashing it in Debugging mode. This should give the chance to raise awareness on some issue at the appropriate time, which I suppose is the intended behaviour.

r3to commented 3 years ago

Hi @colcalex NSAssert is usually disabled on production builds. It looks like that is also the intention in that case. As it would simple log the Error in production. Maybe this discussion can help you? https://forums.swift.org/t/assertions-in-swift-packages/42692