Merge pull request #3874 from davotoula/fix/share-malformed-imeta-mime

Repair bare-subtype imeta MIME types so images can be shared
2026-08-11 08:47:33 +00:00 · 2026-08-07 09:24:19 -04:00
parent 35f96a936d ee4a5e5b89
commit 5e71eec53b
7 changed files with 217 additions and 29 deletions
@@ -25,6 +25,8 @@ import android.net.Uri
 import androidx.annotation.VisibleForTesting
 import androidx.core.content.FileProvider
 import com.vitorpamplona.amethyst.Amethyst
+import com.vitorpamplona.amethyst.commons.richtext.mimeTypeMap
+import com.vitorpamplona.amethyst.commons.richtext.normalizeMimeType
 import com.vitorpamplona.quartz.utils.Log
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
@@ -37,6 +39,7 @@ object ShareHelper {
    private const val DEFAULT_IMAGE_EXTENSION = "jpg"
    private const val DEFAULT_VIDEO_EXTENSION = "mp4"
    private const val SHARED_FILE_PREFIX = "shared_media"
+    private const val GENERIC_BINARY_MIME_TYPE = "application/octet-stream"

    data class SharableFile(
        val uri: Uri,
@@ -65,6 +68,25 @@ object ShareHelper {
    private val MP4_BRAND_MP42 = "mp42".toByteArray()
    private val MOV_BRAND_QT = "qt  ".toByteArray()

+    /**
+     * Picks the MIME type to put on an `ACTION_SEND` intent.
+     *
+     * `Intent.type` has to be a real `type/subtype` — an `IntentFilter` matches the two halves
+     * separately, so a slash-less value matches nothing and the chooser opens empty. The declared
+     * type is author-supplied and may be unusable (see [normalizeMimeType]), so it is treated as a
+     * hint; [fileExtension] is the safer signal because [getMediaExtension] sniffs it from the
+     * file's magic numbers rather than trusting the event.
+     */
+    internal fun resolveShareMimeType(
+        declaredMimeType: String?,
+        fileExtension: String,
+    ): String =
+        normalizeMimeType(declaredMimeType)
+            ?: mimeTypeMap[fileExtension.lowercase()]
+            // Unreachable today: getMediaExtension only ever returns keys of mimeTypeMap. Kept so
+            // the return type stays a well-formed MIME if that ever stops holding.
+            ?: GENERIC_BINARY_MIME_TYPE
+
    suspend fun getSharableUriFromUrl(
        context: Context,
        imageUrl: String,
@@ -1103,7 +1103,7 @@ private suspend fun shareImageFile(
        val (uri, fileExtension) = ShareHelper.getSharableUriFromUrl(context, videoUri)

        // Determine mime type, use provided or derive from extension
-        val determinedMimeType = mimeType ?: "image/$fileExtension"
+        val determinedMimeType = ShareHelper.resolveShareMimeType(mimeType, fileExtension)

        // Create share intent
        val shareIntent =
@@ -1161,7 +1161,7 @@ private suspend fun shareVideoFile(
            sharedFile = sharableFile

            // Determine mime type
-            val determinedMimeType = mimeType ?: "video/$extension"
+            val determinedMimeType = ShareHelper.resolveShareMimeType(mimeType, extension)

            // Create share intent
            val shareIntent =
@@ -1227,7 +1227,7 @@ private suspend fun shareLocalVideoFile(
            val (uri, extension) = ShareHelper.getSharableUriForLocalVideo(context, localFile)

            // Determine mime type
-            val determinedMimeType = mimeType ?: "video/$extension"
+            val determinedMimeType = ShareHelper.resolveShareMimeType(mimeType, extension)

            // Create share intent
            val shareIntent =
@@ -162,4 +162,44 @@ class ShareHelperTest {
        file.writeBytes(bytes)
        return file
    }
+
+    // A slash-less Intent.type matches no IntentFilter, so the chooser opens with zero targets and
+    // the share silently fails. Author-supplied `m` tags can carry exactly that (Primal iOS emits
+    // `m jpeg`), so a bare subtype must be repaired rather than forwarded.
+    @Test
+    fun resolveShareMimeType_bareSubtype_isExpandedToFullMimeType() {
+        assertEquals("image/jpeg", ShareHelper.resolveShareMimeType("jpeg", "jpg"))
+        assertEquals("video/mp4", ShareHelper.resolveShareMimeType("mp4", "mp4"))
+        assertEquals("image/jpeg", ShareHelper.resolveShareMimeType("JPEG", "jpg"))
+    }
+
+    @Test
+    fun resolveShareMimeType_wellFormedDeclaration_isPreserved() {
+        assertEquals("image/png", ShareHelper.resolveShareMimeType("image/png", "png"))
+    }
+
+    // An unusable declaration falls back to the extension, which is sniffed from the file's magic
+    // numbers and so is not attacker-controlled.
+    @Test
+    fun resolveShareMimeType_unrecognizableDeclaration_fallsBackToSniffedExtension() {
+        assertEquals("image/png", ShareHelper.resolveShareMimeType("notatype", "png"))
+        assertEquals("image/png", ShareHelper.resolveShareMimeType("", "png"))
+    }
+
+    @Test
+    fun resolveShareMimeType_noDeclaration_usesCanonicalTypeForExtension() {
+        // Not "image/jpg" -- jpg is not a registered subtype.
+        assertEquals("image/jpeg", ShareHelper.resolveShareMimeType(null, "jpg"))
+        assertEquals("video/quicktime", ShareHelper.resolveShareMimeType(null, "mov"))
+    }
+
+    // The invariant the share sheet depends on, pinned across the whole set of extensions
+    // getMediaExtension can sniff: whatever the event declared, Intent.type stays matchable.
+    @Test
+    fun resolveShareMimeType_alwaysProducesASlashSeparatedType() {
+        listOf("jpg", "png", "gif", "webp", "webm", "avi", "mp4", "mov").forEach { extension ->
+            val resolved = ShareHelper.resolveShareMimeType("notatype", extension)
+            assertTrue("`$extension` resolved to un-matchable type `$resolved`", resolved.contains("/"))
+        }
+    }
 }
@@ -44,10 +44,18 @@ abstract class MediaUrlContent(
    dim: DimensionTag? = null,
    blurhash: String? = null,
    val uri: String? = null,
-    val mimeType: String? = null,
+    mimeType: String? = null,
    thumbhash: String? = null,
    val authorPubKey: String? = null,
-) : BaseMediaContent(description, dim, blurhash, thumbhash)
+) : BaseMediaContent(description, dim, blurhash, thumbhash) {
+    /**
+     * Repaired at construction rather than at each of the eight call sites that build a model from
+     * an author-supplied `m` tag — a bare subtype reaching this field is what puts `Intent.type =
+     * "jpeg"` on the share sheet (matching no `IntentFilter`) and what republishes the malformed
+     * tag under the user's own key when media is added to a gallery. See [normalizeMimeType].
+     */
+    val mimeType: String? = normalizeMimeType(mimeType)
+}

@Immutable
 open class MediaUrlImage(
@@ -557,10 +557,10 @@ class RichTextParser {
         * Resolves which renderer can display a declared blob — the single decision every media
         * renderer must make, from a NIP-94 `m` tag, a NIP-92 imeta, or a bare URL.
         *
-         * A declared MIME type wins; the URL extension is the fallback both for the no-MIME case
-         * and for a *malformed* MIME (Primal iOS emits `m jpeg` rather than `m image/jpeg`, which
-         * matches no prefix below). `data:` URIs carry their type in the prefix, so a miss there is
-         * genuine and the base64 payload is never extension-probed.
+         * A declared MIME type wins, after [normalizeMimeType] repairs the bare-subtype form some
+         * clients emit; the URL extension is the fallback both for the no-MIME case and for a
+         * declaration too mangled to repair. `data:` URIs carry their type in the prefix, so a miss
+         * there is genuine and the base64 payload is never extension-probed.
         *
         * Returns **null** when nothing can render the file. Callers must not substitute a media
         * kind for that null: handing an arbitrary blob — a webxdc app, a zip, an APK — to the
@@ -570,8 +570,9 @@ class RichTextParser {
         */
        fun classifyMedia(
            url: String,
-            mimeType: String?,
+            rawMimeType: String?,
        ): MediaContentKind? {
+            val mimeType = normalizeMimeType(rawMimeType)
            if (mimeType != null) {
                if (mimeType.startsWith("image/")) return MediaContentKind.IMAGE
                // HLS playlists are advertised with a non-`video/*` MIME; see [isHlsMimeType].
@@ -666,6 +667,9 @@ val mimeTypeMap: Map<String, String> =
        // Video
        "mp4" to "video/mp4",
        "webm" to "video/webm",
+        // Dead entry: "ogg" is re-keyed under Audio below and mapOf keeps the last, so every
+        // lookup of it yields audio/ogg. Kept only to show the extension is genuinely ambiguous —
+        // see [ambiguousMimeSubtypes]. Don't read this line as reachable.
        "ogg" to "video/ogg",
        "mov" to "video/quicktime",
        "avi" to "video/x-msvideo",
@@ -681,3 +685,53 @@ val mimeTypeMap: Map<String, String> =
        // Documents
        "pdf" to "application/pdf",
    )
+
+/**
+ * Subtypes that name more than one top-level type: `mpeg`, `mp4`, `ogg`, `webm` and `3gpp` all exist
+ * as both `audio/` and `video/`, so a bare token spelling one of them identifies no family on its
+ * own. See [normalizeMimeType] for what that costs them.
+ */
+private val ambiguousMimeSubtypes = setOf("mpeg", "mp4", "ogg", "webm", "3gpp")
+
+/**
+ * The subtype half of every MIME in [mimeTypeMap], so a bare token can be looked up as what it
+ * actually is. [mimeTypeMap] is keyed by *extension*, which only doubles as a subtype index where
+ * the two spellings coincide — `quicktime`, `x-matroska` and `svg+xml` are subtypes no extension
+ * spells. Consulted after [mimeTypeMap] so the extension spelling keeps priority where they
+ * disagree (`mp4` stays `video/mp4` rather than the later `audio/mp4` entry).
+ */
+private val mimeSubtypeMap: Map<String, String> = mimeTypeMap.values.associateBy { it.substringAfter('/') }
+
+/**
+ * NIP-92's `m` property is meant to carry a full `type/subtype`, but several clients emit the
+ * bare subtype instead — Primal iOS writes `m jpeg` rather than `m image/jpeg`. That value is
+ * useless as a MIME type: it matches none of the `startsWith("image/")`-style checks, and once
+ * it is stored on the media model it travels all the way into Android's `ACTION_SEND` as
+ * `Intent.type = "jpeg"`. No `<data android:mimeType>` filter matches a type without a slash,
+ * so the share sheet opens with zero targets and the image cannot be shared at all.
+ *
+ * Map a bare subtype back onto its canonical MIME. This is called from the two chokepoints every
+ * `m` value passes through — [RichTextParser.classifyMedia] for the render decision and
+ * [MediaUrlContent] for the value the share intent and the gallery entry's republished `m` tag
+ * read — so consumers see a well-formed type without each having to remember to repair it.
+ *
+ * Anything already containing a `/` is passed through untouched, and an unrecognised bare token is
+ * dropped to null rather than propagated — that leaves the caller's extension-based detection to
+ * decide, which is strictly better than carrying garbage forward.
+ *
+ * The same refusal covers a token in [ambiguousMimeSubtypes] that would land in `audio/`. `audio/`
+ * is the one destructive family: it is what [RichTextParser.isAudioContent] reads to drop the
+ * picture, so guessing it for what may be a video loses content, while guessing `video/` for what
+ * may be audio only costs some chrome. That asymmetry is why the guard is one-sided rather than a
+ * blanket refusal — `mp4` and `webm` resolve to `video/` and keep their rescue, so an extensionless
+ * Blossom URL declaring `m mp4` still renders, whereas `ogg` (whose only live [mimeTypeMap] entry
+ * is `audio/ogg`, its `video/ogg` one being a dead duplicate key) and `mpeg` decline.
+ */
+fun normalizeMimeType(rawMimeType: String?): String? {
+    if (rawMimeType == null) return null
+    if (rawMimeType.contains('/')) return rawMimeType
+    val token = rawMimeType.lowercase()
+    val resolved = mimeTypeMap[token] ?: mimeSubtypeMap[token] ?: return null
+    if (token in ambiguousMimeSubtypes && resolved.startsWith("audio/")) return null
+    return resolved
+}
@@ -23,7 +23,9 @@ package com.vitorpamplona.amethyst.commons.richtext
 import com.vitorpamplona.quartz.nip92IMeta.IMetaTag
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
 import kotlin.test.assertNull
+import kotlin.test.assertTrue

 class ClassifyMediaTest {
    @Test
@@ -97,10 +99,67 @@ class ClassifyMediaTest {
    }

    @Test
-    fun extensionRescuesAMalformedMime() {
-        // Primal iOS emits `m jpeg` instead of `m image/jpeg`; the extension must still win
-        // over "unknown". Preserves the behaviour createMediaContent already documented.
+    fun aMalformedMimeIsRepairedRatherThanIgnored() {
+        // Primal iOS emits `m jpeg` instead of `m image/jpeg`. The bare subtype is mapped back to
+        // its canonical MIME here, so it classifies even on the extensionless URLs Blossom hands
+        // out, where the imeta is the only type signal there is.
        assertEquals(MediaContentKind.IMAGE, RichTextParser.classifyMedia("https://x.com/a.jpg", "jpeg"))
+        assertEquals(MediaContentKind.IMAGE, RichTextParser.classifyMedia("https://x.com/abcd1234", "jpeg"))
+        assertEquals(MediaContentKind.VIDEO, RichTextParser.classifyMedia("https://x.com/abcd1234", "quicktime"))
+        // A token that maps to no known type has nothing to recover from; the extension decides.
+        assertNull(RichTextParser.classifyMedia("https://x.com/abcd1234", "notarealtype"))
+        assertEquals(MediaContentKind.IMAGE, RichTextParser.classifyMedia("https://x.com/a.jpg", "notarealtype"))
+    }
+
+    // A subtype that names more than one top-level type identifies no family on its own. Guessing
+    // one is worse than not repairing: `audio/mpeg` classifies as VIDEO either way, but it also
+    // satisfies isAudioContent, which strips the picture and renders an MPEG video as a bare audio
+    // track. Leaving it unresolved hands the decision back to the extension, which knows.
+    @Test
+    fun anAmbiguousBareSubtypeIsLeftForTheExtensionToDecide() {
+        assertNull(normalizeMimeType("mpeg"), "`mpeg` names both audio/mpeg and video/mpeg")
+        // `ogg` reaches the same guard even though mimeTypeMap answers it: the extension table
+        // holds audio/ogg (its video/ogg entry is a dead duplicate key), so short-circuiting there
+        // is exactly the audio mis-flag this guard exists to stop.
+        assertNull(normalizeMimeType("ogg"), "`ogg` names both audio/ogg and video/ogg")
+
+        val url = "https://x.com/clip.mpg"
+        val media = RichTextParser().createMediaContent(url, mapOf(url to imeta(url, "mpeg")), null)
+
+        assertTrue(media is MediaUrlVideo, "the .mpg extension still classifies it")
+        assertFalse(RichTextParser.isAudioContent(media.mimeType, url), "an MPEG video is not an audio track")
+
+        // The case that actually reached a user: an OGG video must not be flagged as an audio track.
+        val ogv = "https://x.com/clip.ogv"
+        assertFalse(
+            RichTextParser.isAudioContent(normalizeMimeType("ogg"), ogv),
+            "an OGG video must not be rendered as a pictureless audio track",
+        )
+    }
+
+    // Declining is reserved for the destructive direction. `audio/` is the one family that strips
+    // the picture, so an ambiguous token whose extension spelling already resolves to `video/`
+    // keeps its rescue — an extensionless Blossom URL with `m mp4` still renders.
+    @Test
+    fun anAmbiguousSubtypeThatResolvesToVideoKeepsItsRescue() {
+        assertEquals("video/mp4", normalizeMimeType("mp4"))
+        assertEquals("video/webm", normalizeMimeType("webm"))
+        assertEquals(MediaContentKind.VIDEO, RichTextParser.classifyMedia("https://x.com/abcd1234", "mp4"))
+
+        // Unambiguously-audio tokens are untouched by the guard.
+        assertEquals("audio/mpeg", normalizeMimeType("mp3"))
+        assertEquals("audio/flac", normalizeMimeType("flac"))
+    }
+
+    // The unambiguous half must keep working: these are subtypes no extension in the table spells,
+    // so they resolve only through the subtype index.
+    @Test
+    fun anUnambiguousBareSubtypeStillResolves() {
+        assertEquals("video/quicktime", normalizeMimeType("quicktime"))
+        assertEquals("video/x-matroska", normalizeMimeType("x-matroska"))
+        assertEquals("image/svg+xml", normalizeMimeType("svg+xml"))
+        // An extension spelling that is also a subtype keeps the extension's family.
+        assertEquals("video/mp4", normalizeMimeType("mp4"))
    }

    @Test
@@ -135,6 +194,9 @@ class ClassifyMediaTest {
                "https://x.com/a.xdc" to "application/x-webxdc",
                "https://x.com/a.zip" to "application/zip",
                "https://x.com/a.jpg" to "jpeg",
+                "https://x.com/abcd1234" to "jpeg",
+                "https://x.com/abcd1234" to "notarealtype",
+                "https://x.com/clip.mpg" to "mpeg",
                "data:image/png;base64,AAAA" to null,
                "data:application/zip;base64,AAAAmp4" to null,
            )
@@ -67,11 +67,10 @@ class PdfParserTest {
        assertTrue(RichTextParser.isPdfUrl("https://example.com/doc.pdf?sig=abc"))
    }

-    // Primal iOS writes a bare subtype (`m jpeg`) instead of a full MIME (`m image/jpeg`).
-    // The bare subtype matches none of the `image/`/`video/`/`application/pdf` prefixes, so before
-    // the extension fallback the whole imeta was dropped: the URL rendered as a plain link, losing
-    // the `dim` needed to reserve the image's height (feed jump) and forcing a URL-preview fetch.
-    // The `.jpg` extension must still route it to a MediaUrlImage that carries `dim`.
+    // Primal iOS writes a bare subtype (`m jpeg`) instead of a full MIME (`m image/jpeg`); see
+    // normalizeMimeType. End-to-end here because the failure was never just the render decision:
+    // dropping the imeta also lost the `dim` that reserves the image's height (feed jump) and
+    // forced a URL-preview fetch to rediscover a type the imeta had already declared.
    @Test
    fun detectsImageFromMalformedImetaMimeWithImageExtension() {
        val url = "https://blossom.primal.net/33e7c01afbea894a64e1db44dece460b09a2426108f47143754e1cf4bfdf747c.jpg"
@@ -88,6 +87,9 @@ class PdfParserTest {

        val imageMedia = state.mediaForPager[url]
        assertTrue(imageMedia is MediaUrlImage, "Expected MediaUrlImage despite the malformed `m jpeg` mime")
+        // Repaired on the model too, not just for the render decision: this field becomes
+        // Intent.type when the image is shared, and a slash-less one matches no IntentFilter.
+        assertEquals("image/jpeg", imageMedia.mimeType, "The bare subtype must not survive onto the model")
        assertEquals("1009x680", imageMedia.dim?.toString(), "The imeta dim must survive so the loader can reserve space")
        assertEquals(1009f / 680f, imageMedia.dim?.aspectRatio())

@@ -112,20 +114,20 @@ class PdfParserTest {
        assertTrue(videoMedia is MediaUrlVideo, "Expected MediaUrlVideo despite the malformed `m mp4` mime")
    }

-    // A malformed mime on a URL with *no* recognizable extension can't be recovered — it stays a
-    // link. This documents the boundary of the fallback so it isn't mistaken for a regression.
+    // An extensionless URL is the shape Blossom hands out, where the imeta is the only type signal
+    // there is: a recognizable bare subtype now carries it, and an unrecognizable one leaves
+    // nothing to recover from. The second half documents the limit so it isn't read as a
+    // regression.
    @Test
-    fun malformedImetaMimeWithoutExtensionStaysUnclassified() {
+    fun malformedImetaMimeWithoutExtensionIsRecoveredFromTheMimeAlone() {
        val url = "https://files.example.com/abcd1234"
-        val tags =
-            ImmutableListOfLists(
-                arrayOf(
-                    arrayOf("imeta", "url $url", "m jpeg"),
-                ),
-            )

-        val state = RichTextParser().parseText(url, tags, null)
+        fun parse(mime: String) =
+            RichTextParser()
+                .parseText(url, ImmutableListOfLists(arrayOf(arrayOf("imeta", "url $url", "m $mime"))), null)
+                .mediaForPager[url]

-        assertEquals(null, state.mediaForPager[url], "No extension to recover from -> not treated as media")
+        assertTrue(parse("jpeg") is MediaUrlImage, "`m jpeg` alone is enough to classify the media")
+        assertEquals(null, parse("notarealtype"), "Nothing to recover from -> not treated as media")
    }
 }