From 457166e3b00f80e9faa2d491ebbbe7db4492dc9a Mon Sep 17 00:00:00 2001 From: Cobb Date: Sun, 21 Jun 2026 20:03:45 -0700 Subject: [PATCH] vc=88: deferred-hygiene sweep (audit #2 leftovers, no behavior change) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M-2: route every SharedPreferences write (Settings/History/Subs/Resume) through one PrefsWriter — a per-store single-thread dispatcher — so the on-disk apply() order matches the in-memory CAS order. Previously a Main-thread toggle and an IO-thread import could land apply() out of order, and ResumePositions detached ordering entirely via a fresh globalScope.launch per write; a stale value could then win the next cold-start load. Each write reads the live StateFlow so disk converges to the latest in-memory state regardless of enqueue order. L-14: Settings storage-usage sampling (File.length() x4 + Coil diskCache.size) moved off the composition/Main thread into a LaunchedEffect on Dispatchers.IO. L-2 / L-4..L-8 / L-15 / L-16: dead code + stale comments from the vc=85 SB/RYD to Rust migration. Http.kt trimmed to STRAW_USER_AGENT; reconciled the network_security_config / feed.rs / SubscriptionFeedViewModel / net.rs / CI comments with reality; recencyScore overflow-guarded; ci/Dockerfile now pre-installs build-tools 36 (AGP 9.2.1's actual floor, was auto-fetched). Verified: headless compileDebugKotlin green on the straw-build image. --- .forgejo/workflows/build.yml | 5 +- buildSrc/src/main/kotlin/ProjectConfig.kt | 23 +++++- ci/Dockerfile | 13 +++- rust/strawcore/src/feed.rs | 17 +++-- rust/strawcore/src/net.rs | 5 ++ .../com/sulkta/straw/data/HistoryStore.kt | 16 +++-- .../com/sulkta/straw/data/PrefsWriter.kt | 47 ++++++++++++ .../sulkta/straw/data/ResumePositionsStore.kt | 24 +++---- .../com/sulkta/straw/data/SettingsStore.kt | 72 +++++++++++-------- .../sulkta/straw/data/SubscriptionsStore.kt | 23 +++--- .../feature/feed/SubscriptionFeedViewModel.kt | 37 +++++----- .../straw/feature/settings/SettingsScreen.kt | 66 +++++++++++------ .../main/kotlin/com/sulkta/straw/net/Http.kt | 69 ++---------------- .../main/res/xml/network_security_config.xml | 22 ++++-- 14 files changed, 256 insertions(+), 183 deletions(-) create mode 100644 strawApp/src/main/kotlin/com/sulkta/straw/data/PrefsWriter.kt diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index 78dadc53c..457987c5b 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -85,7 +85,10 @@ jobs: echo "Built vc=$VC -> $NAME" # The whole series is signed with SHA-1 bb9ca96b...; fail loudly if a # build ever produces a different signer (would break in-place updates). - # Pick whatever build-tools the image actually ships (36 today, not 34). + # apksigner lives under build-tools//. AGP (compileSdk 36) needs + # build-tools 36; older straw-build images pre-installed only 34.0.0 and + # AGP auto-fetched 36 at build time, so this sort -V | tail -1 resolves to + # 36.0.0's apksigner. (ci/Dockerfile now pre-installs 36 too — see there.) # apksigner is a shell wrapper that needs `java` on PATH; the image # sets JAVA_HOME but doesn't put its bin on PATH for run steps (gradle # uses JAVA_HOME directly, so the build itself is fine). diff --git a/buildSrc/src/main/kotlin/ProjectConfig.kt b/buildSrc/src/main/kotlin/ProjectConfig.kt index b90dbd177..8626b2372 100644 --- a/buildSrc/src/main/kotlin/ProjectConfig.kt +++ b/buildSrc/src/main/kotlin/ProjectConfig.kt @@ -9,6 +9,25 @@ const val STRAW_SDK_TARGET = 35 // Sulkta fork — Straw // +// vc=88 / 0.1.0-CV — deferred-hygiene sweep (audit #2 leftovers, no behavior change): +// * SharedPreferences writes across every store (Settings, History, Subs, +// ResumePositions) now route through one PrefsWriter — a single-thread +// dispatcher per store — so the on-disk apply() order matches the in-memory +// CAS order. Previously a Main-thread toggle and an IO-thread settings/history +// import could land their apply() out of order, and ResumePositions detached +// write-ordering entirely via a fresh globalScope.launch per write; a stale +// value could then win the next cold-start load. Each write reads the live +// StateFlow so disk always converges to the latest in-memory state. (M-2) +// * Settings storage-usage sampling (File.length() ×4 + Coil diskCache.size) +// moved off the composition/Main thread into a LaunchedEffect on +// Dispatchers.IO — was real synchronous FS I/O stalling the first Settings +// frame. (L-14) +// * Dead code + stale comments from the vc=85 SB/RYD→Rust migration: Http.kt +// trimmed to just STRAW_USER_AGENT (the OkHttp client + bounded-body reader +// went to Rust); reconciled the network_security_config / feed.rs / +// SubscriptionFeedViewModel / net.rs / CI comments with reality. recencyScore +// now overflow-guards a crafted relative-date. (L-2 / L-4..L-8 / L-15 / L-16) +// // vc=87 / 0.1.0-CU — audit #2 fix sprint (closes two vc=86 regressions + more): // * FIX a duplicate-key crash: the Subs feed, Search, and Channel lists key // their LazyColumn by the video url, but the sources weren't fully deduped @@ -247,6 +266,6 @@ const val STRAW_SDK_TARGET = 35 // 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 = 87 -const val STRAW_VERSION_NAME = "0.1.0-CU" +const val STRAW_VERSION_CODE = 88 +const val STRAW_VERSION_NAME = "0.1.0-CV" const val STRAW_APPLICATION_ID = "com.sulkta.straw" diff --git a/ci/Dockerfile b/ci/Dockerfile index a0b875f48..fc1072623 100644 --- a/ci/Dockerfile +++ b/ci/Dockerfile @@ -6,9 +6,13 @@ # FRESH Forgejo CI job container is fully self-contained (no host /caches # dependency, no per-machine signing key). # -# Toolchain pinned to exactly what builds vc=72 successfully: -# JDK 21 · NDK 27.2.12479018 · build-tools 34.0.0 · platforms android-36 +# Toolchain pinned to exactly what builds successfully: +# JDK 21 · NDK 27.2.12479018 · build-tools 34.0.0 + 36.0.0 · platforms android-36 # Rust stable + 4 Android targets · cargo-ndk · clang/libclang (rquickjs bindgen) +# AGP 9.2.1 (compileSdk 36) requires build-tools 36; we pre-install it so the build +# doesn't sdkmanager-download it on every run (older images shipped only 34.0.0 and +# AGP auto-fetched 36 at build time — works, but a per-build network dependency). +# 34.0.0 is kept for belt-and-suspenders; apksigner uses the highest present (36). FROM eclipse-temurin:21-jdk-jammy ENV DEBIAN_FRONTEND=noninteractive \ @@ -37,6 +41,7 @@ RUN mkdir -p "$ANDROID_SDK_ROOT/cmdline-tools" \ "platform-tools" \ "platforms;android-36" \ "build-tools;34.0.0" \ + "build-tools;36.0.0" \ "ndk;27.2.12479018" >/dev/null \ && rm -rf "$ANDROID_SDK_ROOT/.temp" /tmp/* @@ -49,7 +54,9 @@ RUN curl -fsSL https://sh.rustup.rs | sh -s -- -y --default-toolchain stable --p # Sanity: fail the image build early if anything's missing. RUN java -version && cargo --version && cargo ndk --version || true \ - && test -d "$ANDROID_NDK_HOME" && test -d "$ANDROID_SDK_ROOT/build-tools/34.0.0" + && test -d "$ANDROID_NDK_HOME" \ + && test -d "$ANDROID_SDK_ROOT/build-tools/34.0.0" \ + && test -d "$ANDROID_SDK_ROOT/build-tools/36.0.0" # Publish tooling (appended last so the heavy toolchain layers stay cached): # docker CLI to talk to the runner's host socket for the fdroid steps, and diff --git a/rust/strawcore/src/feed.rs b/rust/strawcore/src/feed.rs index bc5e5c92f..ecf7e4015 100644 --- a/rust/strawcore/src/feed.rs +++ b/rust/strawcore/src/feed.rs @@ -105,12 +105,17 @@ pub async fn channel_feed_rss( crate::runtime::ensure_initialized(); log::info!("strawcore::channel_feed_rss url_len={}", channel_url.len()); let client = rss_client()?; - // Propagate the real failure (network / HTTP / parse) instead of - // collapsing it to an empty Vec. The single-channel caller (the subs - // feed) needs to tell "this channel posted nothing" apart from "this - // fetch broke" — the prior `.unwrap_or_default()` made the `Result` - // a lie. (subscription_feed keeps its own per-channel unwrap_or_default - // below — fan-out tolerance is correct there, not here.) + // Propagate the real failure (network / HTTP / parse) as Err rather than + // collapsing it to an empty Vec. The current Android caller flattens + // Err -> emptyList() and only overwrites its per-channel cache on a + // NON-empty result, so a transient fetch error leaves the prior cache + // intact instead of blanking the channel — which is exactly why the old + // `.unwrap_or_default()` here was wrong (it turned a broken fetch into an + // authoritative "no videos"). Note an unsupported URL (`extract_channel_id` + // -> None) and a genuinely empty feed both still return Ok(vec![]); the + // Err vs Ok distinction only separates *broken* from *empty*. + // subscription_feed keeps its own per-channel unwrap_or_default below — + // fan-out tolerance is correct there, not here. fetch_channel_rss(client, &channel_url).await } diff --git a/rust/strawcore/src/net.rs b/rust/strawcore/src/net.rs index 1ceca7391..95ef78646 100644 --- a/rust/strawcore/src/net.rs +++ b/rust/strawcore/src/net.rs @@ -57,6 +57,11 @@ fn client() -> Option<&'static Client> { /// exceeds `cap`. Shared with the RSS feed path (`feed.rs`). Per-chunk /// guard first (HTTP allows multi-GiB chunks; hyper may have already /// allocated one before we see it), then the running total. +/// +/// The cap bounds gzip-DECOMPRESSED bytes (reqwest is built with `gzip`), +/// not wire bytes — same as the old OkHttp `cappedString`, and fine for the +/// few-hundred-byte-to-low-KB RYD / SponsorBlock / RSS payloads. A hostile +/// host could force up-to-`cap` decompression, but never enough to OOM. pub(crate) async fn read_capped_body(resp: reqwest::Response, cap: usize) -> Option { use futures::StreamExt; let mut total = 0usize; diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/HistoryStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/HistoryStore.kt index 7e12e200f..5bc698a3c 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/HistoryStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/HistoryStore.kt @@ -48,6 +48,10 @@ private const val MAX_SEARCHES_HARD = 100_000 class HistoryStore(context: Context) { private val sp: SharedPreferences = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) private val json = Json { ignoreUnknownKeys = true } + // Writes serialized so a Main-thread recordWatch can't land its apply() + // out of order vs an IO-thread bulk import (audit #2 M-2). Each write + // reads the live StateFlow so disk converges to the latest list. + private val writer = PrefsWriter(sp) private fun maxWatches(): Int { val cap = Settings.get().historyWatchesCap.value.value @@ -79,7 +83,7 @@ class HistoryStore(context: Context) { val without = current.filterNot { it.videoId == item.videoId } (listOf(merged) + without).take(maxWatches()) } - sp.edit().putString(KEY_WATCHES, json.encodeToString(next)).apply() + writer.write { putString(KEY_WATCHES, json.encodeToString(_watches.value)) } } /** @@ -135,7 +139,7 @@ class HistoryStore(context: Context) { (fresh + current).take(maxWatches()) } if (next !== before) { - sp.edit().putString(KEY_WATCHES, json.encodeToString(next)).apply() + writer.write { putString(KEY_WATCHES, json.encodeToString(_watches.value)) } } return counter.get() } @@ -171,7 +175,7 @@ class HistoryStore(context: Context) { (fresh + current).take(maxSearches()) } if (next !== before) { - sp.edit().putString(KEY_SEARCHES, json.encodeToString(next)).apply() + writer.write { putString(KEY_SEARCHES, json.encodeToString(_searches.value)) } } return counter.get() } @@ -183,17 +187,17 @@ class HistoryStore(context: Context) { val without = current.filterNot { it.equals(q, ignoreCase = true) } (listOf(q) + without).take(maxSearches()) } - sp.edit().putString(KEY_SEARCHES, json.encodeToString(next)).apply() + writer.write { putString(KEY_SEARCHES, json.encodeToString(_searches.value)) } } fun clearWatches() { _watches.updateAndGet { emptyList() } - sp.edit().putString(KEY_WATCHES, json.encodeToString(emptyList())).apply() + writer.write { putString(KEY_WATCHES, json.encodeToString(_watches.value)) } } fun clearSearches() { _searches.updateAndGet { emptyList() } - sp.edit().putString(KEY_SEARCHES, json.encodeToString(emptyList())).apply() + writer.write { putString(KEY_SEARCHES, json.encodeToString(_searches.value)) } } private fun loadWatches(): List = runCatching { diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/PrefsWriter.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/PrefsWriter.kt new file mode 100644 index 000000000..d4f6f21ab --- /dev/null +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/PrefsWriter.kt @@ -0,0 +1,47 @@ +/* + * SPDX-FileCopyrightText: 2026 Sulkta-Coop + * SPDX-License-Identifier: GPL-3.0-or-later + * + * Serializes one SharedPreferences file's writes onto a single thread so + * the on-disk apply() order matches the in-memory CAS order (audit #2 M-2). + * + * Every store here updates its MutableStateFlow atomically (updateAndGet), + * but the follow-up SharedPreferences write used to be dispatched + * unsequenced: two concurrent writers (e.g. the IO-thread settings importer + * vs a Main-thread toggle, or a player seek vs the 5s resume-poll) could + * land their apply() calls out of order, leaving disk holding the OLDER + * value while the StateFlow correctly held the newer one — so the stale + * value won on the next cold-start load(). + * + * Each store owns one PrefsWriter. The write lambda runs on a single-thread + * dispatcher and MUST read the store's StateFlow `.value` (not a captured + * snapshot): whichever serialized write executes last then persists the + * latest in-memory state, so disk always converges to it regardless of + * which writer was enqueued first. + */ + +package com.sulkta.straw.data + +import android.content.SharedPreferences +import com.sulkta.straw.StrawApp +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch + +class PrefsWriter(private val sp: SharedPreferences) { + // limitedParallelism(1): a single-worker view of the shared IO pool — + // tasks run one at a time, in dispatch order, so apply() order is stable. + private val dispatcher = Dispatchers.IO.limitedParallelism(1) + + /** + * Enqueue a serialized SharedPreferences write. The block runs with the + * Editor as receiver; read the store's StateFlow `.value` inside it (not + * a value captured at call time) so the latest in-memory state wins. + */ + fun write(block: SharedPreferences.Editor.() -> Unit) { + StrawApp.globalScope.launch(dispatcher) { + val editor = sp.edit() + editor.block() + editor.apply() + } + } +} diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/ResumePositionsStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/ResumePositionsStore.kt index ffa02cd5f..81bb83916 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/ResumePositionsStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/ResumePositionsStore.kt @@ -17,13 +17,10 @@ package com.sulkta.straw.data import android.content.Context import android.content.SharedPreferences -import com.sulkta.straw.StrawApp -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.updateAndGet -import kotlinx.coroutines.launch import kotlinx.serialization.Serializable import kotlinx.serialization.json.Json @@ -63,6 +60,7 @@ private const val END_THRESHOLD_MS = 5_000L class ResumePositionsStore(context: Context) { private val sp: SharedPreferences = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) private val json = Json { ignoreUnknownKeys = true } + private val writer = PrefsWriter(sp) private val _positions = MutableStateFlow(load()) val positions: StateFlow> = _positions.asStateFlow() @@ -131,12 +129,12 @@ class ResumePositionsStore(context: Context) { } } if (next !== before) { - // JSON encode + SP write off Main — encoding 100k entries - // would be ~50-100 ms on a low-end device, and the 5s - // captureResumePosition poll runs on Main. - StrawApp.globalScope.launch(Dispatchers.IO) { - sp.edit().putString(KEY_POSITIONS, json.encodeToString(next)).apply() - } + // JSON encode + SP write off Main (encoding 100k entries is + // ~50-100 ms on a low-end device, and the 5s captureResumePosition + // poll runs on Main), serialized so a quick-succession seek + poll + // tick can't land their apply() out of order (audit #2 M-2). The + // write reads `_positions.value`, so it persists the latest map. + writer.write { putString(KEY_POSITIONS, json.encodeToString(_positions.value)) } } } @@ -153,9 +151,7 @@ class ResumePositionsStore(context: Context) { if (videoId !in current) current else current - videoId } if (next !== before) { - StrawApp.globalScope.launch(Dispatchers.IO) { - sp.edit().putString(KEY_POSITIONS, json.encodeToString(next)).apply() - } + writer.write { putString(KEY_POSITIONS, json.encodeToString(_positions.value)) } } } @@ -163,9 +159,7 @@ class ResumePositionsStore(context: Context) { val before = _positions.value _positions.updateAndGet { emptyMap() } if (before.isNotEmpty()) { - StrawApp.globalScope.launch(Dispatchers.IO) { - sp.edit().putString(KEY_POSITIONS, json.encodeToString(emptyMap())).apply() - } + writer.write { putString(KEY_POSITIONS, json.encodeToString(_positions.value)) } } } 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 279440f19..fefc1ad53 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt @@ -144,6 +144,14 @@ private const val KEY_BG_FEED_REFRESH_INTERVAL = "bg_feed_refresh_interval_v1" class SettingsStore(context: Context) { private val sp: SharedPreferences = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) + // SP writes serialized off the calling thread (audit #2 M-2): the + // importer toggles SponsorBlock categories on Dispatchers.IO while the + // user can tap the same toggles on Main — an unsequenced apply() could + // otherwise revert a user toggle after restart. Each setter updates its + // StateFlow synchronously, then the serialized write reads `.value` so + // disk converges to the latest in-memory state. + private val writer = PrefsWriter(sp) + private val _sbCategories = MutableStateFlow(loadCategories()) val sbCategories: StateFlow> = _sbCategories.asStateFlow() @@ -311,90 +319,92 @@ class SettingsStore(context: Context) { _bgFeedRefreshInterval.asStateFlow() fun toggle(cat: SbCategory) { - // Atomic toggle via updateAndGet — see AUD-HIGH note in HistoryStore. - val next = _sbCategories.updateAndGet { cur -> + // Atomic toggle via updateAndGet; the serialized write reads the live + // set so an IO-thread import + a Main-thread tap converge (audit #2 M-2). + _sbCategories.updateAndGet { cur -> if (cat in cur) cur - cat else cur + cat } - sp.edit().putStringSet(KEY_SB_CATS, next.map { it.key }.toSet()).apply() + writer.write { putStringSet(KEY_SB_CATS, _sbCategories.value.map { it.key }.toSet()) } } - // Atomic + idempotent. Capture before-state, update in-memory, - // skip the SP write when the value didn't actually change. The - // prior shape used `updateAndGet { r } == r` which is unconditionally - // true (the lambda ignores prior) — dead code that confused readers. + // Atomic + idempotent. Capture before-state, update in-memory, skip the SP + // write when the value didn't change. The serialized write reads the live + // StateFlow so disk converges to the latest value (audit #2 M-2). The prior + // shape used `updateAndGet { r } == r` which is unconditionally true (the + // lambda ignores prior) — dead code that confused readers. fun setMaxResolution(r: MaxResolution) { val before = _maxResolution.value if (before == r) return _maxResolution.value = r - sp.edit().putString(KEY_MAX_RES, r.name).apply() + writer.write { putString(KEY_MAX_RES, _maxResolution.value.name) } } fun setThemeMode(t: ThemeMode) { val before = _themeMode.value if (before == t) return _themeMode.value = t - sp.edit().putString(KEY_THEME, t.name).apply() + writer.write { putString(KEY_THEME, _themeMode.value.name) } } fun setCacheEnabled(enabled: Boolean) { val before = _cacheEnabled.value if (before == enabled) return _cacheEnabled.value = enabled - sp.edit().putBoolean(KEY_CACHE_ENABLED, enabled).apply() + writer.write { putBoolean(KEY_CACHE_ENABLED, _cacheEnabled.value) } } fun setAutoplayMode(mode: AutoplayMode) { val before = _autoplayMode.value if (before == mode) return _autoplayMode.value = mode - sp.edit().putString(KEY_AUTOPLAY_MODE, mode.name).apply() + writer.write { putString(KEY_AUTOPLAY_MODE, _autoplayMode.value.name) } } fun setAutoplaySkipWatched(skip: Boolean) { val before = _autoplaySkipWatched.value if (before == skip) return _autoplaySkipWatched.value = skip - sp.edit().putBoolean(KEY_AUTOPLAY_SKIP_WATCHED, skip).apply() + writer.write { putBoolean(KEY_AUTOPLAY_SKIP_WATCHED, _autoplaySkipWatched.value) } } fun setAutoStartPlayback(autoStart: Boolean) { val before = _autoStartPlayback.value if (before == autoStart) return _autoStartPlayback.value = autoStart - sp.edit().putBoolean(KEY_AUTOSTART_PLAYBACK, autoStart).apply() + writer.write { putBoolean(KEY_AUTOSTART_PLAYBACK, _autoStartPlayback.value) } } fun setPauseOnHeadphoneDisconnect(pause: Boolean) { val before = _pauseOnHeadphoneDisconnect.value if (before == pause) return _pauseOnHeadphoneDisconnect.value = pause - sp.edit().putBoolean(KEY_PAUSE_ON_HEADPHONE_DISCONNECT, pause).apply() + writer.write { putBoolean(KEY_PAUSE_ON_HEADPHONE_DISCONNECT, _pauseOnHeadphoneDisconnect.value) } } fun setAutoResume(enabled: Boolean) { val before = _autoResume.value if (before == enabled) return _autoResume.value = enabled - sp.edit().putBoolean(KEY_AUTO_RESUME, enabled).apply() + writer.write { putBoolean(KEY_AUTO_RESUME, _autoResume.value) } } fun setAutoUpdateCheck(enabled: Boolean) { val before = _autoUpdateCheck.value if (before == enabled) return _autoUpdateCheck.value = enabled - sp.edit().putBoolean(KEY_AUTO_UPDATE_CHECK, enabled).apply() + writer.write { putBoolean(KEY_AUTO_UPDATE_CHECK, _autoUpdateCheck.value) } } fun setAutoUpdateInterval(interval: AutoUpdateInterval) { val before = _autoUpdateInterval.value if (before == interval) return _autoUpdateInterval.value = interval - sp.edit().putString(KEY_AUTO_UPDATE_INTERVAL, interval.name).apply() + writer.write { putString(KEY_AUTO_UPDATE_INTERVAL, _autoUpdateInterval.value.name) } } fun setLastUpdateCheck(ms: Long) { _lastUpdateCheckMs.value = ms - sp.edit().putLong(KEY_LAST_UPDATE_CHECK_MS, ms).apply() + writer.write { putLong(KEY_LAST_UPDATE_CHECK_MS, _lastUpdateCheckMs.value) } } fun setLatestKnownVersion(vc: Long, vname: String) { @@ -404,65 +414,65 @@ class SettingsStore(context: Context) { if (_latestKnownVc.value == vc && _latestKnownVname.value == vname) return _latestKnownVc.value = vc _latestKnownVname.value = vname - sp.edit() - .putLong(KEY_LATEST_KNOWN_VC, vc) - .putString(KEY_LATEST_KNOWN_VNAME, vname) - .apply() + writer.write { + putLong(KEY_LATEST_KNOWN_VC, _latestKnownVc.value) + putString(KEY_LATEST_KNOWN_VNAME, _latestKnownVname.value) + } } fun setHideShorts(hide: Boolean) { val before = _hideShorts.value if (before == hide) return _hideShorts.value = hide - sp.edit().putBoolean(KEY_HIDE_SHORTS, hide).apply() + writer.write { putBoolean(KEY_HIDE_SHORTS, _hideShorts.value) } } fun setHideWatched(hide: Boolean) { if (_hideWatched.value == hide) return _hideWatched.value = hide - sp.edit().putBoolean(KEY_HIDE_WATCHED, hide).apply() + writer.write { putBoolean(KEY_HIDE_WATCHED, _hideWatched.value) } } fun setHistoryWatchesCap(cap: CacheCap) { if (_historyWatchesCap.value == cap) return _historyWatchesCap.value = cap - sp.edit().putInt(KEY_CACHE_HISTORY_WATCHES, cap.value).apply() + writer.write { putInt(KEY_CACHE_HISTORY_WATCHES, _historyWatchesCap.value.value) } } fun setHistorySearchesCap(cap: CacheCap) { if (_historySearchesCap.value == cap) return _historySearchesCap.value = cap - sp.edit().putInt(KEY_CACHE_HISTORY_SEARCHES, cap.value).apply() + writer.write { putInt(KEY_CACHE_HISTORY_SEARCHES, _historySearchesCap.value.value) } } fun setResumePositionsCap(cap: CacheCap) { if (_resumePositionsCap.value == cap) return _resumePositionsCap.value = cap - sp.edit().putInt(KEY_CACHE_RESUME_POSITIONS, cap.value).apply() + writer.write { putInt(KEY_CACHE_RESUME_POSITIONS, _resumePositionsCap.value.value) } } fun setSearchCacheCap(cap: CacheCap) { if (_searchCacheCap.value == cap) return _searchCacheCap.value = cap - sp.edit().putInt(KEY_CACHE_SEARCH, cap.value).apply() + writer.write { putInt(KEY_CACHE_SEARCH, _searchCacheCap.value.value) } } fun setCacheTtl(ttl: CacheTtl) { if (_cacheTtl.value == ttl) return _cacheTtl.value = ttl - sp.edit().putString(KEY_CACHE_TTL, ttl.name).apply() + writer.write { putString(KEY_CACHE_TTL, _cacheTtl.value.name) } } fun setBgFeedRefreshEnabled(enabled: Boolean) { if (_bgFeedRefreshEnabled.value == enabled) return _bgFeedRefreshEnabled.value = enabled - sp.edit().putBoolean(KEY_BG_FEED_REFRESH_ENABLED, enabled).apply() + writer.write { putBoolean(KEY_BG_FEED_REFRESH_ENABLED, _bgFeedRefreshEnabled.value) } } fun setBgFeedRefreshInterval(interval: BgFeedRefreshInterval) { if (_bgFeedRefreshInterval.value == interval) return _bgFeedRefreshInterval.value = interval - sp.edit().putString(KEY_BG_FEED_REFRESH_INTERVAL, interval.name).apply() + writer.write { putString(KEY_BG_FEED_REFRESH_INTERVAL, _bgFeedRefreshInterval.value.name) } } private fun loadCap(key: String, default: Int): CacheCap = diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/SubscriptionsStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/SubscriptionsStore.kt index f25a6bbc0..c82696a01 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/SubscriptionsStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/SubscriptionsStore.kt @@ -30,6 +30,9 @@ private const val KEY = "subs_v1" class SubscriptionsStore(context: Context) { private val sp: SharedPreferences = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) private val json = Json { ignoreUnknownKeys = true } + // Writes serialized so a Main-thread toggle and an IO-thread bulk import + // can't land their apply() out of order (audit #2 M-2). + private val writer = PrefsWriter(sp) private val _subs = MutableStateFlow(load()) val subs: StateFlow> = _subs.asStateFlow() @@ -41,14 +44,14 @@ class SubscriptionsStore(context: Context) { // updateAndGet makes the read-modify-write atomic vs. concurrent // toggles (e.g. one channel subscribed from the feed while another // is unsubscribed from VideoDetail). - val next = _subs.updateAndGet { cur -> + _subs.updateAndGet { cur -> if (cur.any { it.url == ref.url }) { cur.filterNot { it.url == ref.url } } else { cur + ref } } - persist(next) + persist() } /** @@ -58,10 +61,10 @@ class SubscriptionsStore(context: Context) { * it at subscribe time). No-op for non-subscribed URLs. */ fun updateAvatar(channelUrl: String, avatar: String) { - val next = _subs.updateAndGet { cur -> + _subs.updateAndGet { cur -> cur.map { if (it.url == channelUrl) it.copy(avatar = avatar) else it } } - persist(next) + persist() } /** @@ -80,7 +83,7 @@ class SubscriptionsStore(context: Context) { // ). The counter lives in an // AtomicInteger so each lambda re-run resets it correctly. val counter = java.util.concurrent.atomic.AtomicInteger(0) - val next = _subs.updateAndGet { state -> + _subs.updateAndGet { state -> counter.set(0) val byUrl = state.associateBy { it.url }.toMutableMap() for (r in refs) { @@ -92,7 +95,7 @@ class SubscriptionsStore(context: Context) { } byUrl.values.toList() } - persist(next) + persist() return counter.get() } @@ -101,11 +104,13 @@ class SubscriptionsStore(context: Context) { // toggle racing the clear and persisting [new-item] after the // remove() call has fired. _subs.updateAndGet { emptyList() } - persist(emptyList()) + persist() } - private fun persist(list: List) { - sp.edit().putString(KEY, json.encodeToString(list)).apply() + // Reads `_subs.value` (not a captured list) so the serialized write + // always persists the latest in-memory state — see PrefsWriter. + private fun persist() { + writer.write { putString(KEY, json.encodeToString(_subs.value)) } } private fun load(): List = runCatching { 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 1dc9b6fd6..7ffef0a5c 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 @@ -140,12 +140,12 @@ class SubscriptionFeedViewModel : ViewModel() { private var inFlight: Job? = null /** - * The background enrichment job runs on StrawApp.globalScope so it - * outlives the VM's viewModelScope — but a refresh-cancel must - * still kill the *previous* enrichment so we don't pile up - * overlapping fan-outs (8-wide × N overlapping refreshes blows the - * concurrency budget). Tracked here, cancelled in the same places - * `inFlight` is./3/8. + * The background enrichment job (enrichVisibleItems) runs on + * viewModelScope, so it dies with the VM — a destroyed _ui must not get + * a stale emit and a torn-down channelCache must not be re-merged. A + * refresh-cancel still kills the *previous* enrichment so overlapping + * fan-outs don't pile up (8-wide × N overlapping refreshes would blow the + * concurrency budget). Tracked here, cancelled wherever `inFlight` is. */ private var enrichJob: Job? = null @@ -468,12 +468,10 @@ class SubscriptionFeedViewModel : ViewModel() { */ fun clearInMemoryCache() { // Cancel any in-flight refresh — without this, fetchChannelInto - // coroutines mid-execution would re-populate the cache after - // the clear. Also cancel any - // enrichment fan-out (lives on globalScope, NOT viewModelScope) - // — otherwise a still-running enrichment would write to - // FeedEnrichment + then push a merged emit reading the empty - // channelCache. + // coroutines mid-execution would re-populate the cache after the + // clear. Also cancel any enrichment fan-out (on viewModelScope) — + // otherwise a still-running enrichment would write to FeedEnrichment + // and then push a merged emit reading the empty channelCache. inFlight?.cancel() enrichJob?.cancel() channelCache.clear() @@ -512,8 +510,15 @@ private fun StreamItem.recencyScore(): Long { "year" -> 31_536_000 else -> return Long.MIN_VALUE } - // Sign flip: smaller "seconds ago" → larger score (more recent). - // Cap at a sane horizon so a "1 second ago" doesn't overwhelm the - // viewCount tiebreaker on items that are functionally tied. - return -(n * unitSecs) + // Sign flip: smaller "seconds ago" → larger (less-negative) score, so + // newer sorts first. A crafted/garbage relative-date ("99999999 years + // ago") could overflow the multiply — treat overflow as unparseable and + // sink it to the bottom. (The RSS path can't reach this; Rust's + // iso_to_relative clamps. A non-RSS-sourced StreamItem theoretically could.) + val secondsAgo = try { + Math.multiplyExact(n, unitSecs) + } catch (e: ArithmeticException) { + return Long.MIN_VALUE + } + return -secondsAgo } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/settings/SettingsScreen.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/settings/SettingsScreen.kt index 2a8af7a18..c1394f129 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/settings/SettingsScreen.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/settings/SettingsScreen.kt @@ -31,6 +31,7 @@ import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -601,23 +602,29 @@ fun SettingsScreen() { color = MaterialTheme.colorScheme.onSurfaceVariant, ) Spacer(modifier = Modifier.height(8.dp)) - // Sample on-disk usage once per Settings entry — File.length() is - // cheap but we don't need it to recompose on every state change. - // remember keeps the same snapshot for the entire session. - val usage = remember { - object { - val history = com.sulkta.straw.util.StorageUsage.sharedPrefBytes(context, "straw_history") - val resume = com.sulkta.straw.util.StorageUsage.sharedPrefBytes(context, "straw_resume_positions") - val search = com.sulkta.straw.util.StorageUsage.sharedPrefBytes(context, "straw_search_cache") - val feed = com.sulkta.straw.util.StorageUsage.sharedPrefBytes(context, "straw_feed_cache") - val coil = com.sulkta.straw.util.StorageUsage.coilDiskCacheBytes(context) + // Sample on-disk usage once per Settings entry, OFF the main thread + // (audit #2 L-14): File.length() ×4 + Coil diskCache.size is real + // synchronous FS I/O, and doing it in the composition stalled the + // first Settings frame. LaunchedEffect → Dispatchers.IO populates the + // snapshot; until it lands `usage` is null and the "Used:" labels + // simply don't render (matching CacheCapRow's hide-on-zero). + var usage by remember { mutableStateOf(null) } + LaunchedEffect(Unit) { + usage = withContext(Dispatchers.IO) { + CacheUsage( + history = com.sulkta.straw.util.StorageUsage.sharedPrefBytes(context, "straw_history"), + resume = com.sulkta.straw.util.StorageUsage.sharedPrefBytes(context, "straw_resume_positions"), + search = com.sulkta.straw.util.StorageUsage.sharedPrefBytes(context, "straw_search_cache"), + feed = com.sulkta.straw.util.StorageUsage.sharedPrefBytes(context, "straw_feed_cache"), + coil = com.sulkta.straw.util.StorageUsage.coilDiskCacheBytes(context), + ) } } CacheCapRow( label = "Watch + search history", selected = store.historyWatchesCap.collectAsState().value, onPick = { store.setHistoryWatchesCap(it) }, - usageBytes = usage.history, + usageBytes = usage?.history ?: 0L, ) CacheCapRow( label = "Search history", @@ -628,13 +635,13 @@ fun SettingsScreen() { label = "Resume positions", selected = store.resumePositionsCap.collectAsState().value, onPick = { store.setResumePositionsCap(it) }, - usageBytes = usage.resume, + usageBytes = usage?.resume ?: 0L, ) CacheCapRow( label = "Search results cache", selected = store.searchCacheCap.collectAsState().value, onPick = { store.setSearchCacheCap(it) }, - usageBytes = usage.search, + usageBytes = usage?.search ?: 0L, ) Row( modifier = Modifier.fillMaxWidth().padding(vertical = 6.dp), @@ -646,11 +653,13 @@ fun SettingsScreen() { fontWeight = FontWeight.SemiBold, modifier = Modifier.weight(1f), ) - Text( - text = "Used: ${com.sulkta.straw.util.StorageUsage.format(usage.feed)}", - style = MaterialTheme.typography.labelSmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) + usage?.let { + Text( + text = "Used: ${com.sulkta.straw.util.StorageUsage.format(it.feed)}", + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } } Row( modifier = Modifier.fillMaxWidth().padding(vertical = 6.dp), @@ -662,11 +671,13 @@ fun SettingsScreen() { fontWeight = FontWeight.SemiBold, modifier = Modifier.weight(1f), ) - Text( - text = "Used: ${com.sulkta.straw.util.StorageUsage.format(usage.coil)}", - style = MaterialTheme.typography.labelSmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) + usage?.let { + Text( + text = "Used: ${com.sulkta.straw.util.StorageUsage.format(it.coil)}", + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } } Spacer(modifier = Modifier.height(8.dp)) Text( @@ -844,6 +855,15 @@ private fun CategoryRow( } } +/** On-disk usage snapshot, sampled off the main thread (audit #2 L-14). */ +private data class CacheUsage( + val history: Long, + val resume: Long, + val search: Long, + val feed: Long, + val coil: Long, +) + /** * Compact chip-group row for picking a CacheCap. Label on the left, * 5 chips on the right, optional "Used: X KB" suffix to the right diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/net/Http.kt b/strawApp/src/main/kotlin/com/sulkta/straw/net/Http.kt index be3ee4dda..bfb489b11 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/net/Http.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/net/Http.kt @@ -2,73 +2,14 @@ * SPDX-FileCopyrightText: 2026 Sulkta-Coop * SPDX-License-Identifier: GPL-3.0-or-later * - * Bounded body reader. AUD-HIGH flagged that body.string() reads the whole - * response into memory with no cap — a hostile or compromised endpoint - * (RYD, SponsorBlock, even a poisoned YT response) can OOM the app. - * - * Partial defense: if Content-Length is advertised and over the cap, we - * refuse the read. If Content-Length is absent (chunked transfer), we - * stream up to the cap and refuse if the body keeps coming. + * Shared HTTP bits for the Android layer. The RYD + SponsorBlock JSON + * clients moved to Rust (strawcore `net.rs`) in the vc=85 backend + * migration, which took the OkHttp client + bounded-body reader with it — + * so all that survives here is the User-Agent string, still used by the + * ExoPlayer DataSource factory (PlaybackService). */ package com.sulkta.straw.net -import okhttp3.OkHttpClient -import okhttp3.ResponseBody -import okio.Buffer -import java.io.IOException -import java.util.concurrent.TimeUnit - -/** - * Path C-6 / Phase U-5: USER_AGENT + shared OkHttpClient that previously - * lived on NewPipeDownloader. After ripping NewPipeExtractor, the RYD + - * SponsorBlock + ExoPlayer HTTP factories still need both. One shared - * client is fine. - */ const val STRAW_USER_AGENT: String = "Mozilla/5.0 (Linux; Android 14) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Mobile Safari/537.36 Straw/1.0" - -// OkHttpClient is internally thread-safe; lazy(SYNCHRONIZED) builds -// exactly once across threads. — the prior -// synchronized(STRAW_USER_AGENT) locked an interned String literal -// shared with any other code in any library that happened to lock -// the same literal. Lazy-delegate avoids the global pool lock. -private val sharedClient: OkHttpClient by lazy(LazyThreadSafetyMode.SYNCHRONIZED) { - OkHttpClient.Builder() - .connectTimeout(15, TimeUnit.SECONDS) - .readTimeout(30, TimeUnit.SECONDS) - .followRedirects(true) - .followSslRedirects(true) - .build() -} - -fun strawHttpClient(): OkHttpClient = sharedClient - -fun ResponseBody.cappedString(maxBytes: Long): String { - val cl = contentLength() - if (cl in 1..maxBytes) { - return string() - } - if (cl > maxBytes) { - close() - throw IOException("response too large: $cl bytes > $maxBytes cap") - } - // Chunked transfer or no content-length advertised — stream up to cap. - val src = source() - val buf = Buffer() - var read = 0L - while (read < maxBytes) { - val chunk = src.read(buf, maxBytes - read) - if (chunk == -1L) break - read += chunk - } - if (!src.exhausted()) { - close() - throw IOException("response exceeded cap of $maxBytes bytes") - } - return buf.readUtf8() -} - -const val NEWPIPE_MAX_BYTES = 8L * 1024 * 1024 -const val RYD_MAX_BYTES = 256L * 1024 -const val SB_MAX_BYTES = 1L * 1024 * 1024 diff --git a/strawApp/src/main/res/xml/network_security_config.xml b/strawApp/src/main/res/xml/network_security_config.xml index 5e3b6a15d..cae2cd200 100644 --- a/strawApp/src/main/res/xml/network_security_config.xml +++ b/strawApp/src/main/res/xml/network_security_config.xml @@ -3,11 +3,18 @@ SPDX-FileCopyrightText: 2026 Sulkta-Coop SPDX-License-Identifier: GPL-3.0-or-later - Returnyoutubedislike.com serves a leaf cert without including the - Sectigo "Public Server Authentication CA DV R36" intermediate. - Android's system trust store has the USERTrust root but not the - intermediate, so chain reconstruction fails. We bundle the intermediate - as an additional trust anchor scoped to that domain. + base-config: app-wide trust = system roots, cleartext OFF. This is live — + it governs every Android-layer request (ExoPlayer DataSource, Coil image + loads, the auto-updater). + + domain-config (RYD): historically RYD's host served a leaf without the + Sectigo "Public Server Authentication CA DV R36" intermediate, so we + bundled that intermediate as an extra trust anchor for the OkHttp RYD + client. As of vc=85 the RYD + SponsorBlock clients moved to Rust + (strawcore net.rs), which uses reqwest's OWN trust store (rustls + + webpki-roots) and does NOT consult this file — so this block + the bundled + @raw/sectigo_dv_r36 cert no longer affect RYD traffic. Kept as an inert, + harmless guard in case any Android-layer code ever hits those hosts again. --> @@ -15,8 +22,9 @@ - + returnyoutubedislike.com returnyoutubedislikeapi.com