vc=37: round-2 audit-fix sprint — 2 CRIT + 11 HIGH + 4 MED

Three round-2 Opus audits ran on the vc=35+vc=36 surface. CVE
returned no new CRITs (round-1 fixes hold) but found 5 new HIGH.
Code-health found 2 CRIT — both my own vc=35 regressions. Function-
correctness found 5 BROKEN that the round-1 sweep missed.

CRIT (from code-health round 2)
  R1  Subs feed avatar-backfill self-cancel loop.
      Subscriptions.updateAvatar emits a new _subs reference;
      SubsPane's LaunchedEffect(subs) reacts → refreshIfStale →
      refresh() → inFlight.cancel(). With N channels needing
      backfill the parallel-12 batch degenerated into N sequential
      single-channel fetches that kept aborting each other. Gated
      refreshIfStale on inFlight.isActive != true.
  R2  HistoryStore.recordAllWatches O(N²) input.
      The vc=35 bulk-import path collapsed N SP writes into 1
      (good) but used ArrayList.add(0, item) inside a loop walking
      up to 50k input rows before take(50). ~1.25B shifts worst
      case. Rewritten: walk newest-first, filter blanks + seen
      IDs, stop at MAX_WATCHES. O(N) bounded by output cap.

HIGH (from CVE round 2)
  CVE-1  PlayerScreen + VideoDetailScreen rendered raw
         error.message into the UI — Media3 HttpDataSource
         exceptions include the full request URI with sig=/pot=.
         User screenshots a playback error to a chat → full
         session credentials in the picture. Both surfaces now
         scrub via LogDump.scrubLine before rendering.
  CVE-2  SubscriptionsStore.addAll counter race —
         updateAndGet's lambda re-runs on CAS retry; var-outside-
         lambda increment double-counted. Now derives `added`
         from next.size - cur.size delta.
  CVE-3  sweepStale ran deleteRecursively() on cacheDir (up to
         ~256MB) on the main thread inside Application.onCreate.
         Moved to appScope.launch(Dispatchers.IO).
  CVE-MED-2  Expanded LogDump.SIGNED_PARAM_RE alternation to
             include n / lsig / ei / key / sparams.
  CVE-MED-3  PlayerScreen + VideoDetailScreen error handlers now
             also NowPlaying.clear() so the minibar doesn't keep
             claiming a dead session is loaded.
  CVE-MED-4  SettingsImport validates imported subscription /
             playlist / history URLs against IMPORT_ALLOWED_HOSTS
             at import time. Hostile NewPipe export can no
             longer smuggle attacker-controlled URLs.

HIGH (from code-health round 2)
  R3  Store constructors hit SP + JSON-decode on main thread at
      Application.onCreate. Small stores (Settings, History,
      Subscriptions, Playlists) stay eager — sub-millisecond
      cost. Heavy stores (FeedCache ~225 KB, SearchCache ~150
      KB) now lazy-init: their `init()` just stashes
      applicationContext; the actual Store + disk decode is
      built on first `get()`, which happens from VM IO-dispatched
      coroutines.
  R4  SearchViewModel.pool race with init coroutine. Switched
      pool to a plain @Volatile var (no observers anyway — LOW-
      R14) and exposed rebuildPool() so the cache-toggle handler
      and a future explicit hook can refresh it.
  R5  SubsPane first-paint empty flash. Seeded
      SubscriptionFeedUiState(loading = true) in the VM's
      initial state — the init coroutine always runs.
  R6  Dropped dead uploaderAvatar field on StreamItem. Written
      three places, read zero. Saved bytes in every cache entry.
  R7  Split mergeFromCache into pruneCacheToSubs + mergeFromCache
      (no side effect in the reader). Callers do prune then
      merge.
  R8  Settings cache-disable wipe now runs on Dispatchers.IO
      (3 SP-edit calls were on the UI thread).

HIGH (from function-correctness round 2)
  B1  refresh() empty-channels also wipes disk cache (was
      in-memory only — disk orphans accumulated).
  B2  Settings cache OFF→ON now triggers feedVm.refresh() +
      searchVm.rebuildPool() so the user doesn't have to
      navigate away and back to repopulate.
  B3  SearchViewModel.submit() cache lookup was still doing
      SearchCache.get().load() on main (CRIT-C1 was only
      partial). Now uses entries.value (StateFlow snapshot).
  B5  SearchCacheStore.record now atomic via MutableStateFlow
      + updateAndGet (was load()→write() with no atomicity, so
      concurrent records lost entries).
  Q9  History.recordWatch wrapped in withContext(Dispatchers.IO).
  Q11 Minibar onPlayerError also stops the controller + clears
      media items (was leaking dead controller state).

MED
  R10 Added comments at the 4 pre-flight NowPlaying checks
      noting they're optimizations, claim() is the safety guard.
      Prevents a future refactor recreating the round-1 race
      after deleting "the guard."
  R11 Minibar Toast continues but now layered with the
      controller.stop() + clearMediaItems().
  CVE-MED-1 NowPlaying.claim updates metadata fields when the
      same URL is re-claimed (was returning false unconditionally,
      pinning truncated search titles over fresh-from-detail titles).
  Q3  onQueryChange clears state.error so a failed-submit's
      banner doesn't haunt the next reactive preview.

Deferred to vc=38 (intentional cost/benefit):
  CVE HIGH-2 (Rust strawcore::search query= info-log) — needs
    a separate strawcore-core edit + rebuild. Logged as a
    follow-up.
  CVE HIGH-3 (DownloadManager setVisibleInDownloadsUi
    deprecated on API 29+) — only the direct-streaming
    download replacement is a full fix; that's a multi-day
    refactor.
  Q5/Q8 (SettingsImport hostile zip silent abort UX) —
    cosmetic dialog-title fix.
  Q12 (loadedUrl assignment ordering) — pre-existing,
    deferred again.
This commit is contained in:
Kayos 2026-05-25 14:05:58 -07:00
parent d1ee9379e0
commit 567423336c
17 changed files with 328 additions and 112 deletions

View file

@ -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-Coop/strawcore) via the UniFFI wrapper; no // strawcore-core (Sulkta-Coop/strawcore) via the UniFFI wrapper; no
// NewPipeExtractor in the runtime path. // NewPipeExtractor in the runtime path.
const val STRAW_VERSION_CODE = 36 const val STRAW_VERSION_CODE = 37
const val STRAW_VERSION_NAME = "0.1.0-AV" const val STRAW_VERSION_NAME = "0.1.0-AW"
const val STRAW_APPLICATION_ID = "com.sulkta.straw" const val STRAW_APPLICATION_ID = "com.sulkta.straw"

View file

@ -13,8 +13,19 @@ 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 kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.launch
class StrawApp : Application() { 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.
*/
private val appScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
override fun onCreate() { override fun onCreate() {
super.onCreate() super.onCreate()
// Path C-7: route Rust `log::*` calls into Android logcat under tag // Path C-7: route Rust `log::*` calls into Android logcat under tag
@ -22,16 +33,29 @@ class StrawApp : Application() {
// strawcore is silently dropped, making playback regressions invisible // strawcore is silently dropped, making playback regressions invisible
// from `adb logcat`. // from `adb logcat`.
uniffi.strawcore.initLogging() uniffi.strawcore.initLogging()
History.init(this) // Small + universally-accessed stores: synchronous init.
// Settings is a handful of SP keys (read on first compose for
// themeMode), History caps at 50 watches + 20 searches,
// Subscriptions is a single channel list — sub-millisecond
// cost on cold cache.
Settings.init(this) Settings.init(this)
History.init(this)
Subscriptions.init(this) Subscriptions.init(this)
Playlists.init(this) Playlists.init(this)
// vc=36 audit HIGH-R3: FeedCache (~225 KB) + SearchCache
// (~150 KB) JSON-decode at construction. Stash the
// applicationContext eagerly (cheap) so `get()` is callable
// anywhere; the actual store construction (and the disk
// decode that goes with it) is lazy. ViewModels accessing
// these on IO trigger the construction there — never on the
// main thread.
FeedCache.init(this) FeedCache.init(this)
SearchCache.init(this) SearchCache.init(this)
// Sweep any newpipe-import-* work-dirs left in cacheDir by a // vc=36 audit CVE HIGH-5: sweepStale's deleteRecursively()
// previous import that was killed mid-extraction. CRIT from // can walk ~256 MB if a previous import was LMK-killed
// the vc=34 security audit — the user's full NewPipe DB would // mid-extraction. Strictly off the main thread.
// otherwise live in cacheDir until the next deleteRecursively. appScope.launch {
SettingsImport.sweepStale(this) SettingsImport.sweepStale(this@StrawApp)
}
} }
} }

View file

@ -56,16 +56,30 @@ class FeedCacheStore(context: Context) {
} }
object FeedCache { object FeedCache {
@Volatile private var appContext: Context? = null
@Volatile private var instance: FeedCacheStore? = null @Volatile private var instance: FeedCacheStore? = null
/**
* Lazy init: stash the applicationContext only. The actual Store
* (and the ~225 KB JSON decode that happens at construction) is
* deferred until the first `get()` call. Lets Application.onCreate
* return quickly while every caller still gets a valid Store
* vc=36 audit HIGH-R3. Callers should access from a coroutine
* (IO dispatcher) where the lazy construction cost is acceptable.
*/
fun init(context: Context) { fun init(context: Context) {
if (instance == null) { appContext = context.applicationContext
synchronized(this) {
if (instance == null) instance = FeedCacheStore(context.applicationContext)
}
}
} }
fun get(): FeedCacheStore = instance fun get(): FeedCacheStore {
?: error("FeedCacheStore not initialized — call FeedCache.init(context)") instance?.let { return it }
synchronized(this) {
instance?.let { return it }
val ctx = appContext
?: error("FeedCacheStore not initialized — call FeedCache.init(context)")
val built = FeedCacheStore(ctx)
instance = built
return built
}
}
} }

View file

@ -58,24 +58,37 @@ class HistoryStore(context: Context) {
/** /**
* Bulk import. Callers (currently SettingsImport) feed * Bulk import. Callers (currently SettingsImport) feed
* oldestnewest so the most-recent entries end up at the front * oldestnewest. Single SP write vc=34 audit flagged the
* of the capped list. Single SP write vc=34 audit flagged the
* per-row recordWatch in importHistory as a write-storm vector. * per-row recordWatch in importHistory as a write-storm vector.
*
* O(N) on input size, not O(). The vc=35 first cut had an
* `add(0, item)` inside a loop walking up to MAX_HISTORY_IMPORT
* (~50k) entries ArrayList shift over `merged.size` each step,
* a billion+ shifts in the worst case for a final `take(50)` that
* discards 99.9% of the work. Round-2 audit CRIT-R2.
*
* New shape: walk input newest-first (reversed; SettingsImport
* fed oldest-first), filter blanks + already-seen videoIds, take
* up to MAX_WATCHES, prepend to current. Done in one pass with
* the capped output never exceeding MAX_WATCHES.
*/ */
fun recordAllWatches(items: List<WatchHistoryItem>) { fun recordAllWatches(items: List<WatchHistoryItem>) {
if (items.isEmpty()) return if (items.isEmpty()) return
val next = _watches.updateAndGet { current -> val next = _watches.updateAndGet { current ->
val seen = current.map { it.videoId }.toMutableSet() val seen = HashSet<String>(current.size + items.size)
val merged = current.toMutableList() current.forEach { seen.add(it.videoId) }
for (item in items) { val capacity = (MAX_WATCHES - current.size).coerceAtLeast(0)
if (capacity == 0) return@updateAndGet current
val fresh = ArrayList<WatchHistoryItem>(capacity)
// Walk newest-first; stop as soon as we have capacity.
val it = items.listIterator(items.size)
while (it.hasPrevious() && fresh.size < capacity) {
val item = it.previous()
if (item.videoId.isBlank()) continue if (item.videoId.isBlank()) continue
if (item.videoId in seen) { if (!seen.add(item.videoId)) continue
merged.removeAll { it.videoId == item.videoId } fresh.add(item)
}
seen.add(item.videoId)
merged.add(0, item)
} }
merged.take(MAX_WATCHES) (fresh + current).take(MAX_WATCHES)
} }
sp.edit().putString(KEY_WATCHES, json.encodeToString(next)).apply() sp.edit().putString(KEY_WATCHES, json.encodeToString(next)).apply()
} }

View file

@ -11,6 +11,11 @@
* Sized for SharedPreferences: 30 queries * 20 items each * ~250 bytes * Sized for SharedPreferences: 30 queries * 20 items each * ~250 bytes
* = ~150 KB worst case. * = ~150 KB worst case.
* *
* Backed by a MutableStateFlow loaded once at construction
* record()/load() are atomic against concurrent calls. vc=36 audit
* B5: the prior load()edit()write() pattern would clobber a
* concurrent record() with whichever happened to persist last.
*
* Skips entirely when Settings.cacheEnabled is false caller checks * Skips entirely when Settings.cacheEnabled is false caller checks
* the flag before reading/writing. * the flag before reading/writing.
*/ */
@ -20,6 +25,10 @@ package com.sulkta.straw.data
import android.content.Context import android.content.Context
import android.content.SharedPreferences import android.content.SharedPreferences
import com.sulkta.straw.feature.search.StreamItem import com.sulkta.straw.feature.search.StreamItem
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.updateAndGet
import kotlinx.serialization.Serializable import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json import kotlinx.serialization.json.Json
@ -39,43 +48,60 @@ class SearchCacheStore(context: Context) {
private val sp: SharedPreferences = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) private val sp: SharedPreferences = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE)
private val json = Json { ignoreUnknownKeys = true } private val json = Json { ignoreUnknownKeys = true }
fun load(): List<SearchCacheEntry> = runCatching { private val _entries = MutableStateFlow(loadFromDisk())
val s = sp.getString(KEY, null) ?: return emptyList() val entries: StateFlow<List<SearchCacheEntry>> = _entries.asStateFlow()
json.decodeFromString<List<SearchCacheEntry>>(s)
}.getOrDefault(emptyList()) /** Snapshot of the cache. Used by the reactive search filter. */
fun load(): List<SearchCacheEntry> = _entries.value
/** /**
* Record a freshly-fetched query result. Idempotent: a re-run of * Record a freshly-fetched query result. Idempotent: a re-run of
* the same query overwrites the prior entry rather than duplicating. * the same query overwrites the prior entry rather than duplicating.
* Oldest entries fall off when MAX_QUERIES is exceeded. * Oldest entries fall off when MAX_QUERIES is exceeded.
*
* Atomic via updateAndGet concurrent records don't lose entries.
*/ */
fun record(query: String, items: List<StreamItem>) { fun record(query: String, items: List<StreamItem>) {
val q = query.trim() val q = query.trim()
if (q.isEmpty() || items.isEmpty()) return if (q.isEmpty() || items.isEmpty()) return
val capped = items.take(MAX_ITEMS_PER_QUERY) val capped = items.take(MAX_ITEMS_PER_QUERY)
val now = System.currentTimeMillis() val now = System.currentTimeMillis()
val current = load() val next = _entries.updateAndGet { current ->
val without = current.filterNot { it.query.equals(q, ignoreCase = true) } val without = current.filterNot { it.query.equals(q, ignoreCase = true) }
val next = (listOf(SearchCacheEntry(q, now, capped)) + without).take(MAX_QUERIES) (listOf(SearchCacheEntry(q, now, capped)) + without).take(MAX_QUERIES)
}
sp.edit().putString(KEY, json.encodeToString(next)).apply() sp.edit().putString(KEY, json.encodeToString(next)).apply()
} }
fun clear() { fun clear() {
_entries.value = emptyList()
sp.edit().remove(KEY).apply() sp.edit().remove(KEY).apply()
} }
private fun loadFromDisk(): List<SearchCacheEntry> = runCatching {
val s = sp.getString(KEY, null) ?: return emptyList()
json.decodeFromString<List<SearchCacheEntry>>(s)
}.getOrDefault(emptyList())
} }
object SearchCache { object SearchCache {
@Volatile private var appContext: Context? = null
@Volatile private var instance: SearchCacheStore? = null @Volatile private var instance: SearchCacheStore? = null
/** Lazy init — see FeedCache.init for the rationale. */
fun init(context: Context) { fun init(context: Context) {
if (instance == null) { appContext = context.applicationContext
synchronized(this) {
if (instance == null) instance = SearchCacheStore(context.applicationContext)
}
}
} }
fun get(): SearchCacheStore = instance fun get(): SearchCacheStore {
?: error("SearchCacheStore not initialized — call SearchCache.init(context)") instance?.let { return it }
synchronized(this) {
instance?.let { return it }
val ctx = appContext
?: error("SearchCacheStore not initialized — call SearchCache.init(context)")
val built = SearchCacheStore(ctx)
instance = built
return built
}
}
} }

View file

@ -73,20 +73,22 @@ class SubscriptionsStore(context: Context) {
* caller can report an "added X" stat. * caller can report an "added X" stat.
*/ */
fun addAll(refs: List<ChannelRef>): Int { fun addAll(refs: List<ChannelRef>): Int {
var added = 0 // Derive `added` from the size delta INSTEAD of incrementing a
val next = _subs.updateAndGet { cur -> // var inside updateAndGet's lambda — that lambda can re-run
val byUrl = cur.associateBy { it.url }.toMutableMap() // under CAS contention (a concurrent toggle from the channel
// screen during a 500-row import), and a var-outside-lambda
// accumulates across retries. vc=36 audit CVE HIGH-4.
val cur = _subs.value
val next = _subs.updateAndGet { state ->
val byUrl = state.associateBy { it.url }.toMutableMap()
for (r in refs) { for (r in refs) {
if (r.url.isBlank()) continue if (r.url.isBlank()) continue
if (r.url !in byUrl) { if (r.url !in byUrl) byUrl[r.url] = r
byUrl[r.url] = r
added++
}
} }
byUrl.values.toList() byUrl.values.toList()
} }
persist(next) persist(next)
return added return next.size - cur.size
} }
fun clear() { fun clear() {

View file

@ -42,7 +42,6 @@ class ChannelViewModel : ViewModel() {
title = v.title.ifBlank { "(no title)" }, title = v.title.ifBlank { "(no title)" },
uploader = v.uploader, uploader = v.uploader,
uploaderUrl = v.uploaderUrl, uploaderUrl = v.uploaderUrl,
uploaderAvatar = ch.avatar,
thumbnail = v.thumbnail, thumbnail = v.thumbnail,
durationSeconds = v.durationSeconds, durationSeconds = v.durationSeconds,
viewCount = v.viewCount, viewCount = v.viewCount,

View file

@ -102,6 +102,23 @@ object SettingsImport {
// YouTube only — Straw doesn't extract from other services. // YouTube only — Straw doesn't extract from other services.
private const val YT_SERVICE_ID = 0 private const val YT_SERVICE_ID = 0
// Mirror of StrawActivity.YT_HOSTS — kept inline rather than
// imported because the activity holds the canonical copy and
// SettingsImport is the only other consumer.
// vc=36 audit CVE MED-4 — validate imported URLs at import time
// so a hostile NewPipe export can't smuggle attacker-controlled
// URLs into PlaylistStore / HistoryStore.
private val IMPORT_ALLOWED_HOSTS = setOf(
"youtube.com", "www.youtube.com", "m.youtube.com",
"music.youtube.com", "youtube-nocookie.com", "www.youtube-nocookie.com",
"youtu.be",
)
private fun isAllowedYtUrl(url: String): Boolean {
val host = runCatching { java.net.URI(url).host?.lowercase() }.getOrNull() ?: return false
return host in IMPORT_ALLOWED_HOSTS
}
suspend fun run(context: Context, zipUri: Uri): Result<ImportResult> = suspend fun run(context: Context, zipUri: Uri): Result<ImportResult> =
withContext(Dispatchers.IO) { withContext(Dispatchers.IO) {
runCatching { runInner(context, zipUri) } runCatching { runInner(context, zipUri) }
@ -275,6 +292,10 @@ object SettingsImport {
continue continue
} }
val url = c.getString(0) ?: continue val url = c.getString(0) ?: continue
if (!isAllowedYtUrl(url)) {
skipped++
continue
}
val name = c.getString(1) ?: continue val name = c.getString(1) ?: continue
val avatar = c.getString(2) val avatar = c.getString(2)
staged += ChannelRef(url = url, name = name, avatar = avatar) staged += ChannelRef(url = url, name = name, avatar = avatar)
@ -314,8 +335,10 @@ object SettingsImport {
).use { c -> ).use { c ->
while (c.moveToNext()) { while (c.moveToNext()) {
if (c.getInt(4) != YT_SERVICE_ID) continue if (c.getInt(4) != YT_SERVICE_ID) continue
val streamUrl = c.getString(0) ?: continue
if (!isAllowedYtUrl(streamUrl)) continue
items += PlaylistItem( items += PlaylistItem(
streamUrl = c.getString(0) ?: continue, streamUrl = streamUrl,
title = c.getString(1) ?: "(no title)", title = c.getString(1) ?: "(no title)",
thumbnail = c.getString(2), thumbnail = c.getString(2),
uploader = c.getString(3) ?: "", uploader = c.getString(3) ?: "",
@ -391,6 +414,7 @@ object SettingsImport {
while (c.moveToNext()) { while (c.moveToNext()) {
if (c.getInt(5) != YT_SERVICE_ID) continue if (c.getInt(5) != YT_SERVICE_ID) continue
val url = c.getString(0) ?: continue val url = c.getString(0) ?: continue
if (!isAllowedYtUrl(url)) continue
val title = c.getString(1) ?: continue val title = c.getString(1) ?: continue
val uploader = c.getString(2) ?: "" val uploader = c.getString(2) ?: ""
val thumb = c.getString(3) val thumb = c.getString(3)

View file

@ -99,6 +99,7 @@ import com.sulkta.straw.feature.player.LocalStrawController
import com.sulkta.straw.feature.player.NowPlaying import com.sulkta.straw.feature.player.NowPlaying
import com.sulkta.straw.feature.player.setPlayingFrom import com.sulkta.straw.feature.player.setPlayingFrom
import com.sulkta.straw.feature.search.StreamItem import com.sulkta.straw.feature.search.StreamItem
import com.sulkta.straw.util.LogDump
import com.sulkta.straw.util.formatCount import com.sulkta.straw.util.formatCount
import com.sulkta.straw.util.formatViews import com.sulkta.straw.util.formatViews
import com.sulkta.straw.util.stripHtml import com.sulkta.straw.util.stripHtml
@ -346,6 +347,11 @@ fun VideoDetailScreen(
// Make sure the controller is playing this video // Make sure the controller is playing this video
// before backing out — otherwise dropping to the // before backing out — otherwise dropping to the
// minibar would dismiss into an empty slot. // minibar would dismiss into an empty slot.
// Optimization: skip the MediaItem build if
// the controller is already on this URL.
// claim() in setPlayingFrom is the
// authoritative race-free guard — this
// check is just to avoid the work.
if (NowPlaying.current.value?.streamUrl != streamUrl) { if (NowPlaying.current.value?.streamUrl != streamUrl) {
val r = state.resolved val r = state.resolved
if (r == null) { if (r == null) {
@ -397,6 +403,11 @@ fun VideoDetailScreen(
Toast.makeText(context, "stream not ready", Toast.LENGTH_SHORT).show() Toast.makeText(context, "stream not ready", Toast.LENGTH_SHORT).show()
return@OutlinedButton return@OutlinedButton
} }
// Optimization: skip the MediaItem build if
// the controller is already on this URL.
// claim() in setPlayingFrom is the
// authoritative race-free guard — this
// check is just to avoid the work.
if (NowPlaying.current.value?.streamUrl != streamUrl) { if (NowPlaying.current.value?.streamUrl != streamUrl) {
c.setPlayingFrom( c.setPlayingFrom(
streamUrl = streamUrl, streamUrl = streamUrl,
@ -714,6 +725,7 @@ private fun InlinePlayer(
LaunchedEffect(controller, resolved, streamUrl) { LaunchedEffect(controller, resolved, streamUrl) {
val c = controller ?: return@LaunchedEffect val c = controller ?: return@LaunchedEffect
val r = resolved ?: return@LaunchedEffect val r = resolved ?: return@LaunchedEffect
// Optimization, not safety. claim() guards the race.
if (NowPlaying.current.value?.streamUrl == streamUrl) return@LaunchedEffect if (NowPlaying.current.value?.streamUrl == streamUrl) return@LaunchedEffect
c.setPlayingFrom( c.setPlayingFrom(
streamUrl = streamUrl, streamUrl = streamUrl,
@ -729,7 +741,14 @@ private fun InlinePlayer(
val c = controller val c = controller
val listener = object : Player.Listener { val listener = object : Player.Listener {
override fun onPlayerError(error: androidx.media3.common.PlaybackException) { override fun onPlayerError(error: androidx.media3.common.PlaybackException) {
playbackError = "${error.errorCodeName}: ${error.message ?: "(no message)"}" // Scrub the message — Media3's HttpDataSource exceptions
// include the full signed URL in .message. vc=36 audit
// CVE HIGH-1.
val raw = error.message ?: "(no message)"
playbackError = "${error.errorCodeName}: ${LogDump.scrubLine(raw)}"
// Clear NowPlaying so the minibar drops the dead
// session. vc=36 audit MED-3.
NowPlaying.clear()
} }
} }
c?.addListener(listener) c?.addListener(listener)

View file

@ -101,17 +101,23 @@ class VideoDetailViewModel : ViewModel() {
val title = info.title.ifBlank { "(no title)" } val title = info.title.ifBlank { "(no title)" }
val uploader = info.uploader val uploader = info.uploader
runCatching { // Move SP write off the main coroutine — recordWatch
History.get().recordWatch( // JSON-encodes the watch list (up to 50 entries) +
WatchHistoryItem( // sp.edit().apply(). Small but synchronous; vc=36
url = streamUrl, // audit Q9.
videoId = videoId, withContext(Dispatchers.IO) {
title = title, runCatching {
uploader = uploader, History.get().recordWatch(
thumbnail = thumb, WatchHistoryItem(
watchedAt = 0L, url = streamUrl,
), videoId = videoId,
) title = title,
uploader = uploader,
thumbnail = thumb,
watchedAt = 0L,
),
)
}
} }
val ryd = withContext(Dispatchers.IO) { val ryd = withContext(Dispatchers.IO) {
@ -152,7 +158,6 @@ class VideoDetailViewModel : ViewModel() {
title = v.title.ifBlank { "(no title)" }, title = v.title.ifBlank { "(no title)" },
uploader = v.uploader.ifBlank { uploader }, uploader = v.uploader.ifBlank { uploader },
uploaderUrl = v.uploaderUrl ?: uploaderUrl, uploaderUrl = v.uploaderUrl ?: uploaderUrl,
uploaderAvatar = ch.avatar,
thumbnail = v.thumbnail, thumbnail = v.thumbnail,
durationSeconds = v.durationSeconds, durationSeconds = v.durationSeconds,
viewCount = v.viewCount, viewCount = v.viewCount,

View file

@ -49,7 +49,11 @@ data class SubscriptionFeedUiState(
) )
class SubscriptionFeedViewModel : ViewModel() { class SubscriptionFeedViewModel : ViewModel() {
private val _ui = MutableStateFlow(SubscriptionFeedUiState()) // Seed loading=true: the init block always either hydrates from
// disk or fires a refresh, so the user should see the spinner
// (or cached content under it) rather than a one-frame flash of
// empty. vc=36 audit HIGH-R5.
private val _ui = MutableStateFlow(SubscriptionFeedUiState(loading = true))
val ui: StateFlow<SubscriptionFeedUiState> = _ui.asStateFlow() val ui: StateFlow<SubscriptionFeedUiState> = _ui.asStateFlow()
/** /**
@ -77,6 +81,7 @@ class SubscriptionFeedViewModel : ViewModel() {
channelCache.putAll(saved) channelCache.putAll(saved)
val channels = Subscriptions.get().subs.value val channels = Subscriptions.get().subs.value
if (channels.isNotEmpty()) { if (channels.isNotEmpty()) {
pruneCacheToSubs(channels)
_ui.value = _ui.value.copy( _ui.value = _ui.value.copy(
items = mergeFromCache(channels), items = mergeFromCache(channels),
lastFetchedAt = saved.values.maxOfOrNull { it.fetchedAt } ?: 0L, lastFetchedAt = saved.values.maxOfOrNull { it.fetchedAt } ?: 0L,
@ -112,6 +117,14 @@ class SubscriptionFeedViewModel : ViewModel() {
private var inFlight: Job? = null private var inFlight: Job? = null
fun refreshIfStale() { fun refreshIfStale() {
// Skip if a refresh is already in flight. vc=36 audit CRIT-R1:
// SubsPane's LaunchedEffect(subs) re-fires every time
// Subscriptions.updateAvatar emits a fresh list reference (which
// fetchChannelInto does opportunistically per channel). Without
// this gate, each per-channel avatar backfill cancels the
// parallel-12 batch and turns the refresh into N sequential
// single-channel fetches.
if (inFlight?.isActive == true) return
val now = System.currentTimeMillis() val now = System.currentTimeMillis()
val anyStale = Subscriptions.get().subs.value.any { ch -> val anyStale = Subscriptions.get().subs.value.any { ch ->
val entry = channelCache[ch.url] val entry = channelCache[ch.url]
@ -125,6 +138,14 @@ class SubscriptionFeedViewModel : ViewModel() {
if (channels.isEmpty()) { if (channels.isEmpty()) {
_ui.update { SubscriptionFeedUiState(loading = false, items = emptyList()) } _ui.update { SubscriptionFeedUiState(loading = false, items = emptyList()) }
channelCache.clear() channelCache.clear()
// Wipe disk too. vc=36 audit B1: previously the disk
// cache kept stale entries indefinitely after the user
// unsubscribed from everything. mergeFromCache eventually
// prunes them on the next merge, but they sat as orphans
// through cold starts in the meantime.
viewModelScope.launch(Dispatchers.IO) {
runCatching { FeedCache.get().clear() }
}
return return
} }
inFlight?.cancel() inFlight?.cancel()
@ -142,6 +163,7 @@ class SubscriptionFeedViewModel : ViewModel() {
.map { ch -> async { gate.withPermit { fetchChannelInto(ch) } } } .map { ch -> async { gate.withPermit { fetchChannelInto(ch) } } }
.awaitAll() .awaitAll()
} }
pruneCacheToSubs(channels)
_ui.update { _ui.update {
SubscriptionFeedUiState( SubscriptionFeedUiState(
loading = false, loading = false,
@ -189,7 +211,6 @@ class SubscriptionFeedViewModel : ViewModel() {
title = v.title.ifBlank { "(no title)" }, title = v.title.ifBlank { "(no title)" },
uploader = v.uploader.ifBlank { ch.name }, uploader = v.uploader.ifBlank { ch.name },
uploaderUrl = v.uploaderUrl ?: ch.url, uploaderUrl = v.uploaderUrl ?: ch.url,
uploaderAvatar = freshAvatar ?: ch.avatar,
thumbnail = v.thumbnail, thumbnail = v.thumbnail,
durationSeconds = v.durationSeconds, durationSeconds = v.durationSeconds,
viewCount = v.viewCount, viewCount = v.viewCount,
@ -211,18 +232,21 @@ class SubscriptionFeedViewModel : ViewModel() {
} }
} }
private fun mergeFromCache(channels: List<ChannelRef>): List<StreamItem> { private fun pruneCacheToSubs(channels: List<ChannelRef>) {
val subUrls = channels.map { it.url }.toSet() val subUrls = channels.map { it.url }.toSet()
// Drop cache entries for unsubscribed channels so removed subs
// fall out of the feed immediately. (Has a side effect on the
// ConcurrentHashMap — kept here for atomicity vs. a separate
// pass.)
channelCache.keys.toList().forEach { if (it !in subUrls) channelCache.remove(it) } channelCache.keys.toList().forEach { if (it !in subUrls) channelCache.remove(it) }
// Newest-first across channels. Pre-compute recencyScore once }
// per item — vc=35 audit MED-Q15: sortedWith's comparator was
// invoking the regex twice per pair, so ~1800 regex matches on private fun mergeFromCache(channels: List<ChannelRef>): List<StreamItem> {
// a 900-item merge. Pairing the score before sort drops that // Pure read. Caller is responsible for calling pruneCacheToSubs
// to N matches. // beforehand when channel-set changes matter — split here
// because the prior version's "merge" name hid a side-effecting
// prune that violated single-responsibility (vc=36 audit
// HIGH-R7).
//
// Pre-compute recencyScore once per item — vc=35 audit
// MED-Q15: sortedWith's comparator was invoking the regex
// twice per pair, so ~1800 regex matches on a 900-item merge.
return channels.flatMap { ch -> channelCache[ch.url]?.items.orEmpty() } return channels.flatMap { ch -> channelCache[ch.url]?.items.orEmpty() }
.map { it to it.recencyScore() } .map { it to it.recencyScore() }
.sortedWith( .sortedWith(

View file

@ -77,15 +77,22 @@ fun MinibarOverlay(
} }
// vc=35 audit MED-Q11: if Background-button took the user // vc=35 audit MED-Q11: if Background-button took the user
// to Home and the foreground audio fails, the only Player // to Home and the foreground audio fails, the only Player
// surface still listening is this minibar. Surface a Toast // surface still listening is this minibar.
// + clear NowPlaying so the minibar hides itself rather // vc=36 audit MED-3 + Q11: also stop the controller so a
// than claiming an already-dead session is "loaded". // future tap doesn't seek into the dead state, AND clear
// NowPlaying so the minibar hides itself. (PlayerScreen
// and VideoDetailScreen's listeners also clear NowPlaying
// now, so this is the fallback when neither is alive.)
override fun onPlayerError(error: androidx.media3.common.PlaybackException) { override fun onPlayerError(error: androidx.media3.common.PlaybackException) {
android.widget.Toast.makeText( android.widget.Toast.makeText(
ctx, ctx,
"playback error: ${error.errorCodeName}", "playback error: ${error.errorCodeName}",
android.widget.Toast.LENGTH_LONG, android.widget.Toast.LENGTH_LONG,
).show() ).show()
runCatching {
controller.stop()
controller.clearMediaItems()
}
NowPlaying.clear() NowPlaying.clear()
} }
} }

View file

@ -53,7 +53,15 @@ object NowPlaying {
fun claim(item: NowPlayingItem): Boolean { fun claim(item: NowPlayingItem): Boolean {
while (true) { while (true) {
val cur = _current.value val cur = _current.value
if (cur?.streamUrl == item.streamUrl) return false if (cur?.streamUrl == item.streamUrl) {
// Same URL — caller doesn't need to re-prepare the
// player, but if it brought richer metadata (full
// title vs the search-result truncation, fresh
// thumbnail, updated SponsorBlock segments) refresh
// those fields. vc=36 round-2 CVE MED-1.
if (cur != item) _current.compareAndSet(cur, item)
return false
}
if (_current.compareAndSet(cur, item)) return true if (_current.compareAndSet(cur, item)) return true
// Lost the CAS to a concurrent writer — retry against the // Lost the CAS to a concurrent writer — retry against the
// fresh state. Bounded: at most a handful of competing // fresh state. Bounded: at most a handful of competing

View file

@ -75,6 +75,7 @@ import androidx.media3.ui.PlayerView
import com.sulkta.straw.OverlayChromeColor import com.sulkta.straw.OverlayChromeColor
import com.sulkta.straw.feature.detail.VideoDetailViewModel import com.sulkta.straw.feature.detail.VideoDetailViewModel
import com.sulkta.straw.net.SbSegment import com.sulkta.straw.net.SbSegment
import com.sulkta.straw.util.LogDump
import com.sulkta.straw.util.strawLogI import com.sulkta.straw.util.strawLogI
import kotlinx.coroutines.delay import kotlinx.coroutines.delay
@ -108,6 +109,7 @@ fun PlayerScreen(
val r = resolved ?: return@LaunchedEffect val r = resolved ?: return@LaunchedEffect
val uploader = detail?.uploader.orEmpty() val uploader = detail?.uploader.orEmpty()
val thumbnail = detail?.thumbnail val thumbnail = detail?.thumbnail
// Optimization, not safety. claim() guards the race.
if (NowPlaying.current.value?.streamUrl == streamUrl) return@LaunchedEffect if (NowPlaying.current.value?.streamUrl == streamUrl) return@LaunchedEffect
c.setPlayingFrom( c.setPlayingFrom(
streamUrl = streamUrl, streamUrl = streamUrl,
@ -124,7 +126,17 @@ fun PlayerScreen(
val c = controller val c = controller
val listener = object : Player.Listener { val listener = object : Player.Listener {
override fun onPlayerError(error: androidx.media3.common.PlaybackException) { override fun onPlayerError(error: androidx.media3.common.PlaybackException) {
playbackError = "${error.errorCodeName}: ${error.message ?: "(no message)"}" // Scrub the message before rendering. Media3's
// HttpDataSource exceptions embed the full request URI
// (with signature= / pot= / cpn=) in the .message
// string — visible in the on-screen error banner and
// a screenshot away from being shared. vc=36 audit
// CVE HIGH-1.
val raw = error.message ?: "(no message)"
playbackError = "${error.errorCodeName}: ${LogDump.scrubLine(raw)}"
// Also clear NowPlaying so the minibar doesn't keep
// claiming a dead session is loaded. vc=36 audit MED-3.
NowPlaying.clear()
} }
} }
c?.addListener(listener) c?.addListener(listener)

View file

@ -37,7 +37,6 @@ data class StreamItem(
val title: String, val title: String,
val uploader: String, val uploader: String,
val uploaderUrl: String?, val uploaderUrl: String?,
val uploaderAvatar: String? = null,
val thumbnail: String?, val thumbnail: String?,
val durationSeconds: Long, val durationSeconds: Long,
val viewCount: Long, val viewCount: Long,
@ -51,18 +50,36 @@ class SearchViewModel : ViewModel() {
/** /**
* In-memory snapshot of the disk corpus (saved search results + * In-memory snapshot of the disk corpus (saved search results +
* subs feed cache) for reactive filtering. Hydrated on Dispatchers.IO * subs feed cache) for reactive filtering. Hydrated on
* once at VM construction and refreshed after a successful submit. * Dispatchers.IO once at VM construction and refreshed after a
* vc=34 audit CRIT the previous implementation hit * successful submit. vc=34 audit CRIT-C1 the previous
* SharedPreferences + JSON-decoded ~225 KB on every keystroke, * implementation hit SharedPreferences + JSON-decoded ~225 KB on
* blocking the main thread. * every keystroke, blocking the main thread.
*
* Plain @Volatile not StateFlow because nothing observes it
* (vc=36 audit LOW-R14 the StateFlow synchronization buys
* nothing here).
*/ */
private val pool = MutableStateFlow<List<StreamItem>>(emptyList()) @Volatile
private var pool: List<StreamItem> = emptyList()
init { init {
rebuildPool()
}
/**
* Re-read both caches off the main thread and replace the pool
* snapshot. Called at construction and from Settings when the
* cache toggle flips ON (so a re-enable picks up freshly-seeded
* entries from a subsequent submit/refresh without waiting for
* process death). vc=36 audit B2/Q10.
*/
fun rebuildPool() {
viewModelScope.launch { viewModelScope.launch {
if (Settings.get().cacheEnabled.value) { pool = if (Settings.get().cacheEnabled.value) {
pool.value = withContext(Dispatchers.IO) { buildPool() } withContext(Dispatchers.IO) { buildPool() }
} else {
emptyList()
} }
} }
} }
@ -73,10 +90,11 @@ class SearchViewModel : ViewModel() {
}.distinctBy { it.url } }.distinctBy { it.url }
fun onQueryChange(q: String) { fun onQueryChange(q: String) {
_ui.value = _ui.value.copy(query = q) // Clear any prior error state when the user resumes typing —
// Reactive filter: scan the in-memory `pool` as the user types. // a failed submit's banner used to persist into the next
// Pool is a List<StreamItem> walked once per keystroke — bounded // reactive preview, looking like the new query had failed.
// (~1500 items typical), no disk I/O, no JSON decode. // vc=36 audit Q3.
_ui.value = _ui.value.copy(query = q, error = null)
if (Settings.get().cacheEnabled.value && q.trim().length >= 2) { if (Settings.get().cacheEnabled.value && q.trim().length >= 2) {
val matches = reactiveFilter(q.trim()) val matches = reactiveFilter(q.trim())
if (matches.isNotEmpty()) { if (matches.isNotEmpty()) {
@ -84,16 +102,11 @@ class SearchViewModel : ViewModel() {
results = matches, results = matches,
fromCache = true, fromCache = true,
loading = false, loading = false,
error = null,
) )
} else if (_ui.value.fromCache) { } else if (_ui.value.fromCache) {
// User typed past what the cache can answer — drop the
// stale preview rather than leaving the prior query's
// results on screen pretending to match.
_ui.value = _ui.value.copy(results = emptyList(), fromCache = false) _ui.value = _ui.value.copy(results = emptyList(), fromCache = false)
} }
} else if (q.isBlank()) { } else if (q.isBlank()) {
// Clear cached preview if the box is cleared.
_ui.value = _ui.value.copy(results = emptyList(), fromCache = false) _ui.value = _ui.value.copy(results = emptyList(), fromCache = false)
} }
} }
@ -102,10 +115,13 @@ class SearchViewModel : ViewModel() {
val q = _ui.value.query.trim() val q = _ui.value.query.trim()
if (q.isEmpty()) return if (q.isEmpty()) return
// Cache hit on submit: show immediately, kick off a refresh // Cache hit on submit: show immediately, kick off refresh
// behind it so the user gets fresh items shortly after. // behind it. vc=36 audit B3 — the previous shape called
// `SearchCache.get().load()` on the main thread, doing the
// exact ~150 KB JSON decode the reactive-filter fix was
// supposed to eliminate. Now uses the StateFlow snapshot.
val cached = if (Settings.get().cacheEnabled.value) { val cached = if (Settings.get().cacheEnabled.value) {
SearchCache.get().load() SearchCache.get().entries.value
.firstOrNull { it.query.equals(q, ignoreCase = true) } .firstOrNull { it.query.equals(q, ignoreCase = true) }
?.items ?.items
} else null } else null

View file

@ -52,7 +52,10 @@ import com.sulkta.straw.data.ThemeMode
import com.sulkta.straw.feature.dataimport.ImportResult import com.sulkta.straw.feature.dataimport.ImportResult
import com.sulkta.straw.feature.dataimport.SettingsImport import com.sulkta.straw.feature.dataimport.SettingsImport
import com.sulkta.straw.feature.feed.SubscriptionFeedViewModel import com.sulkta.straw.feature.feed.SubscriptionFeedViewModel
import com.sulkta.straw.feature.search.SearchViewModel
import com.sulkta.straw.util.LogDump import com.sulkta.straw.util.LogDump
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
@Composable @Composable
@ -203,19 +206,31 @@ fun SettingsScreen() {
fontWeight = FontWeight.SemiBold, fontWeight = FontWeight.SemiBold,
) )
val feedVm: SubscriptionFeedViewModel = viewModel() val feedVm: SubscriptionFeedViewModel = viewModel()
val searchVm: SearchViewModel = viewModel()
Switch( Switch(
checked = cacheEnabled, checked = cacheEnabled,
onCheckedChange = { checked -> onCheckedChange = { checked ->
store.setCacheEnabled(checked) store.setCacheEnabled(checked)
if (!checked) { scope.launch {
// Wipe on disable — leaving stale bytes around if (!checked) {
// defeats the purpose of opting out. withContext(Dispatchers.IO) {
FeedCache.get().clear() runCatching { FeedCache.get().clear() }
SearchCache.get().clear() runCatching { SearchCache.get().clear() }
// vc=35 audit MED-C13 — wipe the in-memory }
// copy too, otherwise items stayed visible feedVm.clearInMemoryCache()
// until process death. // Drop the in-memory reactive-search pool
feedVm.clearInMemoryCache() // too — without this, typing into Search
// still surfaces hits from the just-wiped
// disk cache.
searchVm.rebuildPool()
} else {
// Cache re-enabled: trigger a real refresh
// so the feed repopulates without waiting
// for the user to navigate away and back.
// vc=36 audit B2.
feedVm.refresh()
searchVm.rebuildPool()
}
} }
}, },
) )

View file

@ -96,12 +96,20 @@ object LogDump {
* disk. Cheap line-level pass adversarial-perfect would need a * disk. Cheap line-level pass adversarial-perfect would need a
* URL parser, but the regex approach catches every documented * URL parser, but the regex approach catches every documented
* leak vector at zero allocation cost. * leak vector at zero allocation cost.
*
* Public so error-handler call sites (PlayerScreen / VideoDetail
* `playbackError`) can scrub Media3's `PlaybackException.message`
* before rendering it to the user that string includes the full
* request URI for HttpDataSource exceptions, which would otherwise
* be a leak via screenshot. vc=36 audit CVE HIGH-1.
*/ */
internal fun scrubLine(line: String): String { fun scrubLine(line: String): String {
var s = line var s = line
// Pre-signed googlevideo URLs: keep host visible, drop path+query. // Pre-signed googlevideo URLs: keep host visible, drop path+query.
s = GOOGLEVIDEO_URL_RE.replace(s, "https://<host>.googlevideo.com/<scrubbed>") s = GOOGLEVIDEO_URL_RE.replace(s, "https://<host>.googlevideo.com/<scrubbed>")
// Any remaining signed-param shapes that snuck through other URLs. // Any remaining signed-param shapes that snuck through other URLs.
// Expanded set vc=36 audit CVE MED-2: + n (JS-deobfuscated n-sig),
// lsig (link signature), ei (encrypted event-id), key, sparams.
s = SIGNED_PARAM_RE.replace(s, "$1=<scrubbed>") s = SIGNED_PARAM_RE.replace(s, "$1=<scrubbed>")
return s return s
} }
@ -110,7 +118,7 @@ object LogDump {
"""https?://[a-zA-Z0-9.-]*googlevideo\.com/\S+""", """https?://[a-zA-Z0-9.-]*googlevideo\.com/\S+""",
) )
private val SIGNED_PARAM_RE = Regex( private val SIGNED_PARAM_RE = Regex(
"""\b(signature|sig|pot|cpn|expire|ip|mn|ms|mo|pl)=([^&\s"']+)""", """\b(signature|sig|pot|cpn|expire|ip|mn|ms|mo|pl|n|lsig|ei|key|sparams)=([^&\s"']+)""",
RegexOption.IGNORE_CASE, RegexOption.IGNORE_CASE,
) )
} }