diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/EmbeddedPageRequests.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/EmbeddedPageRequests.kt index 67119d18c3..b7ed7888e8 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/EmbeddedPageRequests.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/EmbeddedPageRequests.kt @@ -51,5 +51,6 @@ data class EmbeddedDownloadRequest( val origin: String, val fileName: String, val sizeBytes: Long, + val sourceHost: String?, val risky: Boolean, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/EmbeddedWebAppController.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/EmbeddedWebAppController.kt index df043eb826..2e56d97b8a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/EmbeddedWebAppController.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/EmbeddedWebAppController.kt @@ -129,7 +129,7 @@ class EmbeddedWebAppController( /** The camera / microphone / location request the page is waiting on, if any. */ val pendingPermission = mutableStateOf(null) - /** The inline download awaiting the user's consent, if any. */ + /** The download the page started that awaits the user's consent, if any. */ val pendingDownload = mutableStateOf(null) /** The certificate of the page on screen, once page info asked for it (null for none, or not yet). */ @@ -377,7 +377,7 @@ class EmbeddedWebAppController( val origin = data.getString(NappletBrowserContract.KEY_BROWSER_ORIGIN) val name = data.getString(NappletBrowserContract.KEY_DOWNLOAD_NAME).orEmpty() // An unnamed/absent origin means the sandbox couldn't even state who is asking: refuse. - if (origin == null || name.isEmpty() || pendingDownload.value != null) { + if (origin == null || name.isEmpty() || pendingDownload.value != null || pendingDialog.value != null || pendingPermission.value != null) { answerDownload(id, allowed = false) return true } @@ -387,9 +387,14 @@ class EmbeddedWebAppController( origin = origin, fileName = name, sizeBytes = data.getLong(NappletBrowserContract.KEY_DOWNLOAD_SIZE, -1L), + sourceHost = data.getString(NappletBrowserContract.KEY_DOWNLOAD_SOURCE), risky = data.getBoolean(NappletBrowserContract.KEY_DOWNLOAD_RISKY, false), ) } + NappletBrowserContract.MSG_DOWNLOAD_CANCEL -> { + val id = msg.data?.getLong(NappletBrowserContract.KEY_DOWNLOAD_ID) + if (pendingDownload.value?.id == id) pendingDownload.value = null + } NappletBrowserContract.MSG_FULLSCREEN -> isFullscreen.value = msg.data?.getBoolean(NappletBrowserContract.KEY_ENABLED, false) ?: false NappletBrowserContract.MSG_MAGNIFIER_FRAME -> { val data = msg.data ?: return true @@ -509,7 +514,7 @@ class EmbeddedWebAppController( } } - /** Answers the download-consent card for [id]: true saves the bytes the sandbox already holds. */ + /** Answers the download-consent card for [id]: true lets the sandbox fetch or write the file it described. */ fun answerDownload( id: Long, allowed: Boolean, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/WebAppScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/WebAppScreen.kt index 36b516a1b7..b66cedceca 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/WebAppScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/WebAppScreen.kt @@ -410,10 +410,9 @@ private fun EmbeddedPageUi( } } - // A download the page started: nothing is fetched or saved until this is answered. - // The card shows the sanitized file name and exact byte count the sandbox will write, headed by the - // WebView-reported origin (never a page-supplied field), with a warning for installer/script-like - // extensions — the social-engineering payload names the disclosure calls out ("invoice.apk"). + // A download the page started: nothing is fetched or saved until this is answered. The card shows the + // file name that would be saved, its size and source host, headed by the WebView-reported origin (never + // a page-supplied field), with a warning for names that can be installed or run ("invoice.apk"). val downloadRequest by controller.pendingDownload downloadRequest?.let { request -> Dialog(onDismissRequest = { controller.answerDownload(request.id, allowed = false) }) { @@ -422,6 +421,7 @@ private fun EmbeddedPageUi( security = ui.security, fileName = request.fileName, sizeBytes = request.sizeBytes, + sourceHost = request.sourceHost, risky = request.risky, onAllow = { controller.answerDownload(request.id, allowed = true) }, onDeny = { controller.answerDownload(request.id, allowed = false) }, diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserDownloadRules.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserDownloadRules.kt index 6b85a01384..fe22abfa1c 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserDownloadRules.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserDownloadRules.kt @@ -22,14 +22,15 @@ package com.vitorpamplona.amethyst.commons.browser import kotlin.io.encoding.Base64 import kotlin.time.Duration +import kotlin.time.Duration.Companion.minutes import kotlin.time.Duration.Companion.seconds import kotlin.time.TimeMark import kotlin.time.TimeSource /** - * The platform-free rules behind the in-app browser's download consent: which file names deserve a - * warning, how a page-supplied `data:` URL turns into the bytes a consent card describes, and how often - * one site may ask. + * The platform-free rules behind the in-app browser's download consent: what a saved file may be named, + * which names deserve a warning, how a page-supplied `data:` URL turns into the bytes a consent card + * describes, and how often one site may ask. * * A page can start a download without any gesture (a navigation to an attachment, a script `.click()` * on an ``, or a hand-forged bridge envelope), so every page-initiated save asks the user @@ -39,12 +40,16 @@ object BrowserDownloadRules { /** Cap for bytes a page hands over inline (a `blob:`/`data:` download travels as base64 over the bridge). */ const val MAX_INLINE_BYTES = 25 * 1024 * 1024 + /** Longest file name kept; longer ones are shortened in the middle so the extension survives. */ + const val MAX_NAME_LENGTH = 120 + /** - * Extensions something can install or run from. The consent card flags them; the name itself is - * never rewritten, so the user judges the real one. + * Extensions something can install, run, or open as a live page from. The consent card flags them; + * the name itself is never rewritten, so the user judges the real one. */ val RISKY_EXTENSIONS = setOf( + // Android "apk", "apks", "apkm", @@ -53,22 +58,34 @@ object BrowserDownloadRules { "jar", "dex", "so", + // Windows "exe", "msi", + "msix", + "appx", "bat", "cmd", "com", + "cpl", + "pif", + "reg", "scr", "hta", "ps1", + "js", + "jse", "vbs", + "vbe", "wsf", "lnk", "url", + // Apple "dmg", "pkg", "app", + "ipa", "command", + // Linux "deb", "rpm", "appimage", @@ -77,15 +94,48 @@ object BrowserDownloadRules { "zsh", "bash", "desktop", + // Pages that run script when opened "html", "htm", "xhtml", + "xht", + "mht", + "mhtml", "svg", + "svgz", ) /** Whether [fileName]'s last extension is one a user should double-check before saving. */ fun isRisky(fileName: String): Boolean = fileName.substringAfterLast('.', "").trim().lowercase() in RISKY_EXTENSIONS + // Path-reserved characters become "_"; C0/C1 controls, DEL, bidi controls and zero-width characters + // are dropped outright, since they can make "invoice[RLO]gpj.apk" render as "invoicekpa.jpg". + private val reservedNameChars = Regex("[:*?\"<>|]") + private val invisibleNameChars = Regex("[\\u0000-\\u001f\\u007f-\\u009f\\u061c\\u200b-\\u200f\\u202a-\\u202e\\u2060-\\u2069\\ufeff]") + + /** + * A plain file name from a page's suggestion: no path parts, no reserved or invisible characters, at + * most [MAX_NAME_LENGTH] long with its extension kept. Null when nothing usable is left. + */ + fun safeFileName(suggested: String?): String? { + val base = + suggested + ?.substringAfterLast('/') + ?.substringAfterLast('\\') + ?.replace(invisibleNameChars, "") + ?.replace(reservedNameChars, "_") + ?.trim() + ?.takeIf { it.isNotEmpty() && it != "." && it != ".." } + ?: return null + if (base.length <= MAX_NAME_LENGTH) return base + val ext = base.substringAfterLast('.', "") + return if (ext.isNotEmpty() && ext.length <= 16) { + base.take(MAX_NAME_LENGTH - ext.length - 1).trimEnd() + "." + ext + } else { + base.take(MAX_NAME_LENGTH) + } + } + /** A decoded `data:` URL: the declared MIME type (null when absent) and the payload bytes. */ class DataUrl( val mimeType: String?, @@ -106,58 +156,85 @@ object BrowserDownloadRules { if (!dataUrl.startsWith("data:", ignoreCase = true)) return null val comma = dataUrl.indexOf(',') if (comma < 0) return null - val header = dataUrl.substring(5, comma) - val params = header.split(';') + val params = dataUrl.substring(5, comma).split(';') val mime = params .first() .trim() .lowercase() .takeIf { it.isNotEmpty() } - val isBase64 = params.drop(1).any { it.trim().equals("base64", ignoreCase = true) } + val isBase64 = params.size > 1 && params.last().trim().equals("base64", ignoreCase = true) val payloadLength = dataUrl.length - comma - 1 val bytes = if (isBase64) { - // 4 chars per 3 bytes, plus slack for padding and the line breaks a lenient decoder skips. - if (payloadLength > maxBytes / 3 * 4 + 1024) return null + // 4 chars per 3 bytes, plus room for the CRLF a MIME encoder puts every 76 chars. + if (payloadLength.toLong() > maxBytes / 3 * 4L + maxBytes / 57 * 2L + 1024) return null runCatching { lenientBase64.decode(dataUrl, comma + 1, dataUrl.length) }.getOrNull() ?: return null } else { - // A percent escape is 3 chars per byte, a literal char up to 4 UTF-8 bytes. - if (payloadLength > maxBytes) return null - percentDecode(dataUrl, comma + 1) ?: return null + // A percent escape is 3 chars per byte; the exact size is counted before allocating. + if (payloadLength.toLong() > maxBytes * 3L) return null + percentDecode(dataUrl, comma + 1, maxBytes) ?: return null } if (bytes.size > maxBytes) return null return DataUrl(mime, bytes) } - /** RFC 3986 percent-decoding of [text] from [start] (a `+` stays a `+`); null on a broken escape. */ + /** + * RFC 3986 percent-decoding of [text] from [start] (a `+` stays a `+`, other characters are UTF-8). + * Null on a broken escape or when the result would exceed [maxBytes], checked before allocating it. + */ private fun percentDecode( text: String, start: Int, + maxBytes: Int, ): ByteArray? { - val source = text.substring(start).encodeToByteArray() - val out = ByteArray(source.size) - var read = 0 - var written = 0 - while (read < source.size) { - val b = source[read] - if (b == '%'.code.toByte()) { - if (read + 2 >= source.size) return null - val hi = hexValue(source[read + 1]) - val lo = hexValue(source[read + 2]) - if (hi < 0 || lo < 0) return null - out[written++] = ((hi shl 4) or lo).toByte() - read += 3 + var size = 0L + var i = start + while (i < text.length) { + val c = text[i] + if (c == '%') { + if (i + 2 >= text.length || hexValue(text[i + 1]) < 0 || hexValue(text[i + 2]) < 0) return null + size += 1 + i += 3 + } else if (c.isHighSurrogate() && i + 1 < text.length && text[i + 1].isLowSurrogate()) { + size += 4 + i += 2 } else { - out[written++] = b - read++ + // A lone surrogate encodes as U+FFFD, 3 bytes, like any other char from U+0800 up. + size += + when { + c.code < 0x80 -> 1 + c.code < 0x800 -> 2 + else -> 3 + } + i++ + } + if (size > maxBytes) return null + } + val out = ByteArray(size.toInt()) + var written = 0 + i = start + var runStart = start + while (i <= text.length) { + if (i == text.length || text[i] == '%') { + if (i > runStart) { + val run = text.encodeToByteArray(runStart, i) + run.copyInto(out, written) + written += run.size + } + if (i == text.length) break + out[written++] = ((hexValue(text[i + 1]) shl 4) or hexValue(text[i + 2])).toByte() + i += 3 + runStart = i + } else { + i++ } } - return out.copyOf(written) + return if (written == out.size) out else out.copyOf(written) } - private fun hexValue(b: Byte): Int = - when (val c = b.toInt().toChar()) { + private fun hexValue(c: Char): Int = + when (c) { in '0'..'9' -> c - '0' in 'a'..'f' -> c - 'a' + 10 in 'A'..'F' -> c - 'A' + 10 @@ -166,21 +243,33 @@ object BrowserDownloadRules { } /** - * How often one browser surface (a window, or one embedded tab) lets a site show a download card: after - * the user answers a site's card, that site can't raise another for [window]. Without it a page that is - * refused can re-ask in a loop, so Cancel would never make the card go away. Main-thread only. + * How often one browser surface (a window, or one embedded tab) lets a site show a download card. After + * the user refuses a site's card, it waits [base] before it may ask again, and each further refusal + * doubles that, up to [max]; a Save resets it. Without this a refused page could re-ask in a loop until + * the user gives in. Main-thread only. */ class DownloadCooldown( - private val window: Duration = 1.seconds, + private val base: Duration = 1.seconds, + private val max: Duration = 1.minutes, private val timeSource: TimeSource = TimeSource.Monotonic, ) { - private val answeredAt = HashMap() + private class Hold( + val since: TimeMark, + val window: Duration, + ) + + private val holds = HashMap() /** Whether [origin] may show a card now. */ - fun allows(origin: String): Boolean = answeredAt[origin]?.let { it.elapsedNow() >= window } ?: true + fun allows(origin: String): Boolean = holds[origin]?.let { it.since.elapsedNow() >= it.window } ?: true - /** Starts [origin]'s cooldown: the user just answered (or the surface dismissed) its card. */ - fun answered(origin: String) { - answeredAt[origin] = timeSource.markNow() + /** The user answered [origin]'s card: [allowed] resets its wait, a refusal (or dismissal) doubles it. */ + fun answered( + origin: String, + allowed: Boolean, + ) { + val previous = holds[origin]?.window + val window = if (allowed || previous == null) base else minOf(previous * 2, max) + holds[origin] = Hold(timeSource.markNow(), window) } } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserDownloadRulesTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserDownloadRulesTest.kt index e186292c62..95b929c5b9 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserDownloadRulesTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserDownloadRulesTest.kt @@ -28,6 +28,7 @@ import kotlin.test.assertFalse import kotlin.test.assertNull import kotlin.test.assertTrue import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.minutes import kotlin.time.Duration.Companion.seconds import kotlin.time.TestTimeSource @@ -81,13 +82,48 @@ class BrowserDownloadRulesTest { assertNull(BrowserDownloadRules.decodeDataUrl("data:," + "a".repeat(11), maxBytes = 10)) } + @Test + fun base64MarkerMustBeTheLastParameter() { + assertEquals("aGk", BrowserDownloadRules.decodeDataUrl("data:text/plain;base64;x=y,aGk")!!.bytes.decodeToString()) + } + + @Test + fun percentDecodingCountsBytesBeforeAllocating() { + assertEquals(3, BrowserDownloadRules.decodeDataUrl("data:,%E4%B8%AD", maxBytes = 3)!!.bytes.size) + assertEquals("中", BrowserDownloadRules.decodeDataUrl("data:,中", maxBytes = 3)!!.bytes.decodeToString()) + assertNull(BrowserDownloadRules.decodeDataUrl("data:,中中", maxBytes = 5)) + assertEquals(4, BrowserDownloadRules.decodeDataUrl("data:,\uD83D\uDE00", maxBytes = 4)!!.bytes.size) + // A lone surrogate must not overflow the pre-counted buffer. + assertTrue(BrowserDownloadRules.decodeDataUrl("data:,a\uDE00b%41")!!.bytes.isNotEmpty()) + } + + @Test + fun safeFileNameStripsPathsAndInvisibleCharacters() { + assertEquals("evil.apk", BrowserDownloadRules.safeFileName("../../evil.apk")) + assertEquals("evil.apk", BrowserDownloadRules.safeFileName("C:\\x\\evil.apk")) + assertEquals("a_b_.pdf", BrowserDownloadRules.safeFileName("a:b?.pdf")) + assertEquals("invoicegpj.apk", BrowserDownloadRules.safeFileName("invoice\u202Egpj.apk")) + assertEquals("ab.txt", BrowserDownloadRules.safeFileName("a\u200Bb\u0085.txt")) + assertNull(BrowserDownloadRules.safeFileName("..")) + assertNull(BrowserDownloadRules.safeFileName(" \u200F ")) + assertNull(BrowserDownloadRules.safeFileName(null)) + } + + @Test + fun safeFileNameKeepsTheExtensionWhenShortening() { + val name = BrowserDownloadRules.safeFileName("photo.jpg" + "x".repeat(300) + ".apk")!! + assertEquals(BrowserDownloadRules.MAX_NAME_LENGTH, name.length) + assertTrue(name.endsWith(".apk")) + assertTrue(BrowserDownloadRules.isRisky(name)) + } + @Test fun cooldownHoldsOffOnlyTheAnsweredOrigin() { val clock = TestTimeSource() - val cooldown = DownloadCooldown(1.seconds, clock) + val cooldown = DownloadCooldown(1.seconds, 1.minutes, clock) assertTrue(cooldown.allows("https://a.example")) - cooldown.answered("https://a.example") + cooldown.answered("https://a.example", allowed = true) assertFalse(cooldown.allows("https://a.example")) assertTrue(cooldown.allows("https://b.example")) @@ -96,4 +132,30 @@ class BrowserDownloadRulesTest { clock += 1.milliseconds assertTrue(cooldown.allows("https://a.example")) } + + @Test + fun cooldownDoublesOnEachRefusalAndResetsOnSave() { + val clock = TestTimeSource() + val cooldown = DownloadCooldown(1.seconds, 4.seconds, clock) + val site = "https://a.example" + + cooldown.answered(site, allowed = false) // 1s + clock += 1.seconds + assertTrue(cooldown.allows(site)) + cooldown.answered(site, allowed = false) // 2s + clock += 1.seconds + assertFalse(cooldown.allows(site)) + clock += 1.seconds + assertTrue(cooldown.allows(site)) + cooldown.answered(site, allowed = false) // 4s + cooldown.answered(site, allowed = false) // capped at 4s + clock += 3.seconds + assertFalse(cooldown.allows(site)) + clock += 1.seconds + assertTrue(cooldown.allows(site)) + + cooldown.answered(site, allowed = true) // back to 1s + clock += 1.seconds + assertTrue(cooldown.allows(site)) + } } diff --git a/commonsUI/src/commonMain/composeResources/values/strings.xml b/commonsUI/src/commonMain/composeResources/values/strings.xml index eb937376ff..b7db85531f 100644 --- a/commonsUI/src/commonMain/composeResources/values/strings.xml +++ b/commonsUI/src/commonMain/composeResources/values/strings.xml @@ -6360,4 +6360,5 @@ Download this file? This file type can run code. Check that the name matches what you meant to get. Save + From %1$s diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/ui/pill/DownloadPromptCard.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/ui/pill/DownloadPromptCard.kt index 09d52f47a1..22ea19d944 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/ui/pill/DownloadPromptCard.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/ui/pill/DownloadPromptCard.kt @@ -38,6 +38,11 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -49,17 +54,21 @@ import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.resources.Res import com.vitorpamplona.amethyst.commons.resources.browser_pill_cancel +import com.vitorpamplona.amethyst.commons.resources.browser_pill_download_from import com.vitorpamplona.amethyst.commons.resources.browser_pill_download_risky import com.vitorpamplona.amethyst.commons.resources.browser_pill_download_save import com.vitorpamplona.amethyst.commons.resources.browser_pill_download_title import com.vitorpamplona.amethyst.commons.ui.note.types.formatBytes import com.vitorpamplona.amethyst.commons.ui.stringRes +import kotlinx.coroutines.delay +import kotlin.time.Duration.Companion.milliseconds /** * A download the page started, waiting for consent before anything is fetched or written to the shared * Downloads collection. A page can start one without any gesture, so the card names the site (the - * WebView-reported origin, never a page-supplied field), the exact file name that would be saved and its - * size ([sizeBytes] is -1 when the server didn't say), and nothing is saved until the user taps Save. + * WebView-reported origin, never a page-supplied field), the exact file name that would be saved, its + * size ([sizeBytes] is -1 when the server didn't say) and, for a network file, the host it comes from + * ([sourceHost]) when that isn't [host]. Nothing is saved until the user taps Save. */ @Composable fun DownloadPromptCard( @@ -67,10 +76,18 @@ fun DownloadPromptCard( security: BrowserChrome.Security, fileName: String, sizeBytes: Long, + sourceHost: String?, risky: Boolean, onAllow: () -> Unit, onDeny: () -> Unit, ) { + // Save ignores taps for a moment after the card appears, so a tap meant for whatever was under the + // finger (say, a page dialog's button the page just replaced with this card) can't land on it. + var armed by remember(fileName) { mutableStateOf(false) } + LaunchedEffect(fileName) { + delay(ARM_DELAY) + armed = true + } PageCard { if (host != null) { OriginBadge(host, security) @@ -90,10 +107,16 @@ fun DownloadPromptCard( } Spacer(Modifier.width(14.dp)) Column { - // Never cut the end off: the extension is what tells "invoice.pdf" from "invoice.pdf.apk". - Text(fileName, style = MaterialTheme.typography.titleSmall, maxLines = 3, overflow = TextOverflow.MiddleEllipsis) - if (sizeBytes >= 0) { - Text(formatBytes(sizeBytes), style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant) + // One line, cut in the middle: the extension is what tells "invoice.pdf" from "invoice.pdf.apk". + Text(fileName, style = MaterialTheme.typography.titleSmall, maxLines = 1, overflow = TextOverflow.MiddleEllipsis) + val details = + listOfNotNull( + sizeBytes.takeIf { it >= 0 }?.let { formatBytes(it) }, + // A network file can come from another host than the page asking (an ad frame, a CDN). + sourceHost?.takeUnless { it.equals(host, ignoreCase = true) }?.let { stringRes(Res.string.browser_pill_download_from, it) }, + ) + if (details.isNotEmpty()) { + Text(details.joinToString(" · "), style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, maxLines = 1, overflow = TextOverflow.MiddleEllipsis) } } } @@ -116,7 +139,9 @@ fun DownloadPromptCard( Row(Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.End) { TextButton(onClick = onDeny) { Text(stringRes(Res.string.browser_pill_cancel)) } Spacer(Modifier.width(8.dp)) - Button(onClick = onAllow) { Text(stringRes(Res.string.browser_pill_download_save)) } + Button(onClick = onAllow, enabled = armed) { Text(stringRes(Res.string.browser_pill_download_save)) } } } } + +private val ARM_DELAY = 500.milliseconds diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/BrowserChromeHost.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/BrowserChromeHost.kt index ad595fd2fb..2950d2c9d4 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/BrowserChromeHost.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/BrowserChromeHost.kt @@ -141,6 +141,7 @@ class BrowserChromeHost( val security: BrowserChrome.Security, val fileName: String, val sizeBytes: Long, + val sourceHost: String?, val risky: Boolean, val answer: (allow: Boolean) -> Unit, ) @@ -371,6 +372,7 @@ class BrowserChromeHost( security = pending.security, fileName = pending.fileName, sizeBytes = pending.sizeBytes, + sourceHost = pending.sourceHost, risky = pending.risky, onAllow = { downloadPrompt = null diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/BrowserDownloads.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/BrowserDownloads.kt index 31a7d57ae1..1676f074d2 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/BrowserDownloads.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/BrowserDownloads.kt @@ -30,6 +30,7 @@ import android.provider.MediaStore import android.webkit.MimeTypeMap import android.webkit.URLUtil import android.widget.Toast +import androidx.core.net.toUri import com.vitorpamplona.amethyst.commons.browser.BrowserDownloadRules import com.vitorpamplona.quartz.utils.Log import okhttp3.Request @@ -37,6 +38,7 @@ import java.io.File import java.io.OutputStream import java.util.concurrent.Executors import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean import com.vitorpamplona.amethyst.commons.R as CommonsR /** @@ -67,22 +69,27 @@ object BrowserDownloads { private val decoder = Executors.newSingleThreadExecutor { Thread(it, "napplet-download-decode").apply { isDaemon = true } } private val main = Handler(Looper.getMainLooper()) - private val unsafeNameChars = Regex("[\\u0000-\\u001f:*?\"<>|]") - /** - * A page-initiated download, resolved to exactly what its consent card shows: [save] writes a file - * named [fileName] and nothing else, and until it runs no byte has been fetched or written. + * A page-initiated download, resolved to what its consent card shows: [save] stores one file named + * [fileName], typed by that name's extension (so the system can't append a different one), and until + * it runs no byte has been fetched or written. [save] runs at most once, however often it is called. */ class DownloadOffer internal constructor( val fileName: String, - /** The exact size in bytes, or -1 when the server didn't say. */ + /** The size in bytes: exact for inline data, the server's advertised length otherwise; -1 when unknown. */ val sizeBytes: Long, + /** The host a network download is fetched from (it can differ from the page's), or null for inline data. */ + val sourceHost: String?, private val start: (Context) -> Unit, ) { + private val started = AtomicBoolean(false) + /** Whether [fileName] is something that can be installed or run, which the card warns about. */ val risky: Boolean get() = BrowserDownloadRules.isRisky(fileName) - fun save(context: Context) = start(context.applicationContext) + fun save(context: Context) { + if (started.compareAndSet(false, true)) start(context.applicationContext) + } } /** @@ -105,7 +112,7 @@ object BrowserDownloads { return } if (!isHttp(url)) return - startNetworkDownload(app, url, networkName(url, contentDisposition, mimeType), userAgent, mimeType, cookie, proxyPort) + startNetworkDownload(app, url, networkName(url, contentDisposition, mimeType), userAgent, cookie, proxyPort) } /** @@ -134,8 +141,8 @@ object BrowserDownloads { } val name = networkName(url, contentDisposition, mimeType) onReady( - DownloadOffer(name, contentLength.takeIf { it > 0 } ?: -1L) { app -> - startNetworkDownload(app, url, name, userAgent, mimeType, cookie, proxyPort) + DownloadOffer(name, contentLength.takeIf { it > 0 } ?: -1L, url.toUri().host) { app -> + startNetworkDownload(app, url, name, userAgent, cookie, proxyPort) }, ) } @@ -151,10 +158,17 @@ object BrowserDownloads { onReady: (DownloadOffer?) -> Unit, ) { decoder.execute { + // Throwable, not Exception: an OutOfMemoryError here must refuse the download, not kill the + // process (and every tab in it) or leave the caller waiting on a callback that never comes. val offer = - BrowserDownloadRules.decodeDataUrl(dataUrl)?.let { data -> - val name = safeName(suggestedName, data.mimeType) - DownloadOffer(name, data.bytes.size.toLong()) { app -> writeInBackground(app, name, data.mimeType, data.bytes) } + try { + BrowserDownloadRules.decodeDataUrl(dataUrl)?.let { data -> + val name = safeName(suggestedName, data.mimeType) + DownloadOffer(name, data.bytes.size.toLong(), null) { app -> writeInBackground(app, name, data.bytes) } + } + } catch (e: Throwable) { + Log.w(TAG, "Inline download refused", e) + null } main.post { onReady(offer) } } @@ -173,7 +187,6 @@ object BrowserDownloads { url: String, name: String, userAgent: String?, - mimeType: String?, cookie: String?, proxyPort: Int, ) { @@ -200,8 +213,7 @@ object BrowserDownloads { .build() client.newCall(request).execute().use { response -> if (!response.isSuccessful) error("HTTP ${response.code}") - val type = mimeType?.takeIf { it.isNotBlank() && it != "application/octet-stream" } ?: response.body.contentType()?.let { "${it.type}/${it.subtype}" } - write(app, name, type) { out -> response.body.byteStream().use { it.copyTo(out) } } + write(app, name) { out -> response.body.byteStream().use { it.copyTo(out) } } } }.onFailure { Log.w(TAG, "Download failed for $url", it) } .getOrDefault(false) @@ -209,26 +221,26 @@ object BrowserDownloads { } } - /** Saves a `data:` URL (`data:[mime][;base64],payload`) the user asked for directly. */ - fun saveDataUrl( - context: Context, + /** Saves a `data:` URL (`data:[mime][;base64],payload`) the user asked for directly, decoded off the main thread. */ + private fun saveDataUrl( + app: Context, dataUrl: String, suggestedName: String?, ) { - val data = BrowserDownloadRules.decodeDataUrl(dataUrl) ?: return - val mime = data.mimeType ?: "application/octet-stream" - writeInBackground(context.applicationContext, safeName(suggestedName, mime), mime, data.bytes) + decoder.execute { + val data = runCatching { BrowserDownloadRules.decodeDataUrl(dataUrl) }.getOrNull() ?: return@execute + writeInBackground(app, safeName(suggestedName, data.mimeType), data.bytes) + } } private fun writeInBackground( app: Context, name: String, - mimeType: String?, bytes: ByteArray, ) { if (bytes.size > MAX_INLINE_BYTES) return io.execute { - val ok = runCatching { write(app, name, mimeType) { it.write(bytes) } }.getOrDefault(false) + val ok = runCatching { write(app, name) { it.write(bytes) } }.getOrDefault(false) toast(app, app.getString(if (ok) CommonsR.string.browser_download_saved else CommonsR.string.browser_download_failed, name)) } } @@ -241,7 +253,6 @@ object BrowserDownloads { private fun write( context: Context, name: String, - mimeType: String?, body: (OutputStream) -> Unit, ): Boolean { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { @@ -249,7 +260,9 @@ object BrowserDownloads { val values = ContentValues().apply { put(MediaStore.Downloads.DISPLAY_NAME, name) - mimeType?.let { put(MediaStore.Downloads.MIME_TYPE, it) } + // Typed by the approved name alone: a page- or server-supplied type that disagrees with + // the extension would make MediaStore append its own ("invoice.pdf" -> "invoice.pdf.apk"). + put(MediaStore.Downloads.MIME_TYPE, mimeTypeOf(name)) put(MediaStore.Downloads.RELATIVE_PATH, Environment.DIRECTORY_DOWNLOADS) put(MediaStore.Downloads.IS_PENDING, 1) } @@ -269,23 +282,28 @@ object BrowserDownloads { return true } - /** A plain filename: the page's suggestion without path parts, else "download" + the MIME's extension. */ + /** + * The file name to save under: the page's suggestion made safe ([BrowserDownloadRules.safeFileName]), + * else "download" + the MIME's extension. + */ private fun safeName( suggested: String?, mimeType: String?, ): String { - val base = - suggested - ?.substringAfterLast('/') - ?.substringAfterLast('\\') - ?.replace(unsafeNameChars, "_") - ?.trim() - ?.takeIf { it.isNotEmpty() && it != "." && it != ".." } - if (base != null) return base.take(120) + BrowserDownloadRules.safeFileName(suggested)?.let { return it } val ext = mimeType?.let { MimeTypeMap.getSingleton().getExtensionFromMimeType(it) } return if (ext != null) "download.$ext" else "download" } + /** The MIME type [name]'s extension implies, or octet-stream, which MediaStore stores under the name as given. */ + private fun mimeTypeOf(name: String): String = + name + .substringAfterLast('.', "") + .lowercase() + .takeIf { it.isNotEmpty() } + ?.let { MimeTypeMap.getSingleton().getMimeTypeFromExtension(it) } + ?: "application/octet-stream" + private fun toast( context: Context, text: String, 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 d724037235..38d2c5503b 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt @@ -365,12 +365,13 @@ class NappletBrowserActivity : ComponentActivity() { wv.webChromeClient = BrowserChromeClient() wv.setFindListener { active, total, _ -> chrome?.setFindResult(active, total) } wv.setDownloadListener { url, userAgent, contentDisposition, mimeType, contentLength -> + // The page's origin; a fresh popup still on about:blank has none, so name the file's own. val origin = chrome ?.ui ?.chrome ?.url - ?.let(BrowserChrome::originOf) ?: return@setDownloadListener + ?.let(BrowserChrome::originOf) ?: BrowserChrome.originOf(url) ?: return@setDownloadListener if (!canOfferDownload(origin)) return@setDownloadListener // Read on the main thread: the cookie jar is the tab's own per-account WebView profile. val cookie = BrowserWebTools.cookieManager(wv).getCookie(url) @@ -971,11 +972,14 @@ class NappletBrowserActivity : ComponentActivity() { } /** - * Whether [origin] may put a download card up now: never over a card already showing (so a page can't - * swap the name under the user's finger) or one still being prepared (so a page can't queue decodes), - * and not within the cooldown after its last card was answered. + * Whether [origin] may put a download card up now: never over another page prompt (so a page can't swap + * the name under the user's finger, or pop the card where a dialog's button just was), never while an + * offer is still being prepared (so a page can't queue decodes), and not within its cooldown. */ - private fun canOfferDownload(origin: String) = chrome?.downloadPrompt == null && !preparingDownload && downloadCooldown.allows(origin) + private fun canOfferDownload(origin: String): Boolean { + val host = chrome ?: return false + return host.downloadPrompt == null && host.dialog == null && host.permissionPrompt == null && !preparingDownload && downloadCooldown.allows(origin) + } /** Runs [prepare] (which answers exactly once, on the main thread) and shows the offer it produces. */ private fun prepareDownloadOffer( @@ -1002,9 +1006,10 @@ class NappletBrowserActivity : ComponentActivity() { security = BrowserChrome.security(host.ui.chrome), fileName = offer.fileName, sizeBytes = offer.sizeBytes, + sourceHost = offer.sourceHost, risky = offer.risky, ) { allowed -> - downloadCooldown.answered(origin) + downloadCooldown.answered(origin, allowed) if (allowed) offer.save(this) } } diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserContract.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserContract.kt index 0cb87270e3..7b42f651d9 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserContract.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserContract.kt @@ -188,7 +188,7 @@ object NappletBrowserContract { * Provider → client: a download the page started (an attachment, ``, or an inline * `browser.download`) needs the user's consent before anything is fetched or written into the shared * Downloads collection. Carries [KEY_DOWNLOAD_ID], the WebView-reported [KEY_BROWSER_ORIGIN] (never a - * page-supplied field), the sanitized [KEY_DOWNLOAD_NAME], [KEY_DOWNLOAD_SIZE], and + * page-supplied field), the sanitized [KEY_DOWNLOAD_NAME], [KEY_DOWNLOAD_SIZE], [KEY_DOWNLOAD_SOURCE], and * [KEY_DOWNLOAD_RISKY] (an install-or-script-like extension). Answered with * [MSG_DOWNLOAD_CONSENT_RESULT] on every outcome; the bytes themselves never leave the `:napplet` process. */ @@ -197,6 +197,9 @@ object NappletBrowserContract { /** Client → provider: the user's answer to [MSG_DOWNLOAD_CONSENT]: [KEY_DOWNLOAD_ID] + [KEY_DOWNLOAD_ALLOWED]. */ const val MSG_DOWNLOAD_CONSENT_RESULT = 35 + /** Provider → client: withdraw the [MSG_DOWNLOAD_CONSENT] card [KEY_DOWNLOAD_ID] (its tab closed). */ + const val MSG_DOWNLOAD_CANCEL = 36 + const val KEY_CAN_GO_FORWARD = "canGoForward" const val KEY_FIND_QUERY = "findQuery" const val KEY_FIND_FORWARD = "findForward" @@ -228,6 +231,9 @@ object NappletBrowserContract { /** The size in bytes the consented download will write, or -1 when the server didn't say. */ const val KEY_DOWNLOAD_SIZE = "downloadSize" + /** The host a network download is fetched from (absent for inline data), shown when it isn't the page's. */ + const val KEY_DOWNLOAD_SOURCE = "downloadSource" + /** Whether [KEY_DOWNLOAD_NAME]'s extension is one the user should double-check (installer/script-like). */ const val KEY_DOWNLOAD_RISKY = "downloadRisky" 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 c8e0697818..dcd898d7df 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserService.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserService.kt @@ -316,8 +316,9 @@ class NappletBrowserService : Service() { val data = msg.data ?: return true val pending = pendingDownloads.remove(data.getLong(NappletBrowserContract.KEY_DOWNLOAD_ID)) ?: return true val tab = tabs[pending.sessionId] ?: return true - tab.downloadCooldown.answered(pending.origin) - if (data.getBoolean(NappletBrowserContract.KEY_DOWNLOAD_ALLOWED, false)) pending.offer.save(this) + val allowed = data.getBoolean(NappletBrowserContract.KEY_DOWNLOAD_ALLOWED, false) + tab.downloadCooldown.answered(pending.origin, allowed) + if (allowed) pending.offer.save(this) } NappletBrowserContract.MSG_EXIT_FULLSCREEN -> tabFor(msg)?.let { exitFullscreen(it) } NappletBrowserContract.MSG_RELOAD -> tabFor(msg)?.webView?.reload() @@ -470,8 +471,12 @@ class NappletBrowserService : Service() { // Release a picker still waiting on this surface before its WebView goes away. tab.fileChooser.cancel() cancelPending(tab) - // An unanswered download card dies with its tab: nothing is saved, and its bytes are freed. - pendingDownloads.values.removeAll { it.sessionId == sessionId } + // An unanswered download card dies with its tab: nothing is saved, its bytes are freed, and the + // client is told so its card doesn't linger with a Save that does nothing. + pendingDownloads.entries.filter { it.value.sessionId == sessionId }.forEach { (id, _) -> + pendingDownloads.remove(id) + sendToClient(tab, NappletBrowserContract.MSG_DOWNLOAD_CANCEL) { putLong(NappletBrowserContract.KEY_DOWNLOAD_ID, id) } + } tab.customViewCallback?.onCustomViewHidden() tab.customViewCallback = null tab.customView = null @@ -489,7 +494,8 @@ class NappletBrowserService : Service() { wv.webChromeClient = BrowserChromeClient(tab) wv.setDownloadListener { url, userAgent, contentDisposition, mimeType, contentLength -> if (tab == null) return@setDownloadListener - val origin = wv.url?.let(BrowserChrome::originOf) ?: return@setDownloadListener + // The page's origin; a fresh popup still on about:blank has none, so name the file's own. + val origin = wv.url?.let(BrowserChrome::originOf) ?: BrowserChrome.originOf(url) ?: return@setDownloadListener if (!canOfferDownload(tab, origin)) return@setDownloadListener // Read on the main thread: the cookie jar is the tab's own per-account WebView profile. val cookie = BrowserWebTools.cookieManager(wv).getCookie(url) @@ -1056,14 +1062,19 @@ class NappletBrowserService : Service() { } /** - * Whether [origin] may put a download card up in [tab] now: never over the tab's card already showing - * (so a page can't swap the name under the user's finger) or one still being prepared (so a page - * can't queue decodes), and not within the cooldown after its last card was answered. + * Whether [origin] may put a download card up in [tab] now: never over another of the tab's page prompts + * (so a page can't swap the name under the user's finger, or pop the card where a dialog's button just + * was), never while an offer is still being prepared (so a page can't queue decodes), and not within + * its cooldown. */ private fun canOfferDownload( tab: BrowserTab, origin: String, - ) = !tab.preparingDownload && pendingDownloads.values.none { it.sessionId == tab.sessionId } && tab.downloadCooldown.allows(origin) + ) = !tab.preparingDownload && + tab.jsDialogs.isEmpty() && + tab.permissionRequests.isEmpty() && + pendingDownloads.values.none { it.sessionId == tab.sessionId } && + tab.downloadCooldown.allows(origin) /** Runs [prepare] (which answers exactly once, on the main thread) and relays the offer it produces. */ private fun prepareDownloadOffer( @@ -1095,6 +1106,7 @@ class NappletBrowserService : Service() { putString(NappletBrowserContract.KEY_BROWSER_ORIGIN, origin) putString(NappletBrowserContract.KEY_DOWNLOAD_NAME, offer.fileName) putLong(NappletBrowserContract.KEY_DOWNLOAD_SIZE, offer.sizeBytes) + putString(NappletBrowserContract.KEY_DOWNLOAD_SOURCE, offer.sourceHost) putBoolean(NappletBrowserContract.KEY_DOWNLOAD_RISKY, offer.risky) } if (sent) pendingDownloads[id] = PendingDownload(tab.sessionId, origin, offer)