Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,32 @@ final class BundledResourceCopier {
private static final class ResolvedResourceSource {
final int resourceId;
final String assetPath;
final TypedValue typedValue;

ResolvedResourceSource(int resourceId, String assetPath) {
ResolvedResourceSource(int resourceId, String assetPath, TypedValue typedValue) {
this.resourceId = resourceId;
this.assetPath = assetPath;
this.typedValue = typedValue;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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.java

Repository: 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:


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.

}
}

BundledResourceCopier(Context context) {
this.context = context.getApplicationContext();
}

void copyFromResource(HashMap<String, ArrayList<File>> resToCopy) throws IOException {
void copyFromResource(
HashMap<String, ArrayList<File>> resToCopy,
HashMap<String, Long> crcByFrom
) throws IOException {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
ArrayList<String> apkPaths = collectApkPaths();
HashMap<String, ZipEntry> availableEntries = new HashMap<String, ZipEntry>();
HashMap<String, SafeZipFile> zipFileMap = new HashMap<String, SafeZipFile>();
HashMap<String, SafeZipFile> entryToZipFileMap = new HashMap<String, SafeZipFile>();
// Content checksum index: CRC32 -> entry name. Lets us locate a file by
// content when its origin path is not present verbatim on device (e.g.
// APK baseline diff applied on an AAB/split-apk install whose res/
// paths were shortened). First entry for a given crc wins.
HashMap<Long, String> crcToEntryName = new HashMap<Long, String>();

try {
for (String apkPath : apkPaths) {
Expand All @@ -55,6 +65,10 @@ void copyFromResource(HashMap<String, ArrayList<File>> resToCopy) throws IOExcep
availableEntries.put(entryName, ze);
entryToZipFileMap.put(entryName, zipFile);
}
long crc = ze.getCrc();
if (crc != -1L && !crcToEntryName.containsKey(crc)) {
crcToEntryName.put(crc, entryName);
}
}
}

Expand Down Expand Up @@ -87,6 +101,20 @@ void copyFromResource(HashMap<String, ArrayList<File>> resToCopy) throws IOExcep
}
}

// Content (CRC32) match: robust across APK/AAB packaging because
// the checksum is over the uncompressed file content, not its
// path. Preferred over the resource-id heuristic below.
if (entry == null && crcByFrom != null) {
Long wantedCrc = crcByFrom.get(fromPath);
if (wantedCrc != null) {
String matchedEntry = crcToEntryName.get(wantedCrc);
if (matchedEntry != null) {
entry = availableEntries.get(matchedEntry);
actualSourcePath = matchedEntry;
}
}
}

if (entry == null) {
resolvedResource = resolveBundledResource(fromPath);
if (resolvedResource != null) {
Expand Down Expand Up @@ -243,12 +271,15 @@ private ResolvedResourceSource resolveBundledResource(String resourcePath) {
assetPath = assetPath.substring(1);
}

return new ResolvedResourceSource(resourceId, assetPath);
return new ResolvedResourceSource(resourceId, assetPath, typedValue);
}

private InputStream openResolvedResourceStream(ResolvedResourceSource source) throws IOException {
try {
return context.getResources().openRawResource(source.resourceId);
// Use the density-resolved TypedValue so we open the exact variant
// that was requested, instead of openRawResource(id) which would
// fall back to the device's current configuration density.
return context.getResources().openRawResource(source.resourceId, source.typedValue);
} catch (Resources.NotFoundException e) {
throw new IOException("Unable to open resolved resource: " + source.assetPath, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ private static final class PatchArchiveContents {
final ArrayList<String> copyFroms = new ArrayList<String>();
final ArrayList<String> copyTos = new ArrayList<String>();
final ArrayList<String> deletes = new ArrayList<String>();
// Maps a copy source path ("from") to the CRC32 of the file content,
// when provided by the manifest ("copiesCrc"). Lets the resource
// copier locate the file by content if the path is not present on
// device (APK baseline -> AAB install path shortening).
final HashMap<String, Long> copyCrcs = new HashMap<String, Long>();
}

private final Context context;
Expand Down Expand Up @@ -140,8 +145,11 @@ private void appendManifestEntries(
JSONObject manifest,
ArrayList<String> copyFroms,
ArrayList<String> copyTos,
ArrayList<String> deletes
ArrayList<String> deletes,
HashMap<String, Long> copyCrcs
) throws JSONException {
JSONObject copiesCrc = manifest.optJSONObject("copiesCrc");

JSONObject copies = manifest.optJSONObject("copies");
if (copies != null) {
Iterator<?> keys = copies.keys();
Expand All @@ -153,6 +161,11 @@ private void appendManifestEntries(
}
copyFroms.add(from);
copyTos.add(to);
if (copiesCrc != null && copyCrcs != null && copiesCrc.has(to)) {
// Same content => same crc, so grouping multiple "to" under
// one "from" stays consistent.
copyCrcs.put(from, copiesCrc.getLong(to));
}
}
}

Expand Down Expand Up @@ -220,7 +233,8 @@ private PatchArchiveContents extractPatchArchive(File archiveFile, File unzipDir
manifest,
contents.copyFroms,
contents.copyTos,
contents.deletes
contents.deletes,
contents.copyCrcs
);
continue;
}
Expand Down Expand Up @@ -285,7 +299,7 @@ private void doPatchFromApk() throws IOException, JSONException {
originBundleFile.delete();
}

bundledResourceCopier.copyFromResource(copyList);
bundledResourceCopier.copyFromResource(copyList, contents.copyCrcs);
}

private void doPatchFromPpk() throws IOException, JSONException {
Expand Down
Loading