Safer temp video handling

Cleanup deletes the actual shared file (and on error) instead of the old temp path in
Guard against empty response.body
Gate media-type detection so image sharing only checks image headers
Added unit test for rename-failure fallback

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ShareHelper.kt

@@ -37,6 +37,12 @@ object ShareHelper {
     private const val DEFAULT_VIDEO_EXTENSION = "mp4"
     private const val SHARED_FILE_PREFIX = "shared_media"
 
+    data class SharableFile(
+        val uri: Uri,
+        val extension: String,
+        val file: File,
+    )
+
     // Image type magic numbers
     private val JPEG_MAGIC = byteArrayOf(0xFF.toByte(), 0xD8.toByte())
     private val PNG_MAGIC = byteArrayOf(0x89.toByte(), 0x50.toByte(), 0x4E.toByte(), 0x47.toByte())
@@ -81,7 +87,7 @@ object ShareHelper {
 
     private fun getImageExtension(file: File): String = getMediaExtension(file, isVideo = false)
 
-    fun getVideoExtension(file: File): String = getMediaExtension(file, isVideo = true)
+    private fun getVideoExtension(file: File): String = getMediaExtension(file, isVideo = true)
 
     private fun getMediaExtension(
         file: File,
@@ -97,43 +103,49 @@ object ShareHelper {
                     return defaultExtension
                 }
 
-                when {
+                if (isVideo) {
-                    // Image formats
+                    when {
-                    // JPEG: Check first 2 bytes
+                        // Video formats
-                    matchesMagicNumbers(header, 0, JPEG_MAGIC) -> "jpg"
+                        // WebM/MKV: Check first 4 bytes (EBML header)
+                        // Both use Matroska container; default to webm as it's more common on web
+                        matchesMagicNumbers(header, 0, WEBM_MAGIC) -> "webm"
 
-                    // PNG: Check first 4 bytes
+                        // AVI: Check "RIFF" (bytes 0-3) and "AVI " (bytes 8-11)
-                    matchesMagicNumbers(header, 0, PNG_MAGIC) -> "png"
+                        matchesMagicNumbers(header, 0, AVI_HEADER_START) &&
+                            bytesRead >= 12 &&
+                            matchesMagicNumbers(header, 8, AVI_HEADER_END) -> "avi"
 
-                    // GIF: Check first 4 bytes for "GIF8"
+                        // MP4/MOV: Check for ftyp box (bytes 4-7 should be "ftyp")
-                    matchesMagicNumbers(header, 0, GIF_MAGIC) -> "gif"
+                        bytesRead >= 12 && matchesMagicNumbers(header, 4, MOV_FTYP) -> detectMp4OrMov(header)
 
-                    // WEBP: Check "RIFF" (bytes 0-3) and "WEBP" (bytes 8-11)
+                        // MP4/MOV alternative: moov, mdat, or free at offset 4
-                    matchesMagicNumbers(header, 0, WEBP_HEADER_START) &&
+                        bytesRead >= 8 && (
-                        bytesRead >= 12 &&
+                            matchesMagicNumbers(header, 4, MOV_MOOV) ||
-                        matchesMagicNumbers(header, 8, WEBP_HEADER_END) -> "webp"
+                                matchesMagicNumbers(header, 4, MOV_MDAT) ||
+                                matchesMagicNumbers(header, 4, MOV_FREE)
+                        ) -> "mp4"
 
-                    // Video formats
+                        else -> defaultExtension
-                    // WebM/MKV: Check first 4 bytes (EBML header)
+                    }
-                    // Both use Matroska container; default to webm as it's more common on web
+                } else {
-                    matchesMagicNumbers(header, 0, WEBM_MAGIC) -> "webm"
+                    when {
+                        // Image formats
+                        // JPEG: Check first 2 bytes
+                        matchesMagicNumbers(header, 0, JPEG_MAGIC) -> "jpg"
 
-                    // AVI: Check "RIFF" (bytes 0-3) and "AVI " (bytes 8-11)
+                        // PNG: Check first 4 bytes
-                    matchesMagicNumbers(header, 0, AVI_HEADER_START) &&
+                        matchesMagicNumbers(header, 0, PNG_MAGIC) -> "png"
-                        bytesRead >= 12 &&
-                        matchesMagicNumbers(header, 8, AVI_HEADER_END) -> "avi"
 
-                    // MP4/MOV: Check for ftyp box (bytes 4-7 should be "ftyp")
+                        // GIF: Check first 4 bytes for "GIF8"
-                    bytesRead >= 12 && matchesMagicNumbers(header, 4, MOV_FTYP) -> detectMp4OrMov(header)
+                        matchesMagicNumbers(header, 0, GIF_MAGIC) -> "gif"
 
-                    // MP4/MOV alternative: moov, mdat, or free at offset 4
+                        // WEBP: Check "RIFF" (bytes 0-3) and "WEBP" (bytes 8-11)
-                    bytesRead >= 8 && (
+                        matchesMagicNumbers(header, 0, WEBP_HEADER_START) &&
-                        matchesMagicNumbers(header, 4, MOV_MOOV) ||
+                            bytesRead >= 12 &&
-                            matchesMagicNumbers(header, 4, MOV_MDAT) ||
+                            matchesMagicNumbers(header, 8, WEBP_HEADER_END) -> "webp"
-                            matchesMagicNumbers(header, 4, MOV_FREE)
-                    ) -> "mp4"
 
-                    else -> defaultExtension
+                        else -> defaultExtension
+                    }
                 }
             }
         } catch (e: IOException) {
@@ -209,21 +221,36 @@ object ShareHelper {
     fun prepareTempVideoForSharing(
         context: Context,
         tempFile: File,
-    ): Pair<Uri, String> {
+    ): SharableFile {
         val extension = getVideoExtension(tempFile)
         val timestamp = System.currentTimeMillis()
         val sharableFile = File(context.cacheDir, "${SHARED_FILE_PREFIX}_$timestamp.$extension")
 
         try {
-            tempFile.renameTo(sharableFile)
+            moveTempFileForSharing(tempFile, sharableFile)
         } catch (e: Exception) {
             Log.e(TAG, "Failed to rename temp video file for sharing", e)
             throw e
         }
 
-        return Pair(
+        return SharableFile(
-            FileProvider.getUriForFile(context, "${context.packageName}.provider", sharableFile),
+            uri = FileProvider.getUriForFile(context, "${context.packageName}.provider", sharableFile),
-            extension,
+            extension = extension,
+            file = sharableFile,
         )
     }
+
+    internal fun moveTempFileForSharing(
+        tempFile: File,
+        sharableFile: File,
+        rename: (File, File) -> Boolean = { source, dest -> source.renameTo(dest) },
+    ) {
+        val renamed = rename(tempFile, sharableFile)
+        if (!renamed) {
+            tempFile.copyTo(sharableFile, overwrite = true)
+            if (!tempFile.delete()) {
+                Log.w(TAG, "Failed to delete temp file ${tempFile.path} after copy")
+            }
+        }
+    }
 }

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt

@@ -121,6 +121,7 @@ import okhttp3.Request
 import okhttp3.coroutines.executeAsync
 import okio.sink
 import java.io.File
+import java.io.IOException
 import kotlin.time.Duration.Companion.seconds
 
 @Composable
@@ -884,6 +885,7 @@ private suspend fun shareVideoFile(
     onError: () -> Unit,
 ) {
     val tempFile = ShareHelper.createTempVideoFile(context)
+    var sharedFile: File? = null
     try {
         withContext(Dispatchers.IO) {
             // Download video using streaming
@@ -897,15 +899,20 @@ private suspend fun shareVideoFile(
 
             client.newCall(request).executeAsync().use { response ->
                 check(response.isSuccessful) { "Download failed: ${response.code}" }
+                val responseBody = response.body
 
                 // Stream the response to the temp file
                 tempFile.outputStream().use { outputStream ->
-                    response.body.source().readAll(outputStream.sink())
+                    val bytesCopied = responseBody.source().readAll(outputStream.sink())
+                    if (bytesCopied == 0L) {
+                        throw IOException("Download failed: empty response body")
+                    }
                 }
             }
 
             // Prepare the temp file for sharing (determines extension and creates sharable URI)
-            val (uri, extension) = ShareHelper.prepareTempVideoForSharing(context, tempFile)
+            val (uri, extension, sharableFile) = ShareHelper.prepareTempVideoForSharing(context, tempFile)
+            sharedFile = sharableFile
 
             // Determine mime type
             val determinedMimeType = mimeType ?: "video/$extension"
@@ -924,7 +931,7 @@ private suspend fun shareVideoFile(
 
             // Schedule cleanup after 60 seconds to allow the receiving app time to copy the file
             Handler(Looper.getMainLooper()).postDelayed({
-                tempFile.delete()
+                sharableFile.delete()
             }, 60_000)
         }
 
@@ -937,6 +944,7 @@ private suspend fun shareVideoFile(
 
         // Clean up temp file on error
         tempFile.delete()
+        sharedFile?.delete()
 
         withContext(Dispatchers.Main) {
             Toast

amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/ShareHelperTest.kt

@@ -0,0 +1,52 @@
+/**
+ * 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.ui.components
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import java.io.File
+import java.nio.file.Files
+
+class ShareHelperTest {
+    @Test
+    fun moveTempFileForSharing_renameFails_copiesAndDeletesSource() {
+        val tempDir = Files.createTempDirectory("sharehelpertest").toFile()
+        val tempFile = File(tempDir, "video.tmp")
+        val content = "test-content"
+        tempFile.writeText(content)
+        val targetFile = File(tempDir, "shared.mp4")
+
+        ShareHelper.moveTempFileForSharing(
+            tempFile,
+            targetFile,
+            rename = { _, _ -> false },
+        )
+
+        assertTrue(targetFile.exists())
+        assertEquals(content, targetFile.readText())
+        assertFalse(tempFile.exists())
+
+        targetFile.delete()
+        tempDir.delete()
+    }
+}