Crash when fetching JSON variants containing null values

[squarefrog]

Describe the bug When using JSON feature variants containing null values, Launch Darkly crashes the app each time it attempts to store fetched values.

To reproduce

• Create a new feature flag
• Set the flag variations to JSON
• In variation 1 add ["name": "control", "content": null]
• In variation 2 add ["name": "variation", "content": "something"]
• Set the default variation for both On and Off to Variation 1
• Launch the app

Expected behavior The app should not crash, as the cached data should be handled appropriately.

Logs

2021-11-04 14:03:53.037502+0000 REDACTED[67773:780563] [User Defaults] Attempt to set a non-property-list object {
  // REDACTED - LaunchDarkly feature keys.
} for key com.launchDarkly.cachedUserEnvironmentFlags'
_LSContextInitReturningError() failed with error Error Domain=NSOSStatusErrorDomain Code=-10817 "(null)" UserInfo={_LSFunction=_LSSchemaConfigureForStore, ExpectedSimulatorHash={length = 32, bytes = 0x15dde658 ed2a1267 ab2496d7 34f186ad ... ec431c65 02d68f35 }, _LSLine=409, WrongSimulatorHash={length = 32, bytes = 0xaf25dda9 e45baa35 610eaabd 5bc09901 ... 9cbe61f3 81d7b9d9 }}
terminating with uncaught exception of type NSException
CoreSimulator 783.5 - Device: iPhone 12 Pro Max (4D09FDF5-5501-47BB-BFA4-ECDE91679FD2) - Runtime: iOS 14.5 (18E182) - DeviceType: iPhone 12 Pro Max

SDK version 5.4.2

Language version, developer tools Swift 5.4, Xcode 12.5, iOS 15.

OS/platform iOS 13+

Additional context The app will then crash shortly after launch as the client SDK attempts to store null in UserDefaults.

RFC-7159 defines null as valid in the JSON specification, so it should be safe for us to assume we can use it.

You currently use UserDefaults as the backing store for values, and you cannot store nil values within a property list.

I believe you have two options here:

1. Strip any null values out before persisting
2. Rather than polluting user defaults with LD data, store the config files as encoded JSON within the documents directory

Out of those two I feel the later is the most appropriate.

[gwhelanLD]

Hi @squarefrog,

We believe issue should be fixed in the existing 5.4.3 release, though in our testing for that release we found that this issue only applied to null values nested (potentially deeply) in JSON arrays. Could you clarify whether your testing found this issue for null values nested only within dictionaries (your example uses square brackets and dictionary syntax so I'm not sure which you intended)? The fix introduced in that patch is to extend the first option described (which should already apply to dictionary values) to array values as well. We definitely agree that this is not ideal, but in order to release a quick patch to address the issue we felt that it was a reasonable compromise to strip the null values from arrays as it is consistent with the current behavior for dictionaries. We plan to address this to be consistent with other SDKs (supporting null values in JSON structures) in a future release. I'll close this issue, but feel free to reopen if updating the to 5.4.3 release does not resolve the crash.

Thanks, @gwhelanLD

[squarefrog]

Thanks for the reply, we did actually experience this within a nested array, so it does sound like your 5.4.3 would solve our issue. I'm not sure why our CI didn't flag our pod as outdated, but I'll try it out when I'm in the office again.

My only worry is this does crash existing users if we turn the toggle back on in production. For now we've just stripped the null values manually, and added an internal ticket to ensure we have no null values in the experiment.

Stripping null values is perfectly acceptable for a hot fix.