fix(android): content-based (CRC32) resource matching for AAB installs + openRawResource density fix#577
Conversation
A from-package (PATCH_FROM_APK) hot update copies unchanged resources out of the on-device package using the path recorded in __diff.json `copies`. When the baseline uploaded to the server was an APK but the app is installed from an AAB (Play split APKs), res/ drawable paths are shortened on device, so the recorded path (e.g. res/drawable-xhdpi-v4/x.webp) does not exist verbatim and images (webp) silently fall through and go missing. Add a CRC32 content-match tier in BundledResourceCopier: build a crc32 -> entry index while scanning the base + split APKs, and when a `from` path is not found by exact/normalized path, locate the file by the content checksum supplied via the new manifest `copiesCrc` map. CRC32 is over the uncompressed content, so it is stable across APK/AAB packaging. This tier runs before resolveBundledResource (content match is more reliable than the resource-id heuristic). Also fix openResolvedResourceStream: use openRawResource(id, typedValue) with the density-resolved TypedValue instead of openRawResource(id), which ignored the requested density and could copy the wrong variant. Backward/forward compatible: manifests without `copiesCrc` (older CLI) simply skip the CRC tier and fall back to today's path-based behavior. Requires CLI support: reactnativecn/react-native-update-cli#54 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThreads CRC32 values from the patch manifest into APK patching and extends BundledResourceCopier.copyFromResource to build a CRC32→ZipEntry index and use CRC matches (with a recorded matchedZipFile) to resolve and extract bundled resources when path lookup fails. Adds comments to openResolvedResourceStream. ChangesCRC-based resource resolution
Sequence DiagramsequenceDiagram
participant DownloadTask
participant PatchArchiveContents
participant BundledResourceCopier
participant SafeZipFile
DownloadTask->>PatchArchiveContents: parse __diff.json -> populate copyCrcs
DownloadTask->>BundledResourceCopier: copyFromResource(copyList, copyCrcs)
BundledResourceCopier->>SafeZipFile: scan archives -> read ZipEntry CRCs
BundledResourceCopier->>BundledResourceCopier: build crc32 -> ZipSource index
BundledResourceCopier->>SafeZipFile: extract using matchedZipFile or entryToZipFileMap
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@android/src/main/java/cn/reactnative/modules/update/BundledResourceCopier.java`:
- Around line 42-45: The CRC index currently records only entry names which can
mismatch when multiple APKs expose the same name; update the CRC map used in
copyFromResource to store the matched ZipEntry and its corresponding SafeZipFile
(or a small holder object) instead of just the entry name, populate that map
wherever crcToEntryName is set (the places noted around lines ~50, ~68, ~107,
~135) and change the lookup paths in copyFromResource to retrieve and use the
stored ZipEntry+SafeZipFile directly when performing the CRC fallback unzip/copy
so you always read from the exact archive that matched the CRC.
- Around line 29-34: The code currently calls
resources.openRawResource(resourceId, typedValue) which re-resolves and
overwrites the density-resolved TypedValue; instead use the already-resolved
TypedValue to open the asset directly (e.g., use
resources.getAssets().openNonAsset(typedValue.assetCookie,
typedValue.string.toString()) or openNonAssetFd(...) as appropriate) so the
density-selected asset is preserved—replace calls to openRawResource(resourceId,
typedValue) in the BundledResourceCopier (look for usages in
ResolvedResourceSource and the methods that open streams/copy resources) with
direct asset opening using typedValue.assetCookie and typedValue.string
(guarding for null string and falling back if necessary), and apply the same
change to the other occurrences called out in the review.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b0257c3-dfc4-454b-95b8-b42e0fb87290
📒 Files selected for processing (2)
android/src/main/java/cn/reactnative/modules/update/BundledResourceCopier.javaandroid/src/main/java/cn/reactnative/modules/update/DownloadTask.java
| final TypedValue typedValue; | ||
|
|
||
| ResolvedResourceSource(int resourceId, String assetPath) { | ||
| ResolvedResourceSource(int resourceId, String assetPath, TypedValue typedValue) { | ||
| this.resourceId = resourceId; | ||
| this.assetPath = assetPath; | ||
| this.typedValue = typedValue; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the referenced lines to ensure the final-state snippet matches
FILE="android/src/main/java/cn/reactnative/modules/update/BundledResourceCopier.java"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
nl -ba "$FILE" | sed -n '1,120p'
echo "----"
nl -ba "$FILE" | sed -n '120,220p'
echo "----"
nl -ba "$FILE" | sed -n '220,340p'Repository: reactnativecn/react-native-update
Length of output: 121
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find occurrences of openRawResource(resourceId, typedValue) to confirm other line ranges
rg -n "openRawResource\s*\(\s*source\.resourceId\s*,\s*source\.typedValue\s*\)" -S android/src/main/java/cn/reactnative/modules/update/BundledResourceCopier.java
rg -n "openRawResource\s*\(" -S android/src/main/java/cn/reactnative/modules/update/BundledResourceCopier.javaRepository: reactnativecn/react-native-update
Length of output: 361
🌐 Web query:
AOSP ResourcesImpl openRawResource(int id, TypedValue value) calls getValue(id, value, true) and overwrites TypedValue
💡 Result:
In AOSP, ResourcesImpl.openRawResource(int id, TypedValue value) does call getValue(id, value, true) to fill the provided TypedValue with the resolved raw asset information (including overwriting its fields). This happens because openRawResource directly passes the same TypedValue instance as the outValue parameter to getValue, which writes into that outValue. Specifically, ResourcesImpl.openRawResource calls: - getValue(id, value, true); and then uses value.assetCookie and value.string to open the asset stream [1]. And ResourcesImpl.getValue is implemented as: - void getValue(... TypedValue outValue, boolean resolveRefs) { boolean found = mAssets.getResourceValue(id, 0, outValue, resolveRefs);... } [1]. Because outValue is used as the target object for mAssets.getResourceValue(..., outValue,...) and openRawResource passes the same TypedValue object, the TypedValue passed to openRawResource is overwritten (filled/resolved) by getValue before being used [1].
Citations:
🌐 Web query:
ResourcesImpl openRawResource TypedValue assetCookie open stream from resolved asset cookie path openNonAsset usage
💡 Result:
In the Android framework, the assetCookie is an integer identifier used by the AssetManager to represent a specific APK or asset package where a resource originates [1][2]. When opening resources, this cookie allows the system to correctly locate and load the file from the appropriate package [2]. The interaction between ResourcesImpl, TypedValue, and AssetManager works as follows: 1. TypedValue and assetCookie: When you request a resource (such as via Resources.openRawResource), the system first resolves the resource ID into a TypedValue object [3][4]. If the resource is a file-based resource (like a layout, drawable, or raw file), the TypedValue is populated with its string path and the assetCookie, which indicates which specific APK/resource package contains the file [1]. 2. ResourcesImpl.openRawResource: The ResourcesImpl class uses the resolved TypedValue.assetCookie to call mAssets.openNonAsset or mAssets.openNonAssetFd [3][5]. The assetCookie serves as an index into the AssetManager's list of loaded ApkAssets (which start at index 1) [2]. 3. AssetManager.openNonAsset: This is a lower-level API that opens a non-asset file (i.e., any file within an APK that is not in the dedicated 'assets' folder) [6][7]. It takes the assetCookie and the file path to stream or mmap the resource data [2][7]. Because this is an internal/low-level API, it is generally not intended for direct use by applications [6][7]. In summary, the assetCookie is essential for internal resource resolution, ensuring that the system retrieves the correct file from the correct APK package when multiple packages are present [2]. Direct usage of openNonAsset is typically restricted and discouraged for standard application development, as developers should rely on the higher-level Resources API [2][6].
Citations:
- 1: https://android.googlesource.com/platform/frameworks/base/+/refs/heads/main/core/java/android/util/TypedValue.java
- 2: https://android.googlesource.com/platform/frameworks/base/+/refs/heads/main/core/java/android/content/res/AssetManager.java?autodive=0%2F%2F%2F%2F%2F%2F%2F%2F%2F
- 3: https://android.googlesource.com/platform/frameworks/base/+/refs/heads/main/core/java/android/content/res/ResourcesImpl.java
- 4: https://android.googlesource.com/platform/frameworks/base/+/53e083c90d84fff53f30379a753612ac5d79bbdf/core/java/android/content/res/Resources.java
- 5: https://android.googlesource.com/platform/frameworks/base/+/android-7.0.0_r33/core/java/android/content/res/ResourcesImpl.java
- 6: https://developer.android.com/reference/kotlin/android/content/res/AssetManager
- 7: https://android.googlesource.com/platform/frameworks/base/+/refs/heads/master/core/java/android/content/res/AssetManager.java
openRawResource(resourceId, typedValue) re-resolves and overwrites the density-resolved TypedValue, so the density fix is a no-op.
AOSP’s ResourcesImpl.openRawResource(int, TypedValue) calls getValue(id, value, true), which writes into the same TypedValue instance using the current configuration before opening the stream. That overwrites the TypedValue previously populated by getValueForDensity(...), so the requested density variant isn’t preserved. (android.googlesource.com)
Use the already-resolved typedValue (e.g., assetCookie + path) to open the stream instead of passing it back through openRawResource(...).
<details>
<summary>Suggested direction</summary>
+import android.content.res.AssetManager;
@@
- return context.getResources().openRawResource(source.resourceId, source.typedValue);
+ return context.getAssets().openNonAsset(
+ source.typedValue.assetCookie,
+ source.typedValue.string.toString(),
+ AssetManager.ACCESS_STREAMING
+ );</details>
Also applies to: 274-275, 279-282
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@android/src/main/java/cn/reactnative/modules/update/BundledResourceCopier.java`
around lines 29 - 34, The code currently calls
resources.openRawResource(resourceId, typedValue) which re-resolves and
overwrites the density-resolved TypedValue; instead use the already-resolved
TypedValue to open the asset directly (e.g., use
resources.getAssets().openNonAsset(typedValue.assetCookie,
typedValue.string.toString()) or openNonAssetFd(...) as appropriate) so the
density-selected asset is preserved—replace calls to openRawResource(resourceId,
typedValue) in the BundledResourceCopier (look for usages in
ResolvedResourceSource and the methods that open streams/copy resources) with
direct asset opening using typedValue.assetCookie and typedValue.string
(guarding for null string and falling back if necessary), and apply the same
change to the other occurrences called out in the review.
…ct copy Two follow-ups from review on the resource copier: 1. CRC index stored only the entry name, so resolving it back through availableEntries could yield a different-content entry when two APKs (base + split) expose the same name with different bytes. Store the matched ZipEntry together with its SafeZipFile (ZipSource) and copy from that archive directly. 2. openRawResource(id, typedValue) is a no-op for density: AOSP's ResourcesImpl re-runs getValue(id, value, true) and overwrites the passed TypedValue at the current configuration before opening the stream. Instead of going through openRawResource (or the hidden openNonAsset API), take the density-correct file path resolved by getValueForDensity and copy that exact entry from the already-open archives — public API, correct variant. openRawResource(id) remains only as a defensive fallback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @coderabbitai — both findings are valid and addressed in f6849fe: 1. CRC index stored only the entry name (could copy wrong bytes on cross-APK name collision). 2. |
|
Tip For best results, initiate chat on the files or code changes.
Great work on the deep dive! 🐇✨ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
1 participant
Problem
热更新「上传 APK 基线 + 设备运行 AAB(Google Play 拆分包)」组合下,WebP 图片加载不出来(其它三种组合 OK)。
PATCH_FROM_APK路径下,未变更资源是按__diff.json的copies里记录的路径从设备包里复制出来的(资源复制全在BundledResourceCopier,native 侧只打 JS bundle 补丁)。但当基线是 APK、设备装的是 AAB 拆分包时,res/drawable 路径在设备上被资源路径压缩(res/drawable-xhdpi-v4/x.webp→res/xY.webp),记录的全名路径不存在 → 精确/归一化匹配落空 →resolveBundledResource对这些 webp 兜不住 → 静默continue→ 图片缺失。assets/路径稳定,故只有图片受影响。Change
1. CRC32 内容兜底层(
BundledResourceCopier)扫描 base + split APK 时顺带建
crc32 -> 条目名索引;当from路径按精确/归一化都找不到时,用新清单字段copiesCrc提供的内容校验和按内容定位文件。CRC32 算的是解压后内容,跨 APK/AAB 打包稳定。该层排在resolveBundledResource之前(内容匹配比资源 ID 反查更可靠)。2. openRawResource 密度修复
resolveBundledResource已用getValueForDensity解析出正确密度的TypedValue,但openResolvedResourceStream之前用单参openRawResource(id)——会忽略请求的密度、退回设备当前密度,可能复制错变体。改为双参openRawResource(id, typedValue)。兼容性
旧 CLI 生成的清单没有
copiesCrc→ CRC 层自动空转,退回当前路径匹配行为,无回归。新增形参全部可选/向后兼容。依赖
需配合 CLI 下发
copiesCrc:reactnativecn/react-native-update-cli#54验证
UpdateContext.DEBUG,logcat 不再出现Skipped N missing bundled resources,WebP 正常显示;回归 case 1/2/3。🤖 Generated with Claude Code
Summary by CodeRabbit