vc=40: loop round 2/5 — round-1 misses + new HIGHs from round-5 audits
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).
This commit is contained in:
parent
8d01dd835c
commit
738401b0ed
15 changed files with 141 additions and 45 deletions
|
|
@ -55,6 +55,6 @@ const val NEWPIPE_APPLICATION_ID_NEW = "net.newpipe.app"
|
||||||
// vc=19 / 0.1.0-AE — rust pipeline cutover. Extraction via
|
// vc=19 / 0.1.0-AE — rust pipeline cutover. Extraction via
|
||||||
// strawcore-core (Sulkta-OSS/strawcore) via the UniFFI wrapper; no
|
// strawcore-core (Sulkta-OSS/strawcore) via the UniFFI wrapper; no
|
||||||
// NewPipeExtractor in the runtime path.
|
// NewPipeExtractor in the runtime path.
|
||||||
const val STRAW_VERSION_CODE = 39
|
const val STRAW_VERSION_CODE = 40
|
||||||
const val STRAW_VERSION_NAME = "0.1.0-AY"
|
const val STRAW_VERSION_NAME = "0.1.0-AZ"
|
||||||
const val STRAW_APPLICATION_ID = "com.sulkta.straw"
|
const val STRAW_APPLICATION_ID = "com.sulkta.straw"
|
||||||
|
|
|
||||||
|
|
@ -24,6 +24,7 @@ pub struct ChannelInfo {
|
||||||
#[uniffi::export(async_runtime = "tokio")]
|
#[uniffi::export(async_runtime = "tokio")]
|
||||||
pub async fn channel_info(input: String) -> Result<ChannelInfo, StrawcoreError> {
|
pub async fn channel_info(input: String) -> Result<ChannelInfo, StrawcoreError> {
|
||||||
log::info!("strawcore::channel_info input_len={}", input.len());
|
log::info!("strawcore::channel_info input_len={}", input.len());
|
||||||
|
crate::runtime::ensure_initialized();
|
||||||
let identifier = resolve_channel_identifier(&input)?;
|
let identifier = resolve_channel_identifier(&input)?;
|
||||||
let core = tokio::task::spawn_blocking(move || core_channel_info(identifier))
|
let core = tokio::task::spawn_blocking(move || core_channel_info(identifier))
|
||||||
.await
|
.await
|
||||||
|
|
|
||||||
|
|
@ -43,26 +43,32 @@ fn now_ms() -> u64 {
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn ensure_initialized() {
|
pub fn ensure_initialized() {
|
||||||
|
// Fast path: already initialized. Just an Acquire load.
|
||||||
if INITIALIZED.load(Ordering::Acquire) {
|
if INITIALIZED.load(Ordering::Acquire) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
// Backoff check OUTSIDE the lock — avoids serializing every
|
// Acquire the lock FIRST. The mutex is the in-progress queue —
|
||||||
// already-throttled caller on a single mutex.
|
// 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 last = LAST_ATTEMPT_MS.load(Ordering::Acquire);
|
||||||
let now = now_ms();
|
let now = now_ms();
|
||||||
if last != 0 && now.saturating_sub(last) < RETRY_BACKOFF_MS {
|
if last != 0 && now.saturating_sub(last) < RETRY_BACKOFF_MS {
|
||||||
return;
|
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() {
|
match ReqwestDownloader::new() {
|
||||||
Ok(dl) => {
|
Ok(dl) => {
|
||||||
NewPipe::init_full(
|
NewPipe::init_full(
|
||||||
|
|
@ -71,11 +77,17 @@ pub fn ensure_initialized() {
|
||||||
ContentCountry::default(),
|
ContentCountry::default(),
|
||||||
);
|
);
|
||||||
INITIALIZED.store(true, Ordering::Release);
|
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");
|
log::info!("strawcore-core: downloader + localization initialized");
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
// Don't surface the underlying error string verbatim —
|
// Stamp the timestamp on FAILURE only, so the next
|
||||||
// it can embed URLs / hosts.
|
// 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)");
|
log::error!("strawcore-core: downloader init failed (will retry on next call)");
|
||||||
let _ = e;
|
let _ = e;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -60,6 +60,11 @@ pub async fn search(query: String) -> Result<Vec<SearchItem>, StrawcoreError> {
|
||||||
// Settings → Export Logs path straight into a user's chat. Log
|
// Settings → Export Logs path straight into a user's chat. Log
|
||||||
// shape, not content. vc=36 audit CVE HIGH-2.
|
// shape, not content. vc=36 audit CVE HIGH-2.
|
||||||
log::info!("strawcore::search query_len={}", query.len());
|
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 || {
|
let result = tokio::task::spawn_blocking(move || {
|
||||||
search_extractor::search(&query, SearchFilter::Videos)
|
search_extractor::search(&query, SearchFilter::Videos)
|
||||||
})
|
})
|
||||||
|
|
|
||||||
|
|
@ -58,6 +58,7 @@ pub struct AudioStreamItem {
|
||||||
#[uniffi::export(async_runtime = "tokio")]
|
#[uniffi::export(async_runtime = "tokio")]
|
||||||
pub async fn stream_info(input: String) -> Result<StreamInfo, StrawcoreError> {
|
pub async fn stream_info(input: String) -> Result<StreamInfo, StrawcoreError> {
|
||||||
log::info!("strawcore::stream_info input_len={}", input.len());
|
log::info!("strawcore::stream_info input_len={}", input.len());
|
||||||
|
crate::runtime::ensure_initialized();
|
||||||
let video_id = resolve_video_id(&input)?;
|
let video_id = resolve_video_id(&input)?;
|
||||||
let video_id_for_call = video_id.clone();
|
let video_id_for_call = video_id.clone();
|
||||||
let core = tokio::task::spawn_blocking(move || core_stream_info(&video_id_for_call))
|
let core = tokio::task::spawn_blocking(move || core_stream_info(&video_id_for_call))
|
||||||
|
|
|
||||||
|
|
@ -13,6 +13,8 @@ import com.sulkta.straw.data.SearchCache
|
||||||
import com.sulkta.straw.data.Settings
|
import com.sulkta.straw.data.Settings
|
||||||
import com.sulkta.straw.data.Subscriptions
|
import com.sulkta.straw.data.Subscriptions
|
||||||
import com.sulkta.straw.feature.dataimport.SettingsImport
|
import com.sulkta.straw.feature.dataimport.SettingsImport
|
||||||
|
import com.sulkta.straw.util.strawLogW
|
||||||
|
import kotlinx.coroutines.CoroutineExceptionHandler
|
||||||
import kotlinx.coroutines.CoroutineScope
|
import kotlinx.coroutines.CoroutineScope
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.SupervisorJob
|
import kotlinx.coroutines.SupervisorJob
|
||||||
|
|
@ -22,9 +24,16 @@ class StrawApp : Application() {
|
||||||
/**
|
/**
|
||||||
* App-scoped coroutine scope for one-time startup work that
|
* App-scoped coroutine scope for one-time startup work that
|
||||||
* shouldn't tie up Application.onCreate. SupervisorJob so a failure
|
* 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() {
|
override fun onCreate() {
|
||||||
super.onCreate()
|
super.onCreate()
|
||||||
|
|
|
||||||
|
|
@ -72,28 +72,29 @@ class SettingsStore(context: Context) {
|
||||||
sp.edit().putStringSet(KEY_SB_CATS, next.map { it.key }.toSet()).apply()
|
sp.edit().putStringSet(KEY_SB_CATS, next.map { it.key }.toSet()).apply()
|
||||||
}
|
}
|
||||||
|
|
||||||
// Atomic + idempotent. updateAndGet matches toggle()'s CAS shape;
|
// Atomic + idempotent. Capture before-state, update in-memory,
|
||||||
// idempotency short-circuit means tap-spamming the radio rows
|
// skip the SP write when the value didn't actually change. Round-5
|
||||||
// (or replaying a settings import that doesn't actually change a
|
// audit LOW-1 / MED-2: the prior shape used
|
||||||
// value) doesn't repeatedly hit SP. Round-4 audit MED-2.
|
// `updateAndGet { r } == r` which is unconditionally true (lambda
|
||||||
|
// ignores prior) — dead code that confused readers.
|
||||||
fun setMaxResolution(r: MaxResolution) {
|
fun setMaxResolution(r: MaxResolution) {
|
||||||
val updated = _maxResolution.updateAndGet { r } == r
|
val before = _maxResolution.value
|
||||||
if (!updated) return
|
if (before == r) return
|
||||||
if (sp.getString(KEY_MAX_RES, null) == r.name) return
|
_maxResolution.value = r
|
||||||
sp.edit().putString(KEY_MAX_RES, r.name).apply()
|
sp.edit().putString(KEY_MAX_RES, r.name).apply()
|
||||||
}
|
}
|
||||||
|
|
||||||
fun setThemeMode(t: ThemeMode) {
|
fun setThemeMode(t: ThemeMode) {
|
||||||
val updated = _themeMode.updateAndGet { t } == t
|
val before = _themeMode.value
|
||||||
if (!updated) return
|
if (before == t) return
|
||||||
if (sp.getString(KEY_THEME, null) == t.name) return
|
_themeMode.value = t
|
||||||
sp.edit().putString(KEY_THEME, t.name).apply()
|
sp.edit().putString(KEY_THEME, t.name).apply()
|
||||||
}
|
}
|
||||||
|
|
||||||
fun setCacheEnabled(enabled: Boolean) {
|
fun setCacheEnabled(enabled: Boolean) {
|
||||||
val before = _cacheEnabled.value
|
val before = _cacheEnabled.value
|
||||||
_cacheEnabled.updateAndGet { enabled }
|
|
||||||
if (before == enabled) return
|
if (before == enabled) return
|
||||||
|
_cacheEnabled.value = enabled
|
||||||
sp.edit().putBoolean(KEY_CACHE_ENABLED, enabled).apply()
|
sp.edit().putBoolean(KEY_CACHE_ENABLED, enabled).apply()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -12,6 +12,7 @@ package com.sulkta.straw.feature.channel
|
||||||
import androidx.lifecycle.ViewModel
|
import androidx.lifecycle.ViewModel
|
||||||
import androidx.lifecycle.viewModelScope
|
import androidx.lifecycle.viewModelScope
|
||||||
import com.sulkta.straw.feature.search.StreamItem
|
import com.sulkta.straw.feature.search.StreamItem
|
||||||
|
import com.sulkta.straw.util.isAllowedYtUrl
|
||||||
import kotlinx.coroutines.CancellationException
|
import kotlinx.coroutines.CancellationException
|
||||||
import kotlinx.coroutines.Job
|
import kotlinx.coroutines.Job
|
||||||
import kotlinx.coroutines.flow.MutableStateFlow
|
import kotlinx.coroutines.flow.MutableStateFlow
|
||||||
|
|
@ -43,6 +44,14 @@ class ChannelViewModel : ViewModel() {
|
||||||
|
|
||||||
fun load(channelUrl: String) {
|
fun load(channelUrl: String) {
|
||||||
if (loadedUrl == channelUrl && _ui.value.videos.isNotEmpty()) return
|
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()
|
inFlight?.cancel()
|
||||||
loadedUrl = channelUrl
|
loadedUrl = channelUrl
|
||||||
_ui.update { ChannelUiState(loading = true) }
|
_ui.update { ChannelUiState(loading = true) }
|
||||||
|
|
|
||||||
|
|
@ -25,6 +25,7 @@ import com.sulkta.straw.net.SbSegment
|
||||||
import com.sulkta.straw.net.SponsorBlockClient
|
import com.sulkta.straw.net.SponsorBlockClient
|
||||||
import com.sulkta.straw.feature.search.StreamItem
|
import com.sulkta.straw.feature.search.StreamItem
|
||||||
import com.sulkta.straw.util.isAllowedYtUrl
|
import com.sulkta.straw.util.isAllowedYtUrl
|
||||||
|
import com.sulkta.straw.util.runCatchingCancellable
|
||||||
import kotlinx.coroutines.CancellationException
|
import kotlinx.coroutines.CancellationException
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.Job
|
import kotlinx.coroutines.Job
|
||||||
|
|
@ -100,6 +101,13 @@ class VideoDetailViewModel : ViewModel() {
|
||||||
// navigations. Skip the refetch if the requested URL already has
|
// navigations. Skip the refetch if the requested URL already has
|
||||||
// a resolved state.
|
// a resolved state.
|
||||||
if (loadedUrl == streamUrl && _ui.value.detail != null) return
|
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()
|
inFlight?.cancel()
|
||||||
loadedUrl = streamUrl
|
loadedUrl = streamUrl
|
||||||
_ui.update { VideoDetailUiState(loading = true) }
|
_ui.update { VideoDetailUiState(loading = true) }
|
||||||
|
|
@ -122,7 +130,7 @@ class VideoDetailViewModel : ViewModel() {
|
||||||
// Round-4 audit HIGH-5.
|
// Round-4 audit HIGH-5.
|
||||||
if (isAllowedYtUrl(streamUrl)) {
|
if (isAllowedYtUrl(streamUrl)) {
|
||||||
withContext(Dispatchers.IO) {
|
withContext(Dispatchers.IO) {
|
||||||
runCatching {
|
runCatchingCancellable {
|
||||||
History.get().recordWatch(
|
History.get().recordWatch(
|
||||||
WatchHistoryItem(
|
WatchHistoryItem(
|
||||||
url = streamUrl,
|
url = streamUrl,
|
||||||
|
|
@ -138,11 +146,11 @@ class VideoDetailViewModel : ViewModel() {
|
||||||
}
|
}
|
||||||
|
|
||||||
val ryd = withContext(Dispatchers.IO) {
|
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 sbCats = Settings.get().sbCategories.value.map { it.key }
|
||||||
val segments = if (sbCats.isEmpty()) emptyList() else withContext(Dispatchers.IO) {
|
val segments = if (sbCats.isEmpty()) emptyList() else withContext(Dispatchers.IO) {
|
||||||
runCatching { SponsorBlockClient.fetch(videoId, sbCats) }
|
runCatchingCancellable { SponsorBlockClient.fetch(videoId, sbCats) }
|
||||||
.getOrDefault(emptyList())
|
.getOrDefault(emptyList())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -168,7 +176,7 @@ class VideoDetailViewModel : ViewModel() {
|
||||||
val uploaderUrl = info.uploaderUrl
|
val uploaderUrl = info.uploaderUrl
|
||||||
val moreFromChannel: List<StreamItem> =
|
val moreFromChannel: List<StreamItem> =
|
||||||
if (uploaderUrl.isNullOrBlank() || !isAllowedYtUrl(uploaderUrl)) emptyList()
|
if (uploaderUrl.isNullOrBlank() || !isAllowedYtUrl(uploaderUrl)) emptyList()
|
||||||
else runCatching {
|
else runCatchingCancellable {
|
||||||
val ch = uniffi.strawcore.channelInfo(uploaderUrl)
|
val ch = uniffi.strawcore.channelInfo(uploaderUrl)
|
||||||
ch.videos
|
ch.videos
|
||||||
.filter { it.url != streamUrl }
|
.filter { it.url != streamUrl }
|
||||||
|
|
|
||||||
|
|
@ -24,6 +24,7 @@ import com.sulkta.straw.data.FeedCacheEntry
|
||||||
import com.sulkta.straw.data.Settings
|
import com.sulkta.straw.data.Settings
|
||||||
import com.sulkta.straw.data.Subscriptions
|
import com.sulkta.straw.data.Subscriptions
|
||||||
import com.sulkta.straw.feature.search.StreamItem
|
import com.sulkta.straw.feature.search.StreamItem
|
||||||
|
import com.sulkta.straw.util.runCatchingCancellable
|
||||||
import com.sulkta.straw.util.strawLogW
|
import com.sulkta.straw.util.strawLogW
|
||||||
import kotlinx.coroutines.CancellationException
|
import kotlinx.coroutines.CancellationException
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
|
|
@ -223,7 +224,7 @@ class SubscriptionFeedViewModel : ViewModel() {
|
||||||
|
|
||||||
private suspend fun fetchChannelInto(ch: ChannelRef) {
|
private suspend fun fetchChannelInto(ch: ChannelRef) {
|
||||||
val outcome = withTimeoutOrNull(perChannelTimeoutMs) {
|
val outcome = withTimeoutOrNull(perChannelTimeoutMs) {
|
||||||
runCatching {
|
runCatchingCancellable {
|
||||||
val info = uniffi.strawcore.channelInfo(ch.url)
|
val info = uniffi.strawcore.channelInfo(ch.url)
|
||||||
// Opportunistic avatar refresh: if our stored ChannelRef
|
// Opportunistic avatar refresh: if our stored ChannelRef
|
||||||
// didn't capture an avatar at subscribe-time (channel
|
// didn't capture an avatar at subscribe-time (channel
|
||||||
|
|
@ -231,7 +232,7 @@ class SubscriptionFeedViewModel : ViewModel() {
|
||||||
// page loaded), backfill from the channel info now.
|
// page loaded), backfill from the channel info now.
|
||||||
val freshAvatar = info.avatar
|
val freshAvatar = info.avatar
|
||||||
if (!freshAvatar.isNullOrBlank() && freshAvatar != ch.avatar) {
|
if (!freshAvatar.isNullOrBlank() && freshAvatar != ch.avatar) {
|
||||||
runCatching {
|
runCatchingCancellable {
|
||||||
Subscriptions.get().updateAvatar(ch.url, freshAvatar)
|
Subscriptions.get().updateAvatar(ch.url, freshAvatar)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -32,10 +32,6 @@ object NowPlaying {
|
||||||
private val _current = MutableStateFlow<NowPlayingItem?>(null)
|
private val _current = MutableStateFlow<NowPlayingItem?>(null)
|
||||||
val current: StateFlow<NowPlayingItem?> = _current.asStateFlow()
|
val current: StateFlow<NowPlayingItem?> = _current.asStateFlow()
|
||||||
|
|
||||||
fun set(item: NowPlayingItem?) {
|
|
||||||
_current.value = item
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Atomically claim playback for `streamUrl`. Returns true if this
|
* Atomically claim playback for `streamUrl`. Returns true if this
|
||||||
* call WON the claim (caller should now do setMediaItem + prepare +
|
* call WON the claim (caller should now do setMediaItem + prepare +
|
||||||
|
|
@ -47,8 +43,10 @@ object NowPlaying {
|
||||||
*
|
*
|
||||||
* Uses MutableStateFlow.compareAndSet for the race-free transition.
|
* Uses MutableStateFlow.compareAndSet for the race-free transition.
|
||||||
* vc=35 audit HIGH-C6 — the previous "check NowPlaying then
|
* vc=35 audit HIGH-C6 — the previous "check NowPlaying then
|
||||||
* setPlayingFrom" sequence had a window where both checks could
|
* direct assign" sequence had a window where both checks could
|
||||||
* pass before either NowPlaying.set ran.
|
* 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 {
|
fun claim(item: NowPlayingItem): Boolean {
|
||||||
while (true) {
|
while (true) {
|
||||||
|
|
|
||||||
|
|
@ -364,6 +364,14 @@ fun SponsorBlockSkipLoop() {
|
||||||
delay(150)
|
delay(150)
|
||||||
val state = controller.playbackState
|
val state = controller.playbackState
|
||||||
if (state == Player.STATE_IDLE || state == Player.STATE_ENDED) continue
|
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 posSec = controller.currentPosition / 1000.0
|
||||||
val s = pickActiveSegment(segments, posSec, skipped) ?: continue
|
val s = pickActiveSegment(segments, posSec, skipped) ?: continue
|
||||||
strawLogI(
|
strawLogI(
|
||||||
|
|
|
||||||
|
|
@ -174,9 +174,13 @@ class SearchViewModel : ViewModel() {
|
||||||
uploadDateRelative = r.uploadDateRelative,
|
uploadDateRelative = r.uploadDateRelative,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
// Fence terminal write against a fresher submit that
|
// Fence ALL post-network side-effects against a
|
||||||
// cancelled this one. Drop our result if the query
|
// fresher submit that cancelled this one — not just
|
||||||
// already moved on.
|
// 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
|
if (_ui.value.query.trim() != q) return@launch
|
||||||
_ui.update {
|
_ui.update {
|
||||||
it.copy(
|
it.copy(
|
||||||
|
|
@ -190,6 +194,10 @@ class SearchViewModel : ViewModel() {
|
||||||
runCatching { History.get().recordSearch(q) }
|
runCatching { History.get().recordSearch(q) }
|
||||||
if (Settings.get().cacheEnabled.value) {
|
if (Settings.get().cacheEnabled.value) {
|
||||||
withContext(Dispatchers.IO) {
|
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) }
|
runCatching { SearchCache.get().record(q, items) }
|
||||||
// Refresh the in-memory pool with the new
|
// Refresh the in-memory pool with the new
|
||||||
// entries so subsequent reactive filters see
|
// entries so subsequent reactive filters see
|
||||||
|
|
|
||||||
28
strawApp/src/main/kotlin/com/sulkta/straw/util/Coroutines.kt
Normal file
28
strawApp/src/main/kotlin/com/sulkta/straw/util/Coroutines.kt
Normal file
|
|
@ -0,0 +1,28 @@
|
||||||
|
/*
|
||||||
|
* SPDX-FileCopyrightText: 2026 Sulkta
|
||||||
|
* 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 <R> runCatchingCancellable(block: () -> R): Result<R> = try {
|
||||||
|
Result.success(block())
|
||||||
|
} catch (c: CancellationException) {
|
||||||
|
throw c
|
||||||
|
} catch (t: Throwable) {
|
||||||
|
Result.failure(t)
|
||||||
|
}
|
||||||
|
|
@ -20,6 +20,13 @@ private val ALLOWED_YT_HOSTS: Set<String> = setOf(
|
||||||
)
|
)
|
||||||
|
|
||||||
fun isAllowedYtUrl(url: String): Boolean {
|
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
|
return host in ALLOWED_YT_HOSTS
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue