From da48109a4d1f8dd7c7195d6c70e28cd1843f2d70 Mon Sep 17 00:00:00 2001 From: Kayos Date: Mon, 25 May 2026 15:12:30 -0700 Subject: [PATCH] =?UTF-8?q?vc=3D40:=20loop=20round=202/5=20=E2=80=94=20rou?= =?UTF-8?q?nd-1=20misses=20+=20new=20HIGHs=20from=20round-5=20audits?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three parallel Opus round-5 audits ran on vc=39. The big finds were regressions on vc=39's own fixes — the retry-init wiring was incomplete, the URL-fence pattern missed CancellationException swallowing, and the SettingsStore idempotency branch was dead code. Real round-5 work. HIGH R5-1 Rust `ensure_initialized` was only called from `init_logging` — extractor entry points never invoked it, so the 5s-backoff retry from vc=39 was unreachable. A cold-boot init failure still bricked extraction for the whole process. Now every `search()` / `stream_info()` / `channel_info()` calls ensure_initialized at entry; cheap when INITIALIZED is true. R5-2 runtime.rs in-progress race: prior shape stamped LAST_ATTEMPT_MS at the *start* of an attempt, then let concurrent callers short-circuit via the backoff check and proceed to call into the extractor before init completed. New: lock FIRST (mutex IS the in-progress queue), re-check INITIALIZED under lock, only THEN check backoff (and only stamp it on failure). R5-3 `runCatching` in VideoDetailViewModel + SubscriptionFeed swallows CancellationException — when a job was cancelled mid-suspend inside channelInfo/RYD/SB, the inner cancellation was eaten, the lambda returned its default, the job carried past the runCatching to its terminal write, and the URL fence let it through because same-URL races can't be distinguished by string equality. New util.runCatchingCancellable re-throws CancellationException; all coroutine-body runCatching sites in the affected VMs migrated. R5-4 SearchViewModel.submit post-network fence only guarded `_ui.update`. SearchCache.record + pool rebuild proceeded for a cancelled query → ghost cache entries for queries the user abandoned mid-stroke. Now: re-check the query AFTER the IO suspend and before the cache write. R5-5 ChannelViewModel.load / VideoDetailViewModel.load now gate the extractor entry on isAllowedYtUrl(channelUrl/streamUrl). Prior shape only gated recordWatch persistence — extractor invocation for poisoned uploaderUrl still happened. MED R5-6 SettingsStore.set{MaxResolution,ThemeMode}: vc=39 used `updateAndGet { r } == r` which is unconditionally true (lambda ignores prior) — the in-memory equality check was dead code. SP-side check still gated the disk write so the feature worked, but the dead branch was a comment-vs-code liar. Rewrote with explicit before-capture + equality gate. R5-7 SponsorBlockSkipLoop polled `controller.currentPosition` every 150ms even when paused — paused-overnight playback ate ~24k binder calls/hour. Now: when `!isPlaying`, sleep 1s and continue. R5-8 StrawApp.appScope had no CoroutineExceptionHandler — an uncaught throwable in a top-level launch could crash on cold start even with SupervisorJob (top-level failure still propagates to default handler). Added handler that logs via strawLogW. R5-9 YtUrl.isAllowedYtUrl now requires http/https scheme (schemeless `//host/...` URLs no longer pass) and strips a single trailing dot from host (RFC FQDN form). Defense in depth. LOW R5-10 NowPlaying.set() removed — non-CAS setter footgun alongside the race-free claim()/CAS path. No external callers (grep clean). Doc-comment updated. Deferred (later round / different scope): - PlaylistsStore URL canonicalization (round-5 MED-1 — needs a shared YT-id-extract util; not blocking). - Release R8 + Nav rememberSaveable (still deferred). - LazyColumn keys + collectAsStateWithLifecycle (cosmetic). - SponsorBlockSkipLoop currentMediaItem binding (round-5 deferred). --- buildSrc/src/main/kotlin/ProjectConfig.kt | 4 +- rust/strawcore/src/channel.rs | 1 + rust/strawcore/src/runtime.rs | 40 ++++++++++++------- rust/strawcore/src/search.rs | 5 +++ rust/strawcore/src/stream.rs | 1 + .../main/kotlin/com/sulkta/straw/StrawApp.kt | 13 +++++- .../com/sulkta/straw/data/SettingsStore.kt | 23 ++++++----- .../straw/feature/channel/ChannelViewModel.kt | 9 +++++ .../feature/detail/VideoDetailViewModel.kt | 16 ++++++-- .../feature/feed/SubscriptionFeedViewModel.kt | 5 ++- .../sulkta/straw/feature/player/NowPlaying.kt | 10 ++--- .../straw/feature/player/PlayerScreen.kt | 8 ++++ .../straw/feature/search/SearchViewModel.kt | 14 +++++-- .../com/sulkta/straw/util/Coroutines.kt | 28 +++++++++++++ .../kotlin/com/sulkta/straw/util/YtUrl.kt | 9 ++++- 15 files changed, 141 insertions(+), 45 deletions(-) create mode 100644 strawApp/src/main/kotlin/com/sulkta/straw/util/Coroutines.kt diff --git a/buildSrc/src/main/kotlin/ProjectConfig.kt b/buildSrc/src/main/kotlin/ProjectConfig.kt index 926d3e84f..e7ae39d83 100644 --- a/buildSrc/src/main/kotlin/ProjectConfig.kt +++ b/buildSrc/src/main/kotlin/ProjectConfig.kt @@ -55,6 +55,6 @@ const val NEWPIPE_APPLICATION_ID_NEW = "net.newpipe.app" // vc=19 / 0.1.0-AE — rust pipeline cutover. Extraction via // strawcore-core (Sulkta-Coop/strawcore) via the UniFFI wrapper; no // NewPipeExtractor in the runtime path. -const val STRAW_VERSION_CODE = 39 -const val STRAW_VERSION_NAME = "0.1.0-AY" +const val STRAW_VERSION_CODE = 40 +const val STRAW_VERSION_NAME = "0.1.0-AZ" const val STRAW_APPLICATION_ID = "com.sulkta.straw" diff --git a/rust/strawcore/src/channel.rs b/rust/strawcore/src/channel.rs index 3434905e5..ec99e1416 100644 --- a/rust/strawcore/src/channel.rs +++ b/rust/strawcore/src/channel.rs @@ -24,6 +24,7 @@ pub struct ChannelInfo { #[uniffi::export(async_runtime = "tokio")] pub async fn channel_info(input: String) -> Result { log::info!("strawcore::channel_info input_len={}", input.len()); + crate::runtime::ensure_initialized(); let identifier = resolve_channel_identifier(&input)?; let core = tokio::task::spawn_blocking(move || core_channel_info(identifier)) .await diff --git a/rust/strawcore/src/runtime.rs b/rust/strawcore/src/runtime.rs index 258e30935..bc02f3879 100644 --- a/rust/strawcore/src/runtime.rs +++ b/rust/strawcore/src/runtime.rs @@ -43,26 +43,32 @@ fn now_ms() -> u64 { } pub fn ensure_initialized() { + // Fast path: already initialized. Just an Acquire load. if INITIALIZED.load(Ordering::Acquire) { return; } - // Backoff check OUTSIDE the lock — avoids serializing every - // already-throttled caller on a single mutex. + // Acquire the lock FIRST. The mutex is the in-progress queue — + // concurrent callers wait here while one thread does init. + // Round-5 audit HIGH-1: the prior shape stamped LAST_ATTEMPT_MS + // at the start of an attempt then let concurrent callers + // short-circuit-out via the backoff check; they'd then call into + // the extractor before init completed → DownloaderMissing. + let _guard = match INIT_LOCK.lock() { + Ok(g) => g, + Err(p) => p.into_inner(), + }; + // Re-check under the lock — another thread may have just succeeded. + if INITIALIZED.load(Ordering::Acquire) { + return; + } + // Now the backoff check — but ONLY skips when a *prior* failed + // attempt is still in cooldown. If LAST_ATTEMPT_MS is zero, no + // attempt has been made yet; the lock fall-through proceeds. let last = LAST_ATTEMPT_MS.load(Ordering::Acquire); let now = now_ms(); if last != 0 && now.saturating_sub(last) < RETRY_BACKOFF_MS { return; } - let _guard = match INIT_LOCK.lock() { - Ok(g) => g, - Err(p) => p.into_inner(), - }; - // Re-check under the lock — another thread may have just - // succeeded while we were waiting. - if INITIALIZED.load(Ordering::Acquire) { - return; - } - LAST_ATTEMPT_MS.store(now_ms(), Ordering::Release); match ReqwestDownloader::new() { Ok(dl) => { NewPipe::init_full( @@ -71,11 +77,17 @@ pub fn ensure_initialized() { ContentCountry::default(), ); INITIALIZED.store(true, Ordering::Release); + // Clear LAST_ATTEMPT_MS so a future hypothetical + // re-init path (none today) wouldn't see cooldown + // bleed from this success. + LAST_ATTEMPT_MS.store(0, Ordering::Release); log::info!("strawcore-core: downloader + localization initialized"); } Err(e) => { - // Don't surface the underlying error string verbatim — - // it can embed URLs / hosts. + // Stamp the timestamp on FAILURE only, so the next + // caller within RETRY_BACKOFF_MS skips, but a successful + // attempt isn't reflected in the backoff state. + LAST_ATTEMPT_MS.store(now, Ordering::Release); log::error!("strawcore-core: downloader init failed (will retry on next call)"); let _ = e; } diff --git a/rust/strawcore/src/search.rs b/rust/strawcore/src/search.rs index 0a693ff61..056af6977 100644 --- a/rust/strawcore/src/search.rs +++ b/rust/strawcore/src/search.rs @@ -60,6 +60,11 @@ pub async fn search(query: String) -> Result, StrawcoreError> { // Settings → Export Logs path straight into a user's chat. Log // shape, not content. vc=36 audit CVE HIGH-2. log::info!("strawcore::search query_len={}", query.len()); + // Round-5 audit MED-1: ensure_initialized was only wired into + // init_logging() so the 5s-backoff retry path never fired from + // the hot entry points. Now every extractor entry re-asserts + // — cheap when INITIALIZED is true (single Acquire load). + crate::runtime::ensure_initialized(); let result = tokio::task::spawn_blocking(move || { search_extractor::search(&query, SearchFilter::Videos) }) diff --git a/rust/strawcore/src/stream.rs b/rust/strawcore/src/stream.rs index 73da83e58..f28080f9c 100644 --- a/rust/strawcore/src/stream.rs +++ b/rust/strawcore/src/stream.rs @@ -58,6 +58,7 @@ pub struct AudioStreamItem { #[uniffi::export(async_runtime = "tokio")] pub async fn stream_info(input: String) -> Result { log::info!("strawcore::stream_info input_len={}", input.len()); + crate::runtime::ensure_initialized(); let video_id = resolve_video_id(&input)?; let video_id_for_call = video_id.clone(); let core = tokio::task::spawn_blocking(move || core_stream_info(&video_id_for_call)) diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/StrawApp.kt b/strawApp/src/main/kotlin/com/sulkta/straw/StrawApp.kt index 2d867add7..6634cee17 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/StrawApp.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/StrawApp.kt @@ -13,6 +13,8 @@ import com.sulkta.straw.data.SearchCache import com.sulkta.straw.data.Settings import com.sulkta.straw.data.Subscriptions import com.sulkta.straw.feature.dataimport.SettingsImport +import com.sulkta.straw.util.strawLogW +import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -22,9 +24,16 @@ class StrawApp : Application() { /** * App-scoped coroutine scope for one-time startup work that * shouldn't tie up Application.onCreate. SupervisorJob so a failure - * in one launch doesn't cascade. + * in one launch doesn't cascade. CoroutineExceptionHandler so an + * uncaught throwable in a top-level launch doesn't crash the + * process on cold start (would otherwise hit the default handler + * even with SupervisorJob). Round-5 audit MED-3. */ - private val appScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + private val appScope = CoroutineScope( + SupervisorJob() + Dispatchers.IO + CoroutineExceptionHandler { _, t -> + strawLogW("StrawApp") { "appScope uncaught: ${t.javaClass.simpleName}: ${t.message}" } + }, + ) override fun onCreate() { super.onCreate() diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt index f5e1abe5f..6f01965ac 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt @@ -72,28 +72,29 @@ class SettingsStore(context: Context) { sp.edit().putStringSet(KEY_SB_CATS, next.map { it.key }.toSet()).apply() } - // Atomic + idempotent. updateAndGet matches toggle()'s CAS shape; - // idempotency short-circuit means tap-spamming the radio rows - // (or replaying a settings import that doesn't actually change a - // value) doesn't repeatedly hit SP. Round-4 audit MED-2. + // Atomic + idempotent. Capture before-state, update in-memory, + // skip the SP write when the value didn't actually change. Round-5 + // audit LOW-1 / MED-2: the prior shape used + // `updateAndGet { r } == r` which is unconditionally true (lambda + // ignores prior) — dead code that confused readers. fun setMaxResolution(r: MaxResolution) { - val updated = _maxResolution.updateAndGet { r } == r - if (!updated) return - if (sp.getString(KEY_MAX_RES, null) == r.name) return + val before = _maxResolution.value + if (before == r) return + _maxResolution.value = r sp.edit().putString(KEY_MAX_RES, r.name).apply() } fun setThemeMode(t: ThemeMode) { - val updated = _themeMode.updateAndGet { t } == t - if (!updated) return - if (sp.getString(KEY_THEME, null) == t.name) return + val before = _themeMode.value + if (before == t) return + _themeMode.value = t sp.edit().putString(KEY_THEME, t.name).apply() } fun setCacheEnabled(enabled: Boolean) { val before = _cacheEnabled.value - _cacheEnabled.updateAndGet { enabled } if (before == enabled) return + _cacheEnabled.value = enabled sp.edit().putBoolean(KEY_CACHE_ENABLED, enabled).apply() } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelViewModel.kt index 7e6d5f6d9..c32b92320 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelViewModel.kt @@ -12,6 +12,7 @@ package com.sulkta.straw.feature.channel import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.sulkta.straw.feature.search.StreamItem +import com.sulkta.straw.util.isAllowedYtUrl import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow @@ -43,6 +44,14 @@ class ChannelViewModel : ViewModel() { fun load(channelUrl: String) { if (loadedUrl == channelUrl && _ui.value.videos.isNotEmpty()) return + // Round-5 audit MED-3: extractor-emitted uploaderUrl can be + // attacker-controlled if the YT response is poisoned upstream. + // Refuse non-YT hosts at the entry point so we don't even + // issue a network call to evil.com via strawcore. + if (!isAllowedYtUrl(channelUrl)) { + _ui.update { ChannelUiState(loading = false, error = "Unsupported URL") } + return + } inFlight?.cancel() loadedUrl = channelUrl _ui.update { ChannelUiState(loading = true) } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt index 4d524c85a..9918c0ead 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt @@ -25,6 +25,7 @@ import com.sulkta.straw.net.SbSegment import com.sulkta.straw.net.SponsorBlockClient import com.sulkta.straw.feature.search.StreamItem import com.sulkta.straw.util.isAllowedYtUrl +import com.sulkta.straw.util.runCatchingCancellable import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -100,6 +101,13 @@ class VideoDetailViewModel : ViewModel() { // navigations. Skip the refetch if the requested URL already has // a resolved state. if (loadedUrl == streamUrl && _ui.value.detail != null) return + // Same YT-host gate as ChannelViewModel — covers the case + // where a tap on a poisoned related-card lands here. + // Round-5 audit MED-3. + if (!isAllowedYtUrl(streamUrl)) { + _ui.update { VideoDetailUiState(loading = false, error = "Unsupported URL") } + return + } inFlight?.cancel() loadedUrl = streamUrl _ui.update { VideoDetailUiState(loading = true) } @@ -122,7 +130,7 @@ class VideoDetailViewModel : ViewModel() { // Round-4 audit HIGH-5. if (isAllowedYtUrl(streamUrl)) { withContext(Dispatchers.IO) { - runCatching { + runCatchingCancellable { History.get().recordWatch( WatchHistoryItem( url = streamUrl, @@ -138,11 +146,11 @@ class VideoDetailViewModel : ViewModel() { } val ryd = withContext(Dispatchers.IO) { - runCatching { RydClient.fetch(videoId) }.getOrNull() + runCatchingCancellable { RydClient.fetch(videoId) }.getOrNull() } val sbCats = Settings.get().sbCategories.value.map { it.key } val segments = if (sbCats.isEmpty()) emptyList() else withContext(Dispatchers.IO) { - runCatching { SponsorBlockClient.fetch(videoId, sbCats) } + runCatchingCancellable { SponsorBlockClient.fetch(videoId, sbCats) } .getOrDefault(emptyList()) } @@ -168,7 +176,7 @@ class VideoDetailViewModel : ViewModel() { val uploaderUrl = info.uploaderUrl val moreFromChannel: List = if (uploaderUrl.isNullOrBlank() || !isAllowedYtUrl(uploaderUrl)) emptyList() - else runCatching { + else runCatchingCancellable { val ch = uniffi.strawcore.channelInfo(uploaderUrl) ch.videos .filter { it.url != streamUrl } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt index 49da1066c..828a6b36b 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt @@ -24,6 +24,7 @@ import com.sulkta.straw.data.FeedCacheEntry import com.sulkta.straw.data.Settings import com.sulkta.straw.data.Subscriptions import com.sulkta.straw.feature.search.StreamItem +import com.sulkta.straw.util.runCatchingCancellable import com.sulkta.straw.util.strawLogW import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers @@ -223,7 +224,7 @@ class SubscriptionFeedViewModel : ViewModel() { private suspend fun fetchChannelInto(ch: ChannelRef) { val outcome = withTimeoutOrNull(perChannelTimeoutMs) { - runCatching { + runCatchingCancellable { val info = uniffi.strawcore.channelInfo(ch.url) // Opportunistic avatar refresh: if our stored ChannelRef // didn't capture an avatar at subscribe-time (channel @@ -231,7 +232,7 @@ class SubscriptionFeedViewModel : ViewModel() { // page loaded), backfill from the channel info now. val freshAvatar = info.avatar if (!freshAvatar.isNullOrBlank() && freshAvatar != ch.avatar) { - runCatching { + runCatchingCancellable { Subscriptions.get().updateAvatar(ch.url, freshAvatar) } } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt index 247ed09ef..446729753 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt @@ -32,10 +32,6 @@ object NowPlaying { private val _current = MutableStateFlow(null) val current: StateFlow = _current.asStateFlow() - fun set(item: NowPlayingItem?) { - _current.value = item - } - /** * Atomically claim playback for `streamUrl`. Returns true if this * call WON the claim (caller should now do setMediaItem + prepare + @@ -47,8 +43,10 @@ object NowPlaying { * * Uses MutableStateFlow.compareAndSet for the race-free transition. * vc=35 audit HIGH-C6 — the previous "check NowPlaying then - * setPlayingFrom" sequence had a window where both checks could - * pass before either NowPlaying.set ran. + * direct assign" sequence had a window where both checks could + * pass before either write happened. The non-CAS `set()` setter + * that lived alongside this method was dropped in round-5 (no + * external callers; left in code purely as a footgun). */ fun claim(item: NowPlayingItem): Boolean { while (true) { diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt index 155f471c2..a2062e7f0 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt @@ -364,6 +364,14 @@ fun SponsorBlockSkipLoop() { delay(150) val state = controller.playbackState if (state == Player.STATE_IDLE || state == Player.STATE_ENDED) continue + // Skip the position read + segment scan when not actively + // playing — on a paused-overnight session the prior shape + // hit the binder every 150ms for hours. Round-5 audit + // MED-2. + if (!controller.isPlaying) { + delay(1000) + continue + } val posSec = controller.currentPosition / 1000.0 val s = pickActiveSegment(segments, posSec, skipped) ?: continue strawLogI( diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt index 3f67ecd75..8ab53bf01 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt @@ -174,9 +174,13 @@ class SearchViewModel : ViewModel() { uploadDateRelative = r.uploadDateRelative, ) } - // Fence terminal write against a fresher submit that - // cancelled this one. Drop our result if the query - // already moved on. + // Fence ALL post-network side-effects against a + // fresher submit that cancelled this one — not just + // the UI update. Round-5 audit HIGH-2: the prior + // fence only guarded `_ui.update`, so the cache write + // + pool rebuild proceeded for a cancelled query, + // polluting SearchCache with results the user + // abandoned mid-stroke. if (_ui.value.query.trim() != q) return@launch _ui.update { it.copy( @@ -190,6 +194,10 @@ class SearchViewModel : ViewModel() { runCatching { History.get().recordSearch(q) } if (Settings.get().cacheEnabled.value) { withContext(Dispatchers.IO) { + // Re-check after suspending — a fresh submit + // can have changed the query between the UI + // update and now. + if (_ui.value.query.trim() != q) return@withContext runCatching { SearchCache.get().record(q, items) } // Refresh the in-memory pool with the new // entries so subsequent reactive filters see diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/util/Coroutines.kt b/strawApp/src/main/kotlin/com/sulkta/straw/util/Coroutines.kt new file mode 100644 index 000000000..4624eb458 --- /dev/null +++ b/strawApp/src/main/kotlin/com/sulkta/straw/util/Coroutines.kt @@ -0,0 +1,28 @@ +/* + * SPDX-FileCopyrightText: 2026 Sulkta-Coop + * SPDX-License-Identifier: GPL-3.0-or-later + * + * Coroutine-safe runCatching. Standard kotlin.runCatching catches + * Throwable — including CancellationException, which is supposed to + * propagate so structured concurrency works. Round-5 audit HIGH-2 + * surfaced the bug: a runCatching around a channelInfo() call + * inside a cancelled job swallowed the cancellation, the job + * carried on past the runCatching, hit its terminal write fence + * (which only checked URL-equality, so same-URL races couldn't + * be distinguished), and clobbered the newer job's state. + * + * Always use this in coroutine bodies. The plain runCatching is + * still fine in non-suspend code (no coroutine to cancel). + */ + +package com.sulkta.straw.util + +import kotlinx.coroutines.CancellationException + +inline fun runCatchingCancellable(block: () -> R): Result = try { + Result.success(block()) +} catch (c: CancellationException) { + throw c +} catch (t: Throwable) { + Result.failure(t) +} diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/util/YtUrl.kt b/strawApp/src/main/kotlin/com/sulkta/straw/util/YtUrl.kt index b42f1e586..5a243189e 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/util/YtUrl.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/util/YtUrl.kt @@ -20,6 +20,13 @@ private val ALLOWED_YT_HOSTS: Set = setOf( ) fun isAllowedYtUrl(url: String): Boolean { - val host = runCatching { java.net.URI(url).host?.lowercase() }.getOrNull() ?: return false + val uri = runCatching { java.net.URI(url) }.getOrNull() ?: return false + // Require an http/https scheme — `//host/...` (schemeless) and + // `mailto:host` both parse with a host attribute. Round-5 audit + // LOW-3. + val scheme = uri.scheme?.lowercase() ?: return false + if (scheme != "https" && scheme != "http") return false + // Strip a single trailing dot (RFC FQDN form) before lookup. + val host = uri.host?.lowercase()?.removeSuffix(".") ?: return false return host in ALLOWED_YT_HOSTS }