diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/WebFileChooserActivity.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/WebFileChooserActivity.kt index 58172c35fb..c49ddda0c2 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/WebFileChooserActivity.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/WebFileChooserActivity.kt @@ -58,9 +58,14 @@ class WebFileChooserActivity : ComponentActivity() { return } - // A recreated instance (rotation) already has its pick in flight; launching again would stack a - // second picker on top of the first. - if (savedInstanceState != null) return + // A recreated instance has lost the in-flight request that its result would be matched against + // (configChanges keeps this rare — a system kill, not a rotation), and relaunching would stack a + // second picker on the first. Release the page's input now rather than let it wait on a result + // that can no longer be routed anywhere. + if (savedInstanceState != null) { + finish() + return + } chooser.launch( acceptTypes = ask.acceptTypes, @@ -76,6 +81,18 @@ class WebFileChooserActivity : ComponentActivity() { super.finish() } + /** + * The system can destroy this host without ever calling [finish] — a low-memory kill while the + * picker is on top. Without this the page's file input would wait on a result nobody is left to + * send, dead for the life of the page, and the coordinator would hold the reply callback (and the + * controller behind it) forever. + */ + override fun onDestroy() { + chooser.teardown() + report(null) + super.onDestroy() + } + private fun report(uris: Array?) { if (reported) return reported = true diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/FileChooserAccept.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/FileChooserAccept.kt index a96fe90a9c..627058f992 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/FileChooserAccept.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/FileChooserAccept.kt @@ -74,10 +74,9 @@ object FileChooserAccept { val mimes = mutableListOf() for (token in tokens) { - val mime = if (token.contains('/')) token else extensionToMime(token.removePrefix(".")) // One name we can't translate means we cannot express this page's filter faithfully. Show // everything rather than a filter that silently excludes part of what it asked for. - if (mime.isNullOrEmpty()) return Resolved(ANY, emptyList()) + val mime = mimeOf(token, extensionToMime) ?: return Resolved(ANY, emptyList()) if (mime !in mimes) mimes.add(mime) } @@ -115,8 +114,7 @@ object FileChooserAccept { val media = mutableSetOf() for (token in tokens) { if (token == ANY) return setOf(CaptureMedia.IMAGE, CaptureMedia.VIDEO) - val mime = if (token.contains('/')) token else extensionToMime(token.removePrefix(".")) - when (mime?.substringBefore('/')) { + when (mimeOf(token, extensionToMime)?.substringBefore('/')) { "image" -> media.add(CaptureMedia.IMAGE) "video" -> media.add(CaptureMedia.VIDEO) } @@ -124,6 +122,24 @@ object FileChooserAccept { return media } + /** + * One `accept` entry as a MIME type, or null when it cannot be turned into one. + * + * A token with a slash is taken as a MIME type, but only when both halves are actually there: a + * page that writes `accept="image/"` would otherwise put that straight into `Intent.setType`, where + * it matches no provider and leaves the user staring at an empty picker with no way out. Everything + * else is an extension, with or without its leading dot. + */ + private fun mimeOf( + token: String, + extensionToMime: (String) -> String?, + ): String? { + if (!token.contains('/')) return extensionToMime(token.removePrefix(".")).takeIf { !it.isNullOrEmpty() } + val type = token.substringBefore('/') + val subtype = token.substringAfter('/') + return token.takeIf { type.isNotEmpty() && subtype.isNotEmpty() && !subtype.contains('/') } + } + /** * The narrowest single type covering [mimes]: the type itself when there is only one, the shared * family's wildcard when they all belong to one family, and [ANY] when they span families (or the diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/FileChooserAcceptTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/FileChooserAcceptTest.kt index 613a256524..0e7daea130 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/FileChooserAcceptTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/FileChooserAcceptTest.kt @@ -126,6 +126,16 @@ class FileChooserAcceptTest { assertEquals(listOf("image/jpeg"), resolved.mimeTypes) } + @Test + fun malformedMimeTokenDoesNotBecomeTheFilter() { + // A half-written MIME would go straight into Intent.setType and match no provider at all, so the + // user gets an empty picker with no way out. Treat it as unnameable and show everything. + assertEquals(FileChooserAccept.ANY, resolve("image/").primaryType) + assertEquals(FileChooserAccept.ANY, resolve("/png").primaryType) + assertEquals(FileChooserAccept.ANY, resolve("/").primaryType) + assertEquals(emptyList(), resolve("image/").mimeTypes) + } + private fun capture(vararg accept: String) = FileChooserAccept.captureMedia(accept.toList(), map::get) @Test 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 9cc171e9e4..0446275389 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt @@ -302,6 +302,7 @@ class NappletBrowserActivity : ComponentActivity() { runCatching { unbindService(brokerConnection) } // A picker still up when the browser is torn down would otherwise leave its callback unanswered. pendingFileChooser.cancel() + fileChooserLauncher.teardown() if (this::webView.isInitialized) { // Detach from the view tree BEFORE destroy(). Destroying a WebView while it is still attached to // the window corrupts the SHARED multiprocess renderer/network state, which then breaks the OTHER diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletCaptureFiles.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletCaptureFiles.kt index 5f55d0eaad..f8d7183c2f 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletCaptureFiles.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletCaptureFiles.kt @@ -62,10 +62,12 @@ object NappletCaptureFiles { ): Pair? = runCatching { val dir = File(context.cacheDir, DIR).apply { mkdirs() } - // Not createTempFile's random name: a stable, sortable prefix makes the sweep below able to - // recognise our own files and nothing else. - val file = File(dir, "capture-${System.currentTimeMillis()}-${counter++}.$extension") - file.createNewFile() + // createTempFile, not a name built from a clock and a per-object sequence: the main and + // `:napplet` processes each hold their OWN copy of this object, so those sequences run + // independently and two picks started in the same millisecond could name the same file — + // one capture would then silently overwrite the other. The extension is what FileProvider + // types the URI from, so it has to survive into the suffix. + val file = File.createTempFile("capture-", ".$extension", dir) file to FileProvider.getUriForFile(context, authority(context), file) }.onFailure { Log.w(TAG, "Could not create a capture file", it) } .getOrNull() @@ -132,6 +134,4 @@ object NappletCaptureFiles { } private const val GRANT_FLAGS = Intent.FLAG_GRANT_WRITE_URI_PERMISSION or Intent.FLAG_GRANT_READ_URI_PERMISSION - - private var counter = 0 } diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletHostActivity.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletHostActivity.kt index 7abf81805b..c933bee4a3 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletHostActivity.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletHostActivity.kt @@ -461,6 +461,7 @@ class NappletHostActivity : ComponentActivity() { keyActions.clear() // A picker still up when the applet is torn down would otherwise leave its callback unanswered. pendingFileChooser.cancel() + fileChooserLauncher.teardown() if (this::webView.isInitialized) { // Detach before destroy(): destroying an attached WebView corrupts the shared multiprocess // renderer/network state and breaks the other (embedded) WebViews in this `:napplet` process diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/WebFileChooserLauncher.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/WebFileChooserLauncher.kt index 9cd4ecac21..4e1d1de74a 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/WebFileChooserLauncher.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/WebFileChooserLauncher.kt @@ -98,6 +98,10 @@ class WebFileChooserLauncher( private fun open(cameraAllowed: Boolean) { val pending = ask ?: return onResult(null) ask = null + // A second file input can ask before the first pick returns. The page's own callback is already + // released by PendingFileChooser, but the superseded request still owns scratch files and camera + // grants that nothing else would ever come back for. + abandonInFlight() val built = NappletFileChooser.buildRequest( @@ -121,6 +125,19 @@ class WebFileChooserLauncher( } } + /** Releases the scratch files and camera grants of a request whose result will never be read. */ + private fun abandonInFlight() { + val stale = request ?: return + request = null + NappletFileChooser.parseResult(activity, stale, Activity.RESULT_CANCELED, null) + } + + /** + * Called when the host is going away with a pick still open, so the request's scratch files and + * camera grants are not left behind for the stale sweep to find a day later. + */ + fun teardown() = abandonInFlight() + private fun hasCameraPermission() = ContextCompat.checkSelfPermission(activity, Manifest.permission.CAMERA) == PackageManager.PERMISSION_GRANTED private companion object {