From 4255ba334584d14decbb0d05d0e8eb742b5d46f4 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Mon, 20 Jul 2026 10:09:24 -0400 Subject: [PATCH] fix(browser): capture icons from sites that declare only an SVG favicon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pinned web app showed a generic placeholder instead of its icon — ditto.pub failed while brainstorm.nosfabrica.com worked. The discriminator is what each site declares: brainstorm: favicon.svg AND favicon.png <- the PNG is why it works ditto: logo.svg only (+ an apple-touch-icon nobody read) Icons were captured solely through `WebChromeClient.onReceivedIcon`, which hands back a rasterized Bitmap. Android WebView does not decode SVG favicons into that callback, so a site offering a raster alternative gets picked up and an SVG-only site never fires it at all: nothing is recorded, `iconModelFor()` returns null forever, and it fails silently — no error, no log line, just a permanently generic icon. SVG-only declarations are increasingly the norm, so this was set to affect more apps over time. Adds a sniffer that, after a main-frame load, ranks the page's declared icons (raster `rel="icon"` → SVG `rel="icon"` → apple-touch-icon → /favicon.ico) and fetches the best one IN PAGE CONTEXT, falling through on failure. The bytes are size-bounded and magic-byte validated before being relayed down the existing record path; anything unrecognised is dropped. Fetching in page context is the point, not an implementation detail. The registry captures from the WebView deliberately — "an alternative to the main app fetching host/favicon.ico itself, which would bypass Tor and leak the request". Ditto's /favicon.ico exists and returns a valid icon, so fetching it from the app would have been the easy fix and the wrong one. Every byte still comes through the sandbox WebView's own network path. No rasterisation needed: coil3's SVG decoder is registered on the singleton loader and sniffs by content rather than extension, so stored SVG bytes decode even under the registry's .png filename. Only a leading BOM/whitespace trim was required so byte 0 is the '<' the sniffer wants. Also fixes a second gap found while diagnosing: `NappletBrowserService` — the EMBEDDED path backing pinned bottom-nav tabs — had no icon capture at all, so a pinned app's icon depended on having once opened it full screen. It now gets both the missing raster callback and the sniff. `NappletHostService` deliberately left alone: it serves verified blobs over a synthetic internal host, and applet icons already come from the manifest, so capture there would be dead code. The sniff runs after a delay and skips when the raster callback already claimed the host, so sites that worked before are untouched. Verified on device: ditto.pub now shows its real icon on the favorite card, the pinned sidebar tab and the recents row, via both the embedded and full-screen paths; Brainstorm is unchanged; example.com (no usable icon) degrades to the placeholder with zero record calls and no hang; no crashes. Pre-existing, not fixed: `BrowserIconRegistry.record` writes the file on the IPC handler thread, tripping StrictMode. It did so before this change — which now simply gives it more occasions. Moving that write off-main is a cheap follow-up. Co-Authored-By: Claude Opus 4.8 --- .../napplethost/NappletBrowserActivity.kt | 33 +++ .../napplethost/NappletBrowserService.kt | 85 +++++++ .../napplethost/NappletFaviconSniffer.kt | 233 ++++++++++++++++++ 3 files changed, 351 insertions(+) create mode 100644 nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletFaviconSniffer.kt diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt index ff389271d7..8d5cb59f9d 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt @@ -432,10 +432,32 @@ class NappletBrowserActivity : ComponentActivity() { // Record only a clean http(s) main-frame load — never a typed-but-failed address. if (!mainFrameLoadFailed && (url.startsWith("https://") || url.startsWith("http://"))) { recordHistory(url, view.title) + scheduleFaviconSniff(view, url) } } } + /** + * Second-chance favicon capture for pages `onReceivedIcon` never fires for (SVG-only declarations — + * WebView does not rasterize those into the callback). Deliberately delayed so the WebView's own + * raster path, which usually lands shortly after the page finishes, gets first claim on the host; + * if it did, [lastIconHost] is already set and we skip out entirely. + */ + private fun scheduleFaviconSniff( + view: WebView, + url: String, + ) { + val host = OmniboxInput.hostOf(url) ?: return + view.postDelayed({ + if (mainFrameLoadFailed || host == lastIconHost || view.url != url) return@postDelayed + NappletFaviconSniffer.capture(view) { sniffedHost, bytes -> + if (sniffedHost == lastIconHost) return@capture + lastIconHost = sniffedHost + recordIconBytes(sniffedHost, bytes) + } + }, FAVICON_SNIFF_DELAY_MS) + } + /** Relays a successfully loaded page to the main-process broker for the device-local visit history. */ private fun recordHistory( url: String, @@ -470,6 +492,14 @@ class NappletBrowserActivity : ComponentActivity() { out.toByteArray() } }.getOrNull() ?: return + recordIconBytes(host, bytes) + } + + /** Relays already-encoded icon bytes (PNG/ICO/… or SVG source) to the broker as [host]'s favicon. */ + private fun recordIconBytes( + host: String, + bytes: ByteArray, + ) { val msg = Message.obtain(null, NappletIpc.MSG_RECORD_ICON).apply { data = @@ -754,6 +784,9 @@ class NappletBrowserActivity : ComponentActivity() { /** Max favicon edge (px) before sending over IPC — keeps the PNG tiny, well under the Binder limit. */ private const val ICON_MAX_PX = 96 + /** Grace period after page-finish before the declared-icon sniff runs, so `onReceivedIcon` wins first. */ + private const val FAVICON_SNIFF_DELAY_MS = 1_200L + private const val EXTRA_URL = "url" private const val EXTRA_PROXY_PORT = "proxyPort" private const val EXTRA_USE_TOR = "useTor" diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserService.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserService.kt index 447499bbd8..27456bcc10 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserService.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserService.kt @@ -46,6 +46,7 @@ import android.webkit.WebView import android.webkit.WebViewClient import androidx.annotation.RequiresApi import androidx.core.graphics.createBitmap +import androidx.core.graphics.scale import androidx.privacysandbox.ui.provider.toCoreLibInfo import androidx.webkit.JavaScriptReplyProxy import androidx.webkit.WebMessageCompat @@ -98,6 +99,11 @@ class NappletBrowserService : Service() { // the surface (the embedded surface has no error page of its own). var loadFailed = false + // Host whose favicon this tab already relayed. A page fires the icon callbacks several times, and + // both capture paths (WebView raster + declared-icon sniff) can land for the same visit, so this + // keeps it to one relay per host per visit. Re-armed when the host changes. + var lastIconHost: String? = null + // Per visited origin: its broker-minted launch token, the requests queued until it arrives, and // the origins a mint is already in flight for — so NIP-07 consent is scoped per site, per tab. val originTokens = mutableMapOf() @@ -328,6 +334,22 @@ class NappletBrowserService : Service() { private inner class BrowserChromeClient( private val tab: BrowserTab?, ) : WebChromeClient() { + /** + * The embedded surface captures favicons just like the full-screen browser does — a site pinned + * to a tab but never opened full-screen would otherwise never contribute an icon at all. + */ + override fun onReceivedIcon( + view: WebView, + icon: Bitmap?, + ) { + val tab = tab ?: return + if (icon == null || tab.loadFailed) return + val host = OmniboxInput.hostOf(view.url ?: return) ?: return + if (host == tab.lastIconHost) return + tab.lastIconHost = host + recordIcon(host, icon) + } + override fun onConsoleMessage(consoleMessage: ConsoleMessage): Boolean { if (tab == null) return false pushConsoleLog( @@ -385,6 +407,8 @@ class NappletBrowserService : Service() { ) { // A new main-frame navigation cleared any prior error. tab?.loadFailed = false + // Re-arm favicon capture when the host changes, so a same-host in-page nav doesn't re-send. + if (tab != null && OmniboxInput.hostOf(url) != tab.lastIconHost) tab.lastIconHost = null pushUrl(tab, view) pushLoadState(tab, view, isLoading = true) } @@ -401,6 +425,7 @@ class NappletBrowserService : Service() { ) { pushUrl(tab, view) pushLoadState(tab, view, isLoading = false) + if (url.startsWith("https://") || url.startsWith("http://")) scheduleFaviconSniff(tab, view, url) } override fun onReceivedError( @@ -551,6 +576,60 @@ class NappletBrowserService : Service() { if (brokerMessenger == null) pendingBrokerRequests.add(msg) else sendToBroker(msg) } + /** + * Second-chance favicon capture for pages `onReceivedIcon` never fires for (SVG-only declarations — + * WebView does not rasterize those into the callback). Delayed so the WebView's own raster path, + * which usually lands just after page-finish, gets first claim on the host. + */ + private fun scheduleFaviconSniff( + tab: BrowserTab?, + view: WebView, + url: String, + ) { + if (tab == null) return + val host = OmniboxInput.hostOf(url) ?: return + view.postDelayed({ + if (tabs[tab.sessionId] !== tab || tab.loadFailed || host == tab.lastIconHost || view.url != url) return@postDelayed + NappletFaviconSniffer.capture(view) { sniffedHost, bytes -> + if (sniffedHost == tab.lastIconHost) return@capture + tab.lastIconHost = sniffedHost + recordIconBytes(sniffedHost, bytes) + } + }, FAVICON_SNIFF_DELAY_MS) + } + + /** Scales [icon] down and relays it to the broker as the favicon for [host] (PNG bytes over IPC). */ + private fun recordIcon( + host: String, + icon: Bitmap, + ) { + val bytes = + runCatching { + val scaled = if (icon.width > ICON_MAX_PX || icon.height > ICON_MAX_PX) icon.scale(ICON_MAX_PX, ICON_MAX_PX) else icon + ByteArrayOutputStream().use { out -> + scaled.compress(Bitmap.CompressFormat.PNG, 100, out) + out.toByteArray() + } + }.getOrNull() ?: return + recordIconBytes(host, bytes) + } + + /** Relays already-encoded icon bytes (PNG/ICO/… or SVG source) to the broker as [host]'s favicon. */ + private fun recordIconBytes( + host: String, + bytes: ByteArray, + ) { + val msg = + Message.obtain(null, NappletIpc.MSG_RECORD_ICON).apply { + data = + Bundle().apply { + putString(NappletIpc.KEY_ICON_HOST, host) + putByteArray(NappletIpc.KEY_ICON_BYTES, bytes) + } + } + if (brokerMessenger == null) pendingBrokerRequests.add(msg) else sendToBroker(msg) + } + private fun sendToBroker(msg: Message) { try { brokerMessenger?.send(msg) @@ -603,5 +682,11 @@ class NappletBrowserService : Service() { private companion object { private const val TAG = "NappletBrowserService" private const val ABOUT_BLANK = "about:blank" + + /** Max favicon edge (px) before sending over IPC — keeps the PNG tiny, well under the Binder limit. */ + private const val ICON_MAX_PX = 96 + + /** Grace period after page-finish before the declared-icon sniff runs, so `onReceivedIcon` wins first. */ + private const val FAVICON_SNIFF_DELAY_MS = 1_200L } } diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletFaviconSniffer.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletFaviconSniffer.kt new file mode 100644 index 0000000000..ee9b5e333e --- /dev/null +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletFaviconSniffer.kt @@ -0,0 +1,233 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.napplethost + +import android.os.Handler +import android.os.Looper +import android.util.Base64 +import android.util.Log +import android.webkit.WebView +import com.vitorpamplona.amethyst.commons.browser.OmniboxInput +import org.json.JSONObject + +/** + * Declared-favicon capture for the sandboxed browser WebViews, complementing + * `WebChromeClient.onReceivedIcon`. + * + * `onReceivedIcon` only ever hands back a **rasterized** `Bitmap`, and Android WebView simply does not + * rasterize an SVG favicon into it. A site that declares *only* `` + * (ditto.pub) therefore never fires the callback at all, while a site that also declares a PNG + * alternative (brainstorm.nosfabrica.com) does. This closes that gap: after the page settles we ask the + * **page itself** for its declared icon and let the **page** fetch the bytes. + * + * Privacy: every byte here is fetched by `fetch()` running *inside the loaded page's own JS context*, so + * the request rides the exact network path the page already rides — the sandbox WebView's proxy, i.e. + * Tor when Tor is on. The main process still never touches an icon URL, which is the property + * `BrowserIconRegistry`'s KDoc protects. Fetches are `credentials: 'omit'` so no cookie rides along. + * + * Trust: the returned bytes come from a page we do not trust, so they are size-bounded and magic-byte + * validated here before they are relayed. Anything unrecognized is dropped silently — a site with no + * usable icon simply has no icon. + */ +internal object NappletFaviconSniffer { + private const val TAG = "NappletFaviconSniffer" + + /** Hard cap on the icon payload. Well under the Binder limit and far above any sane favicon. */ + private const val MAX_ICON_BYTES = 384 * 1024 + + /** Polls for the in-page async fetch to settle. Bounded: gives up (silently) after the last one. */ + private val POLL_DELAYS_MS = longArrayOf(400, 900, 1_800, 3_000, 5_000) + + private val handler = Handler(Looper.getMainLooper()) + + /** + * Asks [webView]'s current page for its best declared icon and relays the bytes to [onIcon]. + * + * No-ops unless the page is a real http(s) page with a parseable host. [onIcon] fires at most once + * per call, on the main thread; it never fires when the page declares nothing usable, when every + * candidate fails to load, or when the page navigates away mid-flight. + */ + fun capture( + webView: WebView, + onIcon: (host: String, bytes: ByteArray) -> Unit, + ) { + val url = webView.url ?: return + if (!url.startsWith("https://") && !url.startsWith("http://")) return + val host = OmniboxInput.hostOf(url) ?: return + + // Scopes this attempt to this navigation: a poll that lands after the page moved on sees a + // different (or absent) seq and drops out instead of attributing a stale icon to the new host. + val seq = System.nanoTime().toString() + runCatching { webView.evaluateJavascript(startScript(seq), null) } + .onFailure { return } + + poll(webView, host, seq, url, 0, onIcon) + } + + private fun poll( + webView: WebView, + host: String, + seq: String, + pageUrl: String, + attempt: Int, + onIcon: (String, ByteArray) -> Unit, + ) { + if (attempt >= POLL_DELAYS_MS.size) return + handler.postDelayed({ + // The page navigated away — this navigation's icon is no longer interesting. + if (webView.url != pageUrl) return@postDelayed + runCatching { + webView.evaluateJavascript(pollScript(seq)) { raw -> + val state = parse(raw) + when { + state == null -> poll(webView, host, seq, pageUrl, attempt + 1, onIcon) + state.first == "ok" -> decode(state.second)?.let { onIcon(host, it) } + state.first == "pending" -> poll(webView, host, seq, pageUrl, attempt + 1, onIcon) + else -> Unit // "none" — the page has no icon we could load. Leave it undecorated. + } + } + } + }, POLL_DELAYS_MS[attempt]) + } + + /** `state` to base64 payload, or null when the page has not produced a result for this seq yet. */ + private fun parse(raw: String?): Pair? { + if (raw == null || raw == "null") return null + val json = runCatching { JSONObject(raw) }.getOrNull() ?: return null + val state = json.optString("state").ifBlank { return null } + return state to json.optString("data") + } + + /** + * Base64 → validated image bytes. Rejects anything that is not a recognizable image, and trims any + * leading whitespace/BOM off an SVG so Coil's content sniffing (which requires `<` at offset 0 to + * recognize an SVG) still fires even though the registry stores every icon under a `.png` name. + */ + private fun decode(base64: String): ByteArray? { + if (base64.isBlank()) return null + // 4 base64 chars per 3 bytes; reject before allocating the decoded array. + if (base64.length > (MAX_ICON_BYTES / 3) * 4 + 4) return null + val bytes = + runCatching { Base64.decode(base64, Base64.DEFAULT) } + .onFailure { Log.w(TAG, "Undecodable favicon payload", it) } + .getOrNull() ?: return null + if (bytes.isEmpty() || bytes.size > MAX_ICON_BYTES) return null + if (isRaster(bytes)) return bytes + return trimmedSvg(bytes) + } + + private fun isRaster(b: ByteArray): Boolean { + if (b.size < 4) return false + + fun at(i: Int) = b[i].toInt() and 0xFF + // PNG, JPEG, GIF, BMP, ICO/CUR, RIFF (WebP). + if (at(0) == 0x89 && at(1) == 0x50 && at(2) == 0x4E && at(3) == 0x47) return true + if (at(0) == 0xFF && at(1) == 0xD8 && at(2) == 0xFF) return true + if (at(0) == 0x47 && at(1) == 0x49 && at(2) == 0x46 && at(3) == 0x38) return true + if (at(0) == 0x42 && at(1) == 0x4D) return true + if (at(0) == 0x00 && at(1) == 0x00 && (at(2) == 0x01 || at(2) == 0x02) && at(3) == 0x00) return true + if (at(0) == 0x52 && at(1) == 0x49 && at(2) == 0x46 && at(3) == 0x46) return true + return false + } + + /** Non-null only when the payload really is XML/SVG text, re-based so byte 0 is the opening `<`. */ + private fun trimmedSvg(b: ByteArray): ByteArray? { + var start = 0 + // UTF-8 BOM. + if (b.size >= 3 && (b[0].toInt() and 0xFF) == 0xEF && (b[1].toInt() and 0xFF) == 0xBB && (b[2].toInt() and 0xFF) == 0xBF) start = 3 + while (start < b.size && b[start].toInt().toChar().isWhitespace()) start++ + if (start >= b.size || b[start].toInt().toChar() != '<') return null + // Must actually mention an = 0; + var isSvg = type.indexOf('svg') >= 0 || /\.svg([?#]|${'$'})/i.test(href); + if (isIcon && !isSvg) cands.push({ h: href, s: 100 }); + else if (isIcon && isSvg) cands.push({ h: href, s: 80 }); + else if (isApple) cands.push({ h: href, s: 60 }); + } + if (location.origin && location.origin.indexOf('http') === 0) { + cands.push({ h: location.origin + '/favicon.ico', s: 10 }); + } + cands.sort(function (a, b) { return b.s - a.s; }); + if (!cands.length) { st.state = 'none'; return; } + (function next(i) { + if (i >= cands.length) { st.state = 'none'; return; } + fetch(cands[i].h, { credentials: 'omit', redirect: 'follow' }) + .then(function (r) { if (!r.ok) throw 0; return r.blob(); }) + .then(function (blob) { + if (!blob || !blob.size || blob.size > $MAX_ICON_BYTES) throw 0; + return new Promise(function (res, rej) { + var fr = new FileReader(); + fr.onload = function () { res(String(fr.result)); }; + fr.onerror = function () { rej(0); }; + fr.readAsDataURL(blob); + }); + }) + .then(function (durl) { + var c = durl.indexOf(','); + if (c < 0) throw 0; + st.data = durl.substring(c + 1); + st.state = 'ok'; + }) + .catch(function () { next(i + 1); }); + })(0); + } catch (e) { + window['__amethystFavicon'] = { seq: '$seq', state: 'none', data: '' }; + } + })(); + """.trimIndent() + + /** Returns the stashed result as an object (WebView JSON-encodes it), or null if it is not ours. */ + private fun pollScript(seq: String): String = + """ + (function () { + var s = window['__amethystFavicon']; + if (!s || s.seq !== '$seq') return null; + return { state: s.state, data: s.data || '' }; + })(); + """.trimIndent() +}