Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
samholmes
force-pushed
the
sam/harden-no-https
branch
from
7d46ab0 to
b1224f5
Compare
android/src/main/java/app/edge/reactnative/core/EdgeCoreWebView.java
Outdated
Show resolved
Hide resolved
android/src/main/java/app/edge/reactnative/core/EdgeCoreWebView.java
Outdated
Show resolved
Hide resolved
android/src/main/java/app/edge/reactnative/core/EdgeCoreWebView.java
Outdated
Show resolved
Hide resolved
android/src/main/java/app/edge/reactnative/core/EdgeCoreWebView.java
Outdated
Show resolved
Hide resolved
ios/EdgeAssetsSchemeHandler.swift
Outdated
| } | ||
| } | ||
| } else { | ||
| // Core file - look in edge-core-js.bundle |
There was a problem hiding this comment.
Similar to Android - can we just pass the right URL to begin with, and avoid this auto-lookup logic?
Maybe change <script charset="utf-8" defer src="/edge-core.js"></script> to not have the /, thereby making the path relative to index.html? Then we can make edgebundle://edge.bundle/edge-core-js.bundle/index.html the default URL on iOS.
We might also consider exporting a constant up to JavaScript, similar to how edge-currency-accountbased does it. See @objc func constantsToExport() on accountbased iOS and the public Map<String, Object> getConstants() on Android. Then JS doesn't need to know where stuff is located - the constants will tell us how to smush paths correctly. For instance, we might export a legacyPrefix and bundlePrefix constant, so we can just do if (url.startsWith(legacyPrefix) return url.replace(legacyPrefix, bundlePrefix) in JavaScript, thereby simplifying things a bunch.
samholmes
force-pushed
the
sam/harden-no-https
branch
2 times, most recently
from
a1be93e to
3804709
Compare
android/src/main/java/app/edge/reactnative/core/EdgeCorePackage.java
Outdated
Show resolved
Hide resolved
android/src/main/java/app/edge/reactnative/core/EdgeCoreWebView.java
Outdated
Show resolved
Hide resolved
| }, | ||
| forceTls: true, // force WSS | ||
| extra: {} | ||
| forceTls: true // force WSS |
There was a problem hiding this comment.
Unrelated NYM configuration removal changes network behavior
Medium Severity
The PR removes preferredGateway, preferredNetworkRequester, and mixFetchOverride.requestTimeoutMs (60 second timeout) from the NYM configuration. These changes are unrelated to the PR's stated purpose of refactoring the bundler to use platform-native asset loaders. The timeout removal in particular could cause privacy-mode fetch requests to fail with shorter default timeouts.
There was a problem hiding this comment.
This was intentional. We want to be as near to default behavior as is observed to be working.
swansontec
left a comment
swansontec
left a comment
There was a problem hiding this comment.
Approving, because we need to get this shipped. But there is on more block of dead code I found, which is probably harmless for now.
ios/EdgeAssetsSchemeHandler.swift
Outdated
| print("EdgeAssetsSchemeHandler: Error reading file at \(fullPath): \(error)") | ||
| } | ||
| } | ||
| } else { |
There was a problem hiding this comment.
We should be able to delete this else block as well.
Android: Use custom WebViewClient to serve assets via https://edge.bundle - Eliminates local HTTP server and TLS certificate management - Assets served directly from app bundle without network stack - COOP/COEP headers added via LocalContentWebViewClient iOS: Use WKURLSchemeHandler to serve assets via edgebundle://edge.bundle - Eliminates local HTTP server and TLS certificate management - Custom URL scheme provides non-null origin for same-origin policy - COOP/COEP headers added via HTTPURLResponse Both platforms: - SharedArrayBuffer support maintained via cross-origin isolation headers - Split handler classes into separate files (LocalContentWebViewClient.java, EdgeAssetsSchemeHandler.swift)
samholmes
force-pushed
the
sam/harden-no-https
branch
from
31f0741 to
c9169c5
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
iOS loadFile else branch is unreachable dead code
Low Severity
The else branch in loadFile is dead code. On iOS, DEFAULT_SOURCE resolves to edgebundle://edge.bundle/edge-core-js.bundle/index.html, so all core file paths contain .bundle/ and take the if branch. Relative resources (like edge-core.js) resolve under the .bundle/ directory too, and plugin paths either contain .bundle/ or start with edge-core/. No normal request can reach the else branch. The reviewer also flagged this block for deletion.
2 participants
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
noneNote
Medium Risk
Touches core WebView asset loading and plugin URI handling on both iOS and Android, which could regress boot/plugin loading or cross-origin isolation behavior if any paths/headers differ across OS versions.
Overview
Refactors React Native bundling to stop running a loopback
BundleHTTPServerand instead serveedge-core-jsassets via platform-native mechanisms while preserving COOP/COEP headers needed forSharedArrayBuffer.On Android,
EdgeCoreWebViewnow blocksfile://access and interceptshttps://edge.bundle/...requests viaLocalContentWebViewClient, serving assets directly fromandroid_assetwith COOP/COEP (+ CORS) headers; a newEdgeCoreModuleexportsbundleBaseUri/rootBaseUriconstants to JS.On iOS, the HTTP server is removed and
WKURLSchemeHandler(EdgeAssetsSchemeHandler) servesedgebundle://edge.bundle/...requests (including plugin files) with the same headers; the podspec is updated to include the new files.JS is updated to normalize plugin URIs using the native constants (removing
/pluginproxying and direct URI conversion in the worker), adjustindex.htmlscript pathing, and simplify webpack devServer by dropping the/pluginproxy;mixFetchOptionsis also trimmed toforceTlsonly.Written by Cursor Bugbot for commit c9169c5. This will update automatically on new commits. Configure here.