diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/hls/HlsUploadPipeline.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/hls/HlsUploadPipeline.kt index 6946dca48..86783a93a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/hls/HlsUploadPipeline.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/hls/HlsUploadPipeline.kt @@ -58,6 +58,12 @@ data class HlsUploadedRendition( * Finally rewrites the master playlist to reference the per-rendition playlist URLs and uploads * the master. The resulting [HlsUploadResult] is what the publisher uses to build the NIP-71 * event. + * + * URL handling policy: the pipeline uses the URL the server returned verbatim. No extension is + * appended, no trailing dot stripped, no bare-hash rewriting. The server is responsible for + * returning a URL that the player can fetch as-is — that way we get the best cache coherence, + * correct Content-Type, clean range requests, and no double round trips. If a server returns an + * unplayable URL, the fix is server-side. */ class HlsUploadPipeline( private val uploader: HlsBlobUploader, @@ -75,10 +81,7 @@ class HlsUploadPipeline( val combined = uploader.upload(rendition.combinedFile, CONTENT_TYPE_VIDEO_MP4) onProgress(++done, total) val combinedUrl = - withExtensionHint( - combined.url ?: error("Uploader returned null URL for ${rendition.combinedFile.name}"), - CONTENT_TYPE_VIDEO_MP4, - ) + combined.url ?: error("Uploader returned null URL for ${rendition.combinedFile.name}") val rewrittenMedia = HlsPlaylistRewriter.rewrite( @@ -90,10 +93,7 @@ class HlsUploadPipeline( val mediaPlaylist = uploader.upload(mediaPlaylistFile, CONTENT_TYPE_HLS) onProgress(++done, total) val mediaPlaylistUrl = - withExtensionHint( - mediaPlaylist.url ?: error("Uploader returned null URL for media playlist ${rendition.label}"), - CONTENT_TYPE_HLS, - ) + mediaPlaylist.url ?: error("Uploader returned null URL for media playlist ${rendition.label}") HlsUploadedRendition( label = rendition.label, @@ -112,10 +112,7 @@ class HlsUploadPipeline( val master = uploader.upload(masterFile, CONTENT_TYPE_HLS) onProgress(++done, total) val masterUrl = - withExtensionHint( - master.url ?: error("Uploader returned null URL for master playlist"), - CONTENT_TYPE_HLS, - ) + master.url ?: error("Uploader returned null URL for master playlist") return HlsUploadResult( masterUrl = masterUrl, @@ -124,28 +121,6 @@ class HlsUploadPipeline( ) } - // Blossom servers typically return bare-hash URLs (https://server/), but HLS parsers - // and ExoPlayer's Util.inferContentType sniff the URL extension to pick the right source - // factory. Append a hint unless the upload server already baked one in. Trailing dots are - // trimmed and any pre-existing doubled extension (e.g. "..m3u8") collapsed, so a server that - // echoes our empty-extension upload filename does not produce unreachable URLs. - private fun withExtensionHint( - url: String, - contentType: String, - ): String { - val ext = - when (contentType) { - CONTENT_TYPE_VIDEO_MP4 -> ".mp4" - CONTENT_TYPE_HLS -> ".m3u8" - else -> return url - } - val sanitised = - url - .replace(Regex("""\.{2,}""" + Regex.escape(ext.trimStart('.')) + "$", RegexOption.IGNORE_CASE), ext) - .trimEnd('.') - return if (sanitised.endsWith(ext, ignoreCase = true)) sanitised else sanitised + ext - } - companion object { const val CONTENT_TYPE_VIDEO_MP4 = "video/mp4" const val CONTENT_TYPE_HLS = "application/vnd.apple.mpegurl" diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/uploads/hls/HlsUploadPipelineTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/uploads/hls/HlsUploadPipelineTest.kt index edac38b65..75b804855 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/uploads/hls/HlsUploadPipelineTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/uploads/hls/HlsUploadPipelineTest.kt @@ -180,36 +180,25 @@ class HlsUploadPipelineTest { } @Test - fun appendsMp4HintToBareCombinedUrlInMediaPlaylist() { - val bundle = createBundle(listOf("360p")) - val uploader = BareUrlUploader() - val pipeline = HlsUploadPipeline(uploader) - - runBlocking { pipeline.upload(bundle) } - - val uploadedMediaPlaylist = uploader.calls[1].third - assertTrue( - "media playlist should reference url with .mp4 hint", - uploadedMediaPlaylist.contains("https://blossom.test/bare-1.mp4"), - ) - assertTrue(!uploadedMediaPlaylist.contains("https://blossom.test/bare-1\"")) - } - - @Test - fun appendsM3u8HintToBarePlaylistUrlInMasterPlaylist() { + fun bareServerUrlsPassThroughVerbatim() { + // Policy: the pipeline uses whatever URL the server returned, unchanged. + // Even a bare-hash URL with no extension flows straight into the rewritten + // playlists. If it does not play, the fix is server-side (return a playable URL). val bundle = createBundle(listOf("360p")) val uploader = BareUrlUploader() val pipeline = HlsUploadPipeline(uploader) val result = runBlocking { pipeline.upload(bundle) } - val uploadedMaster = uploader.calls[2].third + assertEquals("https://blossom.test/bare-1", result.renditions[0].combinedUrl) + assertEquals("https://blossom.test/bare-2", result.renditions[0].playlistUrl) + assertEquals("https://blossom.test/bare-3", result.masterUrl) + // The rewritten media playlist that the server received must contain the bare url. + val uploadedMediaPlaylist = uploader.calls[1].third assertTrue( - "master playlist should reference playlist url with .m3u8 hint", - uploadedMaster.contains("https://blossom.test/bare-2.m3u8"), + "media playlist should reference bare url: $uploadedMediaPlaylist", + uploadedMediaPlaylist.contains("https://blossom.test/bare-1"), ) - assertEquals("https://blossom.test/bare-3.m3u8", result.masterUrl) - assertEquals("https://blossom.test/bare-1.mp4", result.renditions[0].combinedUrl) } @Test @@ -252,46 +241,6 @@ class HlsUploadPipelineTest { assertTrue(!result.renditions[0].playlistUrl.contains(".m3u8.m3u8")) } - @Test - fun trailingDotUrlsAreSanitisedIntoCleanExtension() { - val bundle = createBundle(listOf("360p")) - val uploader = - object : HlsBlobUploader { - var count = 0 - - override suspend fun upload( - file: File, - contentType: String, - ): MediaUploadResult { - count++ - return MediaUploadResult(url = "https://blossom.test/bare-$count.", sha256 = "sha-$count", size = file.length()) - } - } - val pipeline = HlsUploadPipeline(uploader) - - val result = runBlocking { pipeline.upload(bundle) } - - assertTrue("masterUrl must not contain '..': ${result.masterUrl}", !result.masterUrl.contains("..")) - assertTrue("masterUrl must not end with '.': ${result.masterUrl}", !result.masterUrl.endsWith(".")) - assertTrue(result.masterUrl.endsWith(".m3u8")) - assertTrue(!result.renditions[0].combinedUrl.contains("..")) - assertTrue(result.renditions[0].combinedUrl.endsWith(".mp4")) - } - - @Test - fun doesNotDoubleAppendExtensionWhenAlreadyPresent() { - val bundle = createBundle(listOf("360p")) - val uploader = FakeUploader() // returns urls ending in .mp4 / .m3u8 - val pipeline = HlsUploadPipeline(uploader) - - runBlocking { pipeline.upload(bundle) } - - val uploadedMediaPlaylist = uploader.calls[1].content - assertTrue(!uploadedMediaPlaylist.contains(".mp4.mp4")) - val uploadedMaster = uploader.calls[2].content - assertTrue(!uploadedMaster.contains(".m3u8.m3u8")) - } - @Test fun reportsUploadProgressPerStep() { val bundle = createBundle(listOf("360p", "540p"))