diff options
| author | Claude <noreply@anthropic.com> | 2026-10-08 07:23:40 +0000 |
|---|---|---|
| committer | Claude <noreply@anthropic.com> | 2026-10-08 07:23:40 +0000 |
| commit | bf26b97b483bfba52d09adeb6516563830164f62 (patch) | |
| tree | e4373225534176663e0552b46abd19f0ed55f73e /app/src | |
| parent | 7f0601e42b9df979dab2a780ebaf1a19358bf072 (diff) | |
Play episodes whose enclosure is still cleartext http
Reported from the phone: Show 62 of Hardcore History failed with
ERROR_CODE_IO_CLEARTEXT_NOT_PERMITTED. Android has refused cleartext
since API 28 and this app declares no exemption, so any enclosure
published over http fails the moment it is streamed rather than played
from a downloaded file. Dan Carlin's older episodes are published that
way while the recent ones are not, which is why it went unnoticed; the
manifest has not changed since long before the skip server, so this is
not a regression from that work.
Upgrading the scheme rather than permitting cleartext: that exact URL
answers 200 over TLS and a range request returns 206 audio/mpeg, podcast
CDNs serve the same path either way, and an exemption would weaken every
other request the app makes for the sake of a few old enclosures. A host
with genuinely no TLS is no worse off, since cleartext was refused
already.
Two paths, because they do not share a stack. Everything that goes
through OkHttp -- downloads, feeds, chapters, artwork -- is covered by an
interceptor on the one client the graph builds, which is the right home
for a property of the app's networking rather than of any one feature.
ExoPlayer has its own data source, so playback converts the URI as it
builds the media item.
Putting the conversion at the downloader's call site instead broke four
of its MockWebServer tests, which was the tests making a fair point: a
global network policy does not belong at one call site. Components under
test build their own client and are untouched by the interceptor.
Also stops the server starving on-demand episode requests. The poll loop
and the request queue share a goroutine, so an episode the phone asked
for waited out the whole cycle -- hours across 42 shows, which is why the
tap that produced this report registered the episode and never fetched
it. Now drained before each feed, bounding the wait at one feed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
Diffstat (limited to 'app/src')
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"), + ) + } +} |
