Open
Conversation
Note: this is a migration from the old PR due to stale downstream
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+29
to
+30
| val dispatcher = | ||
| Executors.newFixedThreadPool(64).asCoroutineDispatcher() |
Contributor
There was a problem hiding this comment.
64 threads may be excessive for parallel downloads - consider using a smaller pool (e.g., 16-32 threads) to avoid resource exhaustion on devices with limited capabilities
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: superwall/src/main/java/com/superwall/sdk/paywall/archive/ManifestDownloader.kt
Line: 29:30
Comment:
64 threads may be excessive for parallel downloads - consider using a smaller pool (e.g., 16-32 threads) to avoid resource exhaustion on devices with limited capabilities
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+71
to
+76
| archiveFile.content.find { part -> | ||
| if (url.contains("index.html")) { | ||
| part is ArchivePart.Document | ||
| } else { | ||
| part.url.contains(url) | ||
| } |
Contributor
There was a problem hiding this comment.
URL matching with .contains() is fragile - a URL like /about/index.html would incorrectly match when searching for /index.html. Use exact URL matching or path normalization.
Suggested change
| archiveFile.content.find { part -> | |
| if (url.contains("index.html")) { | |
| part is ArchivePart.Document | |
| } else { | |
| part.url.contains(url) | |
| } | |
| archiveFile.content.find { part -> | |
| if (url.contains("index.html")) { | |
| part is ArchivePart.Document | |
| } else { | |
| part.url == url || part.url.endsWith(url) | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: superwall/src/main/java/com/superwall/sdk/paywall/archive/ArchiveWebClient.kt
Line: 71:76
Comment:
URL matching with `.contains()` is fragile - a URL like `/about/index.html` would incorrectly match when searching for `/index.html`. Use exact URL matching or path normalization.
```suggestion
archiveFile.content.find { part ->
if (url.contains("index.html")) {
part is ArchivePart.Document
} else {
part.url == url || part.url.endsWith(url)
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+199
to
+203
| // Enable file access only for archive loading | ||
| settings.allowFileAccess = true | ||
| settings.allowFileAccessFromFileURLs = true | ||
| settings.allowUniversalAccessFromFileURLs = true | ||
| settings.allowContentAccess = true |
Contributor
There was a problem hiding this comment.
Enabling universal file access (allowFileAccessFromFileURLs, allowUniversalAccessFromFileURLs) creates security risks - malicious web content could potentially read local files. Ensure webarchive content is from trusted sources only.
Prompt To Fix With AI
This is a comment left during a code review.
Path: superwall/src/main/java/com/superwall/sdk/paywall/view/webview/SWWebView.kt
Line: 199:203
Comment:
Enabling universal file access (`allowFileAccessFromFileURLs`, `allowUniversalAccessFromFileURLs`) creates security risks - malicious web content could potentially read local files. Ensure webarchive content is from trusted sources only.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+36
to
+46
| override fun encode(content: ByteArray): String { | ||
| return if (content.size > 1) { | ||
| return try { | ||
| return Base64.encodeToString(content, Base64.CRLF) | ||
| } catch (e: Throwable) { | ||
| "ICAgIA==" | ||
| } | ||
| } else { | ||
| "ICAgIA==" | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Multiple redundant return statements - simplify logic
Suggested change
| override fun encode(content: ByteArray): String { | |
| return if (content.size > 1) { | |
| return try { | |
| return Base64.encodeToString(content, Base64.CRLF) | |
| } catch (e: Throwable) { | |
| "ICAgIA==" | |
| } | |
| } else { | |
| "ICAgIA==" | |
| } | |
| } | |
| override fun encode(content: ByteArray): String { | |
| return if (content.size > 1) { | |
| try { | |
| Base64.encodeToString(content, Base64.CRLF) | |
| } catch (e: Throwable) { | |
| "ICAgIA==" | |
| } | |
| } else { | |
| "ICAgIA==" | |
| } | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: superwall/src/main/java/com/superwall/sdk/paywall/archive/Base64ArchiveEncoder.kt
Line: 36:46
Comment:
Multiple redundant `return` statements - simplify logic
```suggestion
override fun encode(content: ByteArray): String {
return if (content.size > 1) {
try {
Base64.encodeToString(content, Base64.CRLF)
} catch (e: Throwable) {
"ICAgIA=="
}
} else {
"ICAgIA=="
}
}
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+124
to
+128
| if (relativeUrlsOnly.contains(it.url)) { | ||
| if (it.url.contains("favicon.ico")) { | ||
| "favicon.ico" | ||
| } | ||
| it.copy(url = it.url.removePrefix("https://${host.host}")) |
Contributor
There was a problem hiding this comment.
Missing return statement causes compilation error
Suggested change
| if (relativeUrlsOnly.contains(it.url)) { | |
| if (it.url.contains("favicon.ico")) { | |
| "favicon.ico" | |
| } | |
| it.copy(url = it.url.removePrefix("https://${host.host}")) | |
| if (relativeUrlsOnly.contains(it.url)) { | |
| if (it.url.contains("favicon.ico")) { | |
| it.copy(url = "favicon.ico") | |
| } else { | |
| it.copy(url = it.url.removePrefix("https://${host.host}")) | |
| } | |
| } else { | |
| it | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: superwall/src/main/java/com/superwall/sdk/paywall/archive/ManifestDownloader.kt
Line: 124:128
Comment:
Missing return statement causes compilation error
```suggestion
if (relativeUrlsOnly.contains(it.url)) {
if (it.url.contains("favicon.ico")) {
it.copy(url = "favicon.ico")
} else {
it.copy(url = it.url.removePrefix("https://${host.host}"))
}
} else {
it
}
```
How can I resolve this? If you propose a fix, please make it concise.
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
1 participant
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.
Note: this is a migration from the old PR due to stale downstream
Changes in this pull request
Checklist
CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes.ktlintin the main directory and fixed any issues.Greptile Overview
Greptile Summary
This PR implements webarchive support for instant paywall loading by downloading, compressing, and caching HTML paywalls with their dependencies as MHTML-style archives. The implementation includes a manifest-based resource discovery system that uses regex to find relative and absolute resources in HTML, downloads them in parallel using a 64-thread pool, and compresses them into multipart archives stored on disk. When a paywall is displayed, the SDK checks if a cached archive exists and loads it directly through a custom
WebViewClientthat intercepts requests and serves content from the decompressed archive.Key changes:
ManifestDownloaderwith regex-based resource discovery and parallel download systemStreamArchiveCompressorandStringArchiveCompressorfor MHTML-style compression/decompressionCachedArchiveLibraryto manage archive lifecycle with download queuePaywallViewandSWWebViewto conditionally load from archive with fallback to URL loadingStorageinterface for archive persistenceIssues found:
ManifestDownloader.kt:124-128- missing return statement in if-expression will cause compilation failureArchiveWebClient.kt:71-76- URL matching uses fragile.contains()logic that could match incorrect resourcesSWWebView.kt:199-203- enabling universal file access creates potential vulnerabilityConfidence Score: 2/5
ManifestDownloader.kt:124-128where an if-expression is missing a return statement. Additionally, the fragile URL matching logic inArchiveWebClientcould cause runtime failures when loading archived paywalls, and the security implications of enabling universal file access need careful review.ManifestDownloader.kt(syntax error),ArchiveWebClient.kt(URL matching logic), andSWWebView.kt(security permissions)Important Files Changed
.contains()logicSequence Diagram
sequenceDiagram participant SDK as Superwall SDK participant PM as PaywallManager participant PW as PaywallView participant AL as CachedArchiveLibrary participant MD as ManifestDownloader participant Net as Network/ArchiveService participant WV as SWWebView participant AC as ArchiveWebClient Note over SDK,PM: Paywall Preload Flow SDK->>PM: preloadAllPaywalls(config) PM->>AL: downloadManifest(paywallId, url, manifest) AL->>MD: downloadArchiveForManifest(id, manifest) MD->>Net: fetchRemoteFile(mainDocumentUrl) Net-->>MD: main HTML content MD->>MD: discoverRelativeResources(html) MD->>MD: discoverAbsoluteResources(html) par Parallel Downloads (64 threads) MD->>Net: fetchRemoteFile(resource1) MD->>Net: fetchRemoteFile(resource2) MD->>Net: fetchRemoteFile(resourceN) end Net-->>MD: all resources MD-->>AL: List<ArchivePart> AL->>AL: compressToStream(url, parts, fileStream) AL-->>PM: archive saved to disk Note over PW,WV: Paywall Display Flow SDK->>PM: getPaywallView(request) PM->>PW: create PaywallView PW->>AL: checkIfArchived(paywallId) alt Archive exists AL-->>PW: true PW->>AL: loadArchive(paywallId) AL-->>PW: DecompressedWebArchive PW->>WV: loadFromArchive(archive) WV->>AC: create ArchiveWebClient(archive) WV->>WV: enable file access permissions WV->>WV: loadUrl(OVERRIDE_PATH) WV->>AC: shouldInterceptRequest(url) AC->>AC: resolveUrlFromArchive(archive, url) AC-->>WV: WebResourceResponse(content) else No archive PW->>WV: setup(url) WV->>WV: loadUrl(paywall.url) end WV-->>PW: paywall rendered