fix(bundle-update): resume downloads, per-failure SHA256 subtypes, and path-leak hardening#53
Open
huhuanming wants to merge 13 commits intomainfrom
Open
fix(bundle-update): resume downloads, per-failure SHA256 subtypes, and path-leak hardening#53huhuanming wants to merge 13 commits intomainfrom
huhuanming wants to merge 13 commits intomainfrom
Conversation
iOS — persist URLSession resumeData on failure
- DownloadDelegate captures NSURLSessionDownloadTaskResumeData from the
failed task's userInfo; the previous code threw it away even though
iOS attaches ~11KB of partial-transfer state on every NSURL -1005 /
-1001 (~5,940 mixpanel users / 64% of all download failures).
- downloadBundle persists the blob to <filePath>.resume on error, then
on the next call passes it to session.downloadTask(withResumeData:)
so the OS finishes from the cut point instead of byte 0.
- Outcome wrapped in a Result switch so the failure branch can persist
resumeData before re-throwing, and the success branch can unlink the
stale blob — keeping the .resume sidecar in lockstep with the bundle.
Android — Range-based resume via .partial sidecar
- Download streams to <filePath>.partial; rename to <filePath> only
after SHA256 verifies, mirroring the Desktop implementation. A
corrupt full file can no longer poison the "exists -> already valid"
fast path.
- On retry, .partial size becomes the Range: bytes=<offset>- header.
Server-side 206 with parsed Content-Range gives the true total; 200
on a Range request transparently restarts; 416 wipes the .partial
and bubbles up so the next attempt starts clean.
SHA256 failure subtypes (both platforms)
- ThreadLocal/Thread.threadDictionary stamp records FILE_NOT_FOUND /
FILE_EMPTY / FILE_DISAPPEARED / FILE_TRUNCATED / PERMISSION_DENIED /
OOM / IO_<class> / UNEXPECTED_<class>; verifyBundleSHA256 reads it
back so a hash-mismatch (reason==null) is distinguishable from a
computation failure.
- iOS replaces the raise-prone readData(ofLength:) with a throwing
read(upToCount:) wrapper so disk I/O failures become catchable Swift
errors instead of NSException surface.
- Download flow attaches the subtype to its update/error event payload
("SHA256_FILE_TRUNCATED", "SHA256_OOM", …) so the previously opaque
91.3% Android verifyPackage bucket can be split in mixpanel.
… prevent path leakage
Two thrown messages embedded the lower-level exception's text directly:
iOS verifyBundleASC (unzip catch)
Before: throw NSError(... "Failed to unzip bundle: \(error.localizedDescription)")
After: throw NSError(... "Failed to unzip bundle: IO_\(nsErr.code)")
SSZipArchive's localizedDescription frequently embeds the failing
file path which, on a real iOS device, contains the install UUID
under /var/mobile/Containers/Data/Application/<UUID>/. The full
description still flows to OneKeyLog (local-only) for support
diagnostics; only an IO_<NSError code> tag escapes into the
Promise rejection that JS analytics observes.
Android getMetadata (JSON parse catch)
Before: throw Exception("Failed to parse metadata.json: ${e.message}")
After: throw Exception("Failed to parse metadata.json: IO_<class>")
org.json's parse-failure messages can carry partial JSON contents
or local file paths from the underlying reader. Same split: rich
detail in OneKeyLog, IO_<class> tag in the thrown message that
becomes errorMessage upstream.
Both new shapes (`IO_<code>` / `IO_<class>`) match the SHA256_<reason>
convention introduced in the prior commit so extractUpdateErrorCode
in hooks.tsx classifies them into the same `IO_*` mixpanel bucket
without a separate parser.
The companion sanitizer in UpdateReminder/hooks.tsx
(sanitizeUpdateErrorMessage) provides a redaction safety net that
would have caught these too, but trimming at the source means we
never have to count on the safety net for the codepaths we own.
iOS force-quit (user swipes the app off the App Switcher) cannot fire URLSession's didCompleteWithError — SIGKILL leaves no time for callbacks, so the resume blob never reaches disk. The kill is, however, *always* preceded by the app transitioning to the background. Memory-pressure kills follow the same chain because iOS only reaps backgrounded apps under memory pressure. So a hook on UIApplication.didEnterBackground- Notification gives us the only deterministic snapshot point before termination. Implementation: - ReactNativeBundleUpdate.init registers an observer for didEnterBackgroundNotification (paired removal in deinit). On fire: beginBackgroundTask extends the ~5s of guaranteed background runtime to ~30s; we walk URLSession.getAllTasks and call cancel(byProducingResumeData:) on every running download task. The cancel callback hands us the resume blob synchronously, so even if iOS suspends right after endBackgroundTask the blob is already at <filePath>.resume on disk. - A new private activeDownloadFilePath property anchors the snapshot to the right .resume sidecar; downloadBundle sets it BEFORE task.resume() (so a foreground→background race on the very first bytes still has a path) and clears it in defer (so a stray didEnterBackground after completion doesn't try to cancel a finished task). - Snapshot guards on isDownloading + activeDownloadFilePath, so observers fired outside an active download are no-ops. Companion change in app-monorepo's UpdateReminder/hooks.tsx (C2 / C1) will retrigger downloadPackage on AppState.active so the next call picks up the freshly snapshotted blob via downloadTask(withResumeData:). Android and Desktop don't need an equivalent hook: they write incrementally to <filePath>.partial via FileOutputStream / WriteStream on each chunk, so SIGKILL only loses the last <8KB OS pagecache buffer. The next attempt's Range: bytes=<offset>- finds whatever has been flushed.
…l recovery, verify-stage SHA reason)
Audit (Codex) on the prior commits surfaced four native-side issues.
iOS — protect activeDownloadFilePath under stateQueue
Previously isDownloading was read inside the stateQueue but
activeDownloadFilePath was read outside it, so a foreground/background
race could observe a torn state (isDownloading=true with a stale or
nil filePath, or vice versa). Read both inside the same sync block,
and pair the writes (set on entry, clear in defer) with the same
queue. Mirrors the protection level isDownloading already had.
iOS + Android — verify-stage SHA failures now propagate the subtype
verifyBundleASC's SHA256 guard threw "Bundle signature verification
failed" — opaque to extractUpdateErrorCode, which collapsed every
verify-stage SHA failure into the same Mixpanel bucket. Read the
side-channel reason (FILE_TRUNCATED / OOM / IO_<class> / MISMATCH)
and throw the same shape the download stage uses
("Bundle SHA256 verification failed: <reason>") so the JS extractor
splits the bucket end-to-end.
Android — recover crashed-before-rename .partial via promote+verify
If the JVM was killed between the last byte being written to .partial
and the rename to the final filename, the previous code unconditionally
deleted the partial when its size hit expectedSize (forcing a full
re-download of an already-complete bundle). Now: when partialSize ==
expectedSize, attempt rename + SHA verify first; if it passes, treat
the bundle as complete and skip the download entirely. Only deletes
on verify failure.
Android — recover from 416 when Content-Range says we have the full file
HTTP 416 with `Content-Range: bytes */<total>` where total ==
partialBytes means our local partial IS the complete bundle (server's
Range request was correctly rejected because we'd want byte N+1 of an
N-byte file). Parse the header, attempt promote+verify before falling
through to the wipe branch; otherwise behavior is unchanged.
Companion app-monorepo audit fixes (privacy + concurrency) land in
the wallet repo.
…ate iOS URLSession - Android: route exception messages through sanitizeErrorMessageForEvent before emitting update/error, so FileNotFoundException / IOException payloads never leak /data/user/<u>/<pkg>/... paths to JS listeners or downstream analytics. Known low-cardinality reasons (SHA256 verify, HTTP <code>, guard messages) are preserved verbatim; everything else collapses to IO_<exceptionClassName>. - iOS: call urlSession?.invalidateAndCancel() in deinit. URLSession retains its delegate strongly until invalidated, so without this the session and its DownloadDelegate would outlive the module — relevant on dev hot-reload and any future test harness with multiple module instances.
There was a problem hiding this comment.
Pull request overview
Hardens the react-native-bundle-update native module by improving download resilience (resume support) and making error reporting lower-cardinality and safer to surface to JS/analytics, plus some lifecycle cleanup.
Changes:
- iOS: Track SHA256 calculation failure subtypes; persist
URLSessionresume data on backgrounding; invalidate theURLSessionon module deinit. - Android: Add resumable downloads via
.partialfiles +Rangerequests; add per-failure SHA256 reason subtypes; sanitize error messages emitted viaupdate/error.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| native-modules/react-native-bundle-update/ios/ReactNativeBundleUpdate.swift | Adds SHA256 failure subtyping, resume-data snapshotting on background, and URLSession invalidation on teardown. |
| native-modules/react-native-bundle-update/android/src/main/java/com/margelo/nitro/reactnativebundleupdate/ReactNativeBundleUpdate.kt | Adds resumable download implementation, SHA256 failure reasons, and path-safe error event payload shaping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
originalix
reviewed
May 9, 2026
originalix
reviewed
May 9, 2026
- Allow legitimate 0-byte files in calculateSHA256 on both platforms
(validateAllFilesInDir / validateWebEmbedSha256 / launch entry verify
share this calculator; rejecting empty files broke valid bundles).
- iOS: keep IO_<code> failure tag low-cardinality to match doc and
the analytics bucket cap (drop NSError domain from the tag; full
detail still goes to OneKeyLog).
- iOS: serialize bgTaskId in snapshotResumeDataForBackgrounding via
a small holder + dedicated queue so the expiration handler,
getAllTasks completion (off-main), and group.notify cannot
double-end or leak the background task.
- iOS: correct doc on background snapshot — getAllTasks and
cancel(byProducingResumeData:) deliver via async closures, not
synchronously; the begin/endBackgroundTask window is what gives
the closures time to run before suspension.
- Android: emit update/error from the outer catch only; remove the
pre-emit at HTTP 416 / non-2xx HTTP / rename failure / SHA256
verify failure paths to avoid double-fired events with
divergent payload shapes.
- Android: drop the \`\${e.javaClass.simpleName}: \` prefix from the
emitted update/error message so the verbatim payloads documented
on sanitizeErrorMessageForEvent (HTTP / SHA256 / allowlist
strings) actually arrive verbatim at JS listeners.
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
- CHANGELOG: align version to 3.0.30 (was 3.0.29), correct cancel(byProducingResumeData:) wording from synchronous to asynchronous-via-closure, drop FILE_EMPTY from the SHA256 taxonomy (no code path emits it after the empty-file early return was removed), and split the per-platform reason lists. - iOS: rewrite the snapshotResumeDataForBackgrounding ordering comment to match the actual sequence (isDownloading flips before activeDownloadFilePath is written) and call out the defer-unwind race; on download failure rethrow a sanitized NSError carrying only "<domain> <code>" so URLSession localizedDescription cannot re-leak temp file paths through the Promise rejection — full detail still goes to OneKeyLog. - Android: extend lastSHA256FailureReason() doc to include FILE_DISAPPEARED / PERMISSION_DENIED (already emitted by calculateSHA256); rewrap the downloadBundle catch as Exception(sanitized, e) so the Promise rejection message matches the sanitized update/error payload (verbatim allowlist preserved, everything else collapses to IO_<class>) instead of leaking /data/user/<u>/<pkg>/ paths via FileNotFoundException.message. Original exception is attached as cause so crash reporters keep the full chain.
- CHANGELOG: align release header and chores line to 3.0.31 (matches the package.json bump on this PR). - iOS lastSHA256FailureReason() doc: drop UNEXPECTED from the taxonomy — calculateSHA256 only emits FILE_NOT_FOUND / FILE_DISAPPEARED / IO_<NSError code>; called out the iOS vs. Android reason-set split so the doc stays in sync. - iOS calculateSHA256 read comment: drop the misleading "NSException bridge" wording. read(upToCount:) on iOS 13.4+ throws Swift NSError (catchable here); the pre-13.4 fallback uses readData(ofLength:) which raises NSException — Swift cannot catch that, so legacy OSes will still abort. - Android downloadBundle: close the OkHttp response before throwing on a null body. Without the explicit close, the response (which holds the connection) leaked because the byteStream() consumption path that would otherwise return the connection to the pool is skipped by the throw.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch hardens the
react-native-bundle-updatenative module on both platforms. Highlights across the 5 commits:feat(bundle-update): support resuming partial downloads and split SHA256 failures into actionable subtypes so JS listeners can distinguish corruption vs. mismatch vs. verification-stage failures.feat(bundle-update-ios): persist resumeData when the app backgrounds so an interrupted download can be picked up after relaunch.fix(bundle-update): stop embedding inner exception text in thrown messages so/data/user/<u>/<pkg>/...style paths don't leak through error propagation.fix(bundle-update): queue synchronization, partial-state recovery, and a clearer SHA reason at the verify stage.fix(bundle-update)(this commit):sanitizeErrorMessageForEventbefore emittingupdate/error, soFileNotFoundException/IOExceptionpayloads never carry/data/user/<u>/<pkg>/...paths to JS listeners or analytics. Known low-cardinality reasons (SHA256 verify,HTTP <code>, guard messages) are preserved verbatim; anything else collapses toIO_<exceptionClassName>.urlSession?.invalidateAndCancel()indeinitso the session and itsDownloadDelegatedon't outlive the module (matters on dev hot-reload and future multi-instance test harnesses).Test plan
FileNotFoundExceptionand confirm the JS-sideupdate/errorevent payload contains onlyIO_FileNotFoundException(no/data/user/...substring).Bundle SHA256 verification failed: <REASON>so the JS extractor can map toSHA256_<REASON>.HTTP 416 ...,Already downloading, and other guard messages are preserved verbatim.URLSessionis invalidated (no leaked delegate, no duplicate progress callbacks).