Skip to content

fix(ios): prevent crash in saveFailedUpdate when server returns null fields#43

Open
MatthewPattell wants to merge 1 commit intorevopush:masterfrom
MatthewPattell:fix/ios-savefailedupdate-nsnull
Open

fix(ios): prevent crash in saveFailedUpdate when server returns null fields#43
MatthewPattell wants to merge 1 commit intorevopush:masterfrom
MatthewPattell:fix/ios-savefailedupdate-nsnull

Conversation

@MatthewPattell
Copy link
Copy Markdown

Problem

-[CodePush saveFailedUpdate:] crashes the app on iOS with NSInvalidArgumentException when the failed package metadata contains NSNull values for optional fields:

Fatal Exception: NSInvalidArgumentException
Attempt to insert non-property list object ( {
    appVersion = "1.6.0";
    assetDownloadUrl = "<null>";
    binaryDate = "1776900944.000000";
    bundleDiffBlobUrl = "<null>";
    bundlePath = "revopush_package.../main.jsbundle";
    deploymentKey = ...;
    description = "";
    downloadUrl = "https://blob.revopush.org/...";
    failedInstall = 0;
    isMandatory = 0;
    isPending = 0;
    label = v13;
    packageHash = ...;
    packageSize = 11773151;
} ) for key CODE_PUSH_FAILED_UPDATES

Root cause

The package metadata is built from the server response via NSJSONSerialization. JSON null is parsed as NSNull. When saveFailedUpdate: writes the dictionary to NSUserDefaults, _CFPrefsValidateValueForKey rejects NSNull (only NSString / NSNumber / NSDate / NSArray / NSDictionary / NSData are valid property-list types) and throws NSInvalidArgumentException. The exception is uncaught and aborts the process.

In the field this triggers for fields like assetDownloadUrl and bundleDiffBlobUrl, which the server emits as `null` when no asset/diff URL is registered for the failed package.

When it fires

`saveFailedUpdate:` is invoked from `-[CodePush init]` via `initializeUpdateAfterRestart` -> `rollbackPackage` (when a previous update was marked `isLoading=YES` and never confirmed via `notifyApplicationReady`). This runs during `RCTTurboModuleManager` instantiation, before any JS executes - so the app crashes on launch, every launch, until the corrupt state is cleared by an uninstall + reinstall.

Related history

This is a recurrence of microsoft/react-native-code-push#493 (fixed by PR #497) and #606. Those fixes only nil-checked the dictionary itself; they did not strip `NSNull` values from inside the dictionary, so the underlying bug was never fully addressed.

Fix

Two layers in `saveFailedUpdate:`:

  1. Sanitize the dictionary - copy `failedPackage` into an `NSMutableDictionary`, dropping any value that is `NSNull` (or `nil`). This handles the documented current crash (`assetDownloadUrl`, `bundleDiffBlobUrl`).
  2. Defensive `@try/@catch` - if a future server-response shape introduces a non-plist value the filter doesn't catch, log it and clear `CODE_PUSH_FAILED_UPDATES` so subsequent launches don't crash on every `initializeUpdateAfterRestart` run. Worst-case behavior degrades to "the broken update may be retried once" instead of "the app is bricked until reinstall".

The dictionary stored in `failedUpdates` is only consumed by `+isFailedHash:`, which reads `packageHash` (always an `NSString`, never null). Dropping the URL/diff fields has no functional impact.

Verification

Reproduced on iOS 26 / iPhone 13 mini against a deployment whose manifest contained `null` for `assetDownloadUrl` and `bundleDiffBlobUrl`. Before the fix: 100% crash on the second launch after a fresh install (first launch downloads the OTA, app is killed before `notifyApplicationReady`, second launch enters the rollback path and crashes in `saveFailedUpdate:`). After the fix: rollback completes cleanly and the app falls back to the binary bundle.

…fields

Filter NSNull from the failed package dictionary before storing it in
NSUserDefaults, and wrap the write in @try/@catch as a defensive guard.

The server may return null for optional metadata fields (e.g.
assetDownloadUrl, bundleDiffBlobUrl). NSJSONSerialization parses these
to NSNull, which is not a property-list type. Passing such a dictionary
to -[NSUserDefaults setObject:forKey:] raises NSInvalidArgumentException
("Attempt to insert non-property list object").

Because saveFailedUpdate: is invoked from -[CodePush init] via
initializeUpdateAfterRestart -> rollbackPackage, an uncaught throw here
aborts the process during native module instantiation - i.e. the app
crashes on every launch until the corrupt state is cleared by uninstall
+ reinstall.

The dictionary stored in failedUpdates is only consumed by
+isFailedHash:, which reads the packageHash field (always an NSString),
so dropping the URL/diff fields has no functional impact.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant