diff options
Diffstat (limited to 'app')
6 files changed, 203 insertions, 1 deletions
diff --git a/app/src/main/java/org/terst/listen/data/MediaUrl.kt b/app/src/main/java/org/terst/listen/data/MediaUrl.kt new file mode 100644 index 0000000..2269888 --- /dev/null +++ b/app/src/main/java/org/terst/listen/data/MediaUrl.kt @@ -0,0 +1,23 @@ +package org.terst.listen.data + +/** + * The URL a remote episode is actually opened with. + * + * Android refuses cleartext HTTP by default from API 28 onward and this app declares no + * exemption, so an `http://` enclosure fails with ERROR_CODE_IO_CLEARTEXT_NOT_PERMITTED as soon + * as it is streamed rather than downloaded. Older podcast episodes are still published that way. + * + * Upgrading the scheme is the right fix rather than permitting cleartext. Podcast CDNs serve the + * same path over TLS, and an exemption would weaken every other request the app makes for the + * sake of a handful of old enclosures. A host that genuinely has no TLS ends up no worse off, + * since cleartext was refused already. + */ +object MediaUrl { + + private const val CLEARTEXT = "http://" + private const val SECURE = "https://" + + /** The same URL over TLS, if it was a cleartext one. Anything else is returned unchanged. */ + fun secured(url: String): String = + if (url.startsWith(CLEARTEXT, ignoreCase = true)) SECURE + url.substring(CLEARTEXT.length) else url +} diff --git a/app/src/main/java/org/terst/listen/data/Repositories.kt b/app/src/main/java/org/terst/listen/data/Repositories.kt index 07ffab2..f7333f0 100644 --- a/app/src/main/java/org/terst/listen/data/Repositories.kt +++ b/app/src/main/java/org/terst/listen/data/Repositories.kt @@ -46,6 +46,8 @@ class AppGraph(private val context: Context) { val client: OkHttpClient by lazy { OkHttpClient.Builder() + // Android refuses cleartext, and older episodes are still published over http. + .addInterceptor(SecureSchemeInterceptor) // Generous: an episode is tens of megabytes and a phone's connection comes and goes. .callTimeout(0, TimeUnit.MILLISECONDS) .connectTimeout(CONNECT_TIMEOUT_SECONDS, TimeUnit.SECONDS) diff --git a/app/src/main/java/org/terst/listen/data/SecureSchemeInterceptor.kt b/app/src/main/java/org/terst/listen/data/SecureSchemeInterceptor.kt new file mode 100644 index 0000000..32a452c --- /dev/null +++ b/app/src/main/java/org/terst/listen/data/SecureSchemeInterceptor.kt @@ -0,0 +1,28 @@ +package org.terst.listen.data + +import okhttp3.Interceptor +import okhttp3.Response + +/** + * Sends every request over TLS, upgrading the scheme of any that was not. + * + * Android has refused cleartext HTTP by default since API 28 and this app declares no exemption, + * so an `http://` URL fails as ERROR_CODE_IO_CLEARTEXT_NOT_PERMITTED. Older podcast episodes are + * still published that way -- Dan Carlin's Show 62 is `http://dts.podtrac.com/...` while the + * recent ones are not, which is why it went unnoticed until an old episode was played. + * + * Here rather than at the call sites because it is a property of the app's networking rather + * than of any one feature: downloads, feeds, chapters and artwork share this client, and a rule + * each of them has to remember is a rule one of them will forget. Upgrading beats declaring a + * cleartext exemption, which would weaken every request for the sake of a few old enclosures, + * and a host with no TLS at all is no worse off because cleartext was refused already. + */ +object SecureSchemeInterceptor : Interceptor { + + override fun intercept(chain: Interceptor.Chain): Response { + val request = chain.request() + if (request.url.isHttps) return chain.proceed(request) + val secured = request.newBuilder().url(request.url.newBuilder().scheme("https").build()).build() + return chain.proceed(secured) + } +} diff --git a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt index 3a10e4e..e2ca3a8 100644 --- a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt +++ b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt @@ -20,6 +20,7 @@ import kotlinx.coroutines.launch import org.terst.listen.data.AppDatabase import org.terst.listen.data.Artwork import org.terst.listen.data.EpisodeEntity +import org.terst.listen.data.MediaUrl import org.terst.listen.data.Graph /** @@ -292,7 +293,10 @@ fun EpisodeEntity.toMediaItem( MediaItem.Builder() // The episode id, so whatever comes back from the player can be tied to a row. .setMediaId(id) - .setUri(localPath ?: mediaUrl) + // A downloaded file is a path and is used as it stands; a remote enclosure is upgraded + // to TLS, because Android refuses cleartext and older episodes are still published over + // http -- which fails as ERROR_CODE_IO_CLEARTEXT_NOT_PERMITTED the moment it is streamed. + .setUri(localPath ?: MediaUrl.secured(mediaUrl)) .setMimeType(mediaType) .setMediaMetadata( androidx.media3.common.MediaMetadata.Builder() diff --git a/app/src/test/java/org/terst/listen/data/MediaUrlTest.kt b/app/src/test/java/org/terst/listen/data/MediaUrlTest.kt new file mode 100644 index 0000000..8b0be16 --- /dev/null +++ b/app/src/test/java/org/terst/listen/data/MediaUrlTest.kt @@ -0,0 +1,76 @@ +package org.terst.listen.data + +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * Streaming an episode whose enclosure is cleartext. + * + * Android has blocked cleartext HTTP by default since API 28, and the app declares no exemption, + * so an `http://` enclosure fails with ERROR_CODE_IO_CLEARTEXT_NOT_PERMITTED the moment it is + * played without being downloaded first. Dan Carlin's older episodes are published that way -- + * `http://dts.podtrac.com/...` -- while the recent ones are not, which is why this went unnoticed. + * + * Upgrading the scheme is the fix rather than permitting cleartext: podcast CDNs serve the same + * path over TLS (that exact URL answers 200 and a range request returns 206 audio/mpeg), and an + * exemption would weaken every other request the app makes. A host that genuinely has no TLS is + * no worse off, because cleartext was already refused. + */ +class MediaUrlTest { + + @Test + fun `upgrades a cleartext url so playback is allowed to open it`() { + assertEquals( + "https://dts.podtrac.com/redirect.mp3/traffic.libsyn.com/dancarlinhh/dchha62.mp3", + MediaUrl.secured("http://dts.podtrac.com/redirect.mp3/traffic.libsyn.com/dancarlinhh/dchha62.mp3"), + ) + } + + @Test + fun `leaves a url that is already secure alone`() { + val url = "https://cdn.example/episode.mp3" + assertEquals(url, MediaUrl.secured(url)) + } + + /** Feeds are hand-written often enough that the scheme's case cannot be relied on. */ + @Test + fun `upgrades regardless of how the scheme is capitalised`() { + assertEquals("https://cdn.example/a.mp3", MediaUrl.secured("HTTP://cdn.example/a.mp3")) + assertEquals("https://cdn.example/b.mp3", MediaUrl.secured("Http://cdn.example/b.mp3")) + } + + /** + * A downloaded episode is played from a path, not a URL, and a local file has no scheme to + * upgrade. Rewriting one would point the player at nothing. + */ + @Test + fun `leaves a local file path untouched`() { + val path = "/data/user/0/org.terst.listen/files/library/abc.mp3" + assertEquals(path, MediaUrl.secured(path)) + assertEquals("file:///tmp/a.mp3", MediaUrl.secured("file:///tmp/a.mp3")) + assertEquals("content://media/external/audio/1", MediaUrl.secured("content://media/external/audio/1")) + } + + /** Only the scheme changes: a query string carrying a signature must survive intact. */ + @Test + fun `changes nothing but the scheme`() { + assertEquals( + "https://cdn.example/a.mp3?Signature=ab%2Fcd&Expires=1791465291", + MediaUrl.secured("http://cdn.example/a.mp3?Signature=ab%2Fcd&Expires=1791465291"), + ) + } + + /** A host that happens to contain the scheme as text must not be mangled. */ + @Test + fun `only rewrites the scheme at the front of the url`() { + assertEquals( + "https://cdn.example/redirect?to=http://elsewhere/a.mp3", + MediaUrl.secured("http://cdn.example/redirect?to=http://elsewhere/a.mp3"), + ) + } + + @Test + fun `an empty url is left as it is`() { + assertEquals("", MediaUrl.secured("")) + } +} diff --git a/app/src/test/java/org/terst/listen/data/SecureSchemeInterceptorTest.kt b/app/src/test/java/org/terst/listen/data/SecureSchemeInterceptorTest.kt new file mode 100644 index 0000000..3bb23f9 --- /dev/null +++ b/app/src/test/java/org/terst/listen/data/SecureSchemeInterceptorTest.kt @@ -0,0 +1,69 @@ +package org.terst.listen.data + +import okhttp3.Interceptor +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * Cleartext is refused by Android from API 28 and this app declares no exemption, so every + * remote fetch it makes has to be over TLS -- not only the ones somebody remembered to convert. + * + * An interceptor rather than a call site, because it is a property of the app's networking and + * not of any one feature: downloads, feeds, chapters and artwork all go through the same client. + * Components under test build their own client and are unaffected, which is why this does not + * break the MockWebServer suites. + */ +class SecureSchemeInterceptorTest { + + /** Records what actually went out, and answers it, so no socket is involved. */ + private class Recorder : Interceptor { + val seen = mutableListOf<String>() + override fun intercept(chain: Interceptor.Chain): Response { + seen += chain.request().url.toString() + return Response.Builder() + .request(chain.request()) + .protocol(Protocol.HTTP_1_1) + .code(200) + .message("OK") + .body("".toResponseBody()) + .build() + } + } + + private fun fetch(url: String): List<String> { + val recorder = Recorder() + val client = OkHttpClient.Builder() + .addInterceptor(SecureSchemeInterceptor) + .addInterceptor(recorder) + .build() + client.newCall(Request.Builder().url(url).build()).execute().close() + return recorder.seen + } + + @Test + fun `a cleartext request goes out over tls instead`() { + assertEquals( + listOf("https://dts.podtrac.com/redirect.mp3/traffic.libsyn.com/dchha62.mp3"), + fetch("http://dts.podtrac.com/redirect.mp3/traffic.libsyn.com/dchha62.mp3"), + ) + } + + @Test + fun `a request that is already secure is sent unchanged`() { + assertEquals(listOf("https://cdn.example/a.mp3"), fetch("https://cdn.example/a.mp3")) + } + + /** The signature on a CDN URL is part of what is fetched; only the scheme may change. */ + @Test + fun `the path and query survive the upgrade`() { + assertEquals( + listOf("https://cdn.example/a.mp3?Signature=ab%2Fcd&Expires=1791465291"), + fetch("http://cdn.example/a.mp3?Signature=ab%2Fcd&Expires=1791465291"), + ) + } +} |