vc=39: loop round 1/5 — 9 HIGH + 7 MED from 3 Opus round-4 audits
Three parallel Opus max-effort audits ran on vc=38. No new CRITs (the
LogDump + VM-error-scrub chain held), but real new HIGHs across VMs
that weren't touched in rounds 1-3 + the Rust runtime's brittle
one-shot init.
HIGH
R4-1 Rust runtime::ensure_initialized was one-shot via Once.
First-call failure (cold-boot in airplane mode, transient
DNS/SELinux denial on first TLS init) consumed the Once slot
and bricked the extractor for the rest of the process —
every subsequent search/streamInfo/channelInfo returned
DownloaderMissing forever. Replaced with AtomicBool + 5s
backoff retry; success closes the door, failure retries on
the next call.
R4-2 VideoDetailViewModel.load tracked no inFlight Job.
Activity-scoped VM is reused; tap video A → quickly tap a
related-video B → both loads race, slower-finisher wins.
A's resolved payload (different itags, different SB
segments, wrong title chip) could render on the B detail
page; recordWatch logged B while the player streamed A.
Now: inFlight?.cancel() at top, fenced terminal writes with
loadedUrl-stable guard. Same shape applied to
ChannelViewModel (had no in-flight tracking at all).
R4-3 `_ui.value = _ui.value.copy(...)` lost-write patterns
survived round-3's pass in SearchViewModel + VideoDetail +
Channel. Migrated all to `_ui.update {}` — same atomicity
regression class round 3 was supposed to close. Submit/load
terminal writes also now fence against late-arrivals.
R4-4 HistoryStore.recordAllWatches reported `size_after -
size_before` to SettingsImport — at a saturated store the
post-state size equals the pre-state size even when 20
fresh imports landed and 20 older entries got truncated.
User saw "0 watch history imported" when 30 actually
landed. Now: recordAllWatches/recordAllSearches return an
AtomicInteger-counted actual-fresh-added count from inside
the CAS lambda; SettingsImport plumbs through to the report.
R4-5 SubscriptionFeedViewModel.refresh() filtered to stale-only
— user-initiated tap of Refresh was a silent no-op when
every channel had been refreshed in the last 28min.
Split: refresh() forces fan-out across every sub;
refreshIfStale() keeps the TTL filter. Both share
refreshInternal(force: Bool).
R4-6 SettingsImport.importPlaylists called create() + addItem()
in a loop — both write SP, and addItem walks every playlist
linearly per insert. A NewPipe export with 100 playlists ×
100 items = ~10k SP commits + O(N²) work. New
PlaylistsStore.importPlaylist mints a single Playlist with
pre-attached items, one CAS, one SP write per playlist.
R4-7 VideoDetailViewModel auto-called channelInfo(uploaderUrl)
on every load — no allowlist gate. An extractor-emitted
non-YT uploaderUrl (poisoned related/moreFromChannel)
would have triggered an arbitrary-host network call.
R4-8 Similar shape: VideoDetailViewModel.recordWatch persisted
whatever URL was passed to load() — extractor-emitted non-YT
URLs would have survived in Recent Watches past process
death. Same import-time URL allowlist now gates both.
CVE-1 The reCAPTCHA error path embedded the full google.com/sorry/
URL into the user-visible banner. That URL carries
`continue=<full-signed-googlevideo-url>` — and LogDump's
scrub only matches googlevideo.com hosts. Now: strip the
`continue=` param in Rust before propagating; UI shows a
tappable challenge URL that still solves the rate-limit
when the user opens it.
MED
R4-9 SettingsStore.setMaxResolution/setThemeMode/setCacheEnabled
were not atomic vs toggle()'s updateAndGet pattern. Now
CAS-safe + idempotent (no SP write when the value is
already what's stored).
R4-10 SponsorBlockClient.fetch built the URL via string concat
with un-percent-encoded JSON-shaped categories list.
Switched to HttpUrl.Builder().addQueryParameter() — okhttp
does the right escaping. SB happens to accept the raw form
today; this guards future user-typed categories.
R4-11 strawHttpClient() synchronized on the interned
STRAW_USER_AGENT string literal — any unrelated code that
happened to lock the same literal could contend. Replaced
with lazy(SYNCHRONIZED) — same one-shot init, no shared
global lock.
R4-12 DownloadsScreen.queryDownloads ran on the main coroutine
every 1-5s. DownloadManager.query is a ContentResolver IPC
+ SQLite cursor walk; on devices with hundreds of historical
downloads it stuttered. withContext(Dispatchers.IO).
R4-13 Co-located the YT host allowlist (was inline in
SettingsImport) into util/YtUrl.kt — VideoDetailViewModel
now imports the same function. Future host changes are
one edit.
Deferred to round 2-5:
R4-MED — Nav.kt has no rememberSaveable / Parcelize on Screen
sealed types. Process-death loses entire back stack.
Needs Parcelize plugin add + listSaver — bigger refactor.
R4-HIGH — Release isMinifyEnabled = false / no R8. Needs
comprehensive keep-rules for UniFFI + kotlinx-serialization
before flipping safely. Holding for a dedicated round.
R4-MED — LazyColumn key= missing in 5 list sites; quick win
but cosmetic, won't slip into post-round-5 ship.
R4-MED — collectAsStateWithLifecycle bulk-replace.
R4-MED — SponsorBlock skip-loop should bind segments to
controller.currentMediaItem to avoid one-tick misapply on
track changes.
This commit is contained in:
parent
cbdba302ce
commit
b8325d1726
15 changed files with 443 additions and 167 deletions
|
|
@ -31,13 +31,47 @@ pub enum StrawcoreError {
|
|||
RequiresLogin { detail: String },
|
||||
}
|
||||
|
||||
/// Drop the `continue=<signed-url>` param from a google.com/sorry/...
|
||||
/// URL while leaving every other param intact. Used only for surfacing
|
||||
/// recaptcha challenge URLs to the UI; keeps the URL tappable for the
|
||||
/// user to solve the challenge while scrubbing the embedded
|
||||
/// googlevideo signature.
|
||||
fn strip_continue_param(url: &str) -> String {
|
||||
let (base, query) = match url.split_once('?') {
|
||||
Some(pair) => pair,
|
||||
None => return url.to_owned(),
|
||||
};
|
||||
let filtered: Vec<&str> = query
|
||||
.split('&')
|
||||
.filter(|kv| {
|
||||
let key = kv.split_once('=').map(|(k, _)| k).unwrap_or(*kv);
|
||||
!key.eq_ignore_ascii_case("continue")
|
||||
})
|
||||
.collect();
|
||||
if filtered.is_empty() {
|
||||
base.to_owned()
|
||||
} else {
|
||||
format!("{}?{}", base, filtered.join("&"))
|
||||
}
|
||||
}
|
||||
|
||||
impl From<strawcore_core::exceptions::ExtractionError> for StrawcoreError {
|
||||
fn from(e: strawcore_core::exceptions::ExtractionError) -> Self {
|
||||
use strawcore_core::exceptions::{ContentUnavailable, ExtractionError, NetworkError};
|
||||
match e {
|
||||
ExtractionError::Network(NetworkError::Recaptcha { url }) => {
|
||||
// Strip the `continue=` query param before propagating.
|
||||
// google.com/sorry/index carries the full signed
|
||||
// googlevideo URL in `continue=` so the user can be
|
||||
// sent back to the stream after solving — but
|
||||
// surfacing that to the UI is a credential leak via
|
||||
// screenshot, and Kotlin's LogDump scrubber only
|
||||
// catches googlevideo.com hosts. The challenge URL
|
||||
// itself still solves without `continue=`, so the
|
||||
// user can tap to unblock without leaking the
|
||||
// signature/expire/pot token. Round-4 audit LOW-1.
|
||||
StrawcoreError::RequiresLogin {
|
||||
detail: format!("reCAPTCHA at {url}"),
|
||||
detail: format!("reCAPTCHA challenge: {}", strip_continue_param(&url)),
|
||||
}
|
||||
}
|
||||
ExtractionError::Network(NetworkError::Transport(msg)) => {
|
||||
|
|
|
|||
|
|
@ -1,29 +1,83 @@
|
|||
// Runtime bootstrap. Called once from Kotlin's StrawApp.onCreate via
|
||||
// init_logging(). Wires the strawcore-core Downloader + Localization
|
||||
// singleton so the extractor calls have an HTTP client to use.
|
||||
// init_logging(), and again before every strawcore call. Wires the
|
||||
// strawcore-core Downloader + Localization singleton so the extractor
|
||||
// has an HTTP client to use.
|
||||
//
|
||||
// Round-4 audit HIGH-1: the prior shape used `Once::call_once` and
|
||||
// silently swallowed errors. If the FIRST call ran while the network
|
||||
// stack wasn't ready (cold boot in airplane mode, SELinux denial on
|
||||
// first TLS init, transient resolver failure), the Once slot was
|
||||
// consumed, NewPipe::init_full never ran, and every subsequent
|
||||
// search/streamInfo/channelInfo returned DownloaderMissing for the
|
||||
// rest of the process lifetime.
|
||||
//
|
||||
// New shape: use an AtomicBool to track success. Only "success" closes
|
||||
// the door. On failure we retry — rate-limited so a persistently-broken
|
||||
// network doesn't hammer reqwest::Client::new() on every call.
|
||||
|
||||
use std::sync::{Arc, Once};
|
||||
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::time::{SystemTime, UNIX_EPOCH};
|
||||
|
||||
use strawcore_core::downloader::ReqwestDownloader;
|
||||
use strawcore_core::localization::{ContentCountry, Localization};
|
||||
use strawcore_core::newpipe::NewPipe;
|
||||
|
||||
static INIT: Once = Once::new();
|
||||
static INITIALIZED: AtomicBool = AtomicBool::new(false);
|
||||
static LAST_ATTEMPT_MS: AtomicU64 = AtomicU64::new(0);
|
||||
// Guards the actual init attempt so concurrent calls don't all try
|
||||
// to build the downloader in parallel; serial retry is the goal.
|
||||
static INIT_LOCK: Mutex<()> = Mutex::new(());
|
||||
|
||||
/// Min ms between retries when init has failed. 5s — enough that a
|
||||
/// hot loop of failed searches doesn't pin a CPU on reqwest setup,
|
||||
/// short enough that a user who toggled airplane mode off recovers
|
||||
/// within one tap.
|
||||
const RETRY_BACKOFF_MS: u64 = 5_000;
|
||||
|
||||
fn now_ms() -> u64 {
|
||||
SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0)
|
||||
}
|
||||
|
||||
pub fn ensure_initialized() {
|
||||
INIT.call_once(|| {
|
||||
match ReqwestDownloader::new() {
|
||||
Ok(dl) => {
|
||||
NewPipe::init_full(
|
||||
Arc::new(dl),
|
||||
Localization::default(),
|
||||
ContentCountry::default(),
|
||||
);
|
||||
log::info!("strawcore-core: downloader + localization initialized");
|
||||
}
|
||||
Err(e) => {
|
||||
log::error!("strawcore-core: failed to build downloader: {e}");
|
||||
}
|
||||
if INITIALIZED.load(Ordering::Acquire) {
|
||||
return;
|
||||
}
|
||||
// Backoff check OUTSIDE the lock — avoids serializing every
|
||||
// already-throttled caller on a single mutex.
|
||||
let last = LAST_ATTEMPT_MS.load(Ordering::Acquire);
|
||||
let now = now_ms();
|
||||
if last != 0 && now.saturating_sub(last) < RETRY_BACKOFF_MS {
|
||||
return;
|
||||
}
|
||||
let _guard = match INIT_LOCK.lock() {
|
||||
Ok(g) => g,
|
||||
Err(p) => p.into_inner(),
|
||||
};
|
||||
// Re-check under the lock — another thread may have just
|
||||
// succeeded while we were waiting.
|
||||
if INITIALIZED.load(Ordering::Acquire) {
|
||||
return;
|
||||
}
|
||||
LAST_ATTEMPT_MS.store(now_ms(), Ordering::Release);
|
||||
match ReqwestDownloader::new() {
|
||||
Ok(dl) => {
|
||||
NewPipe::init_full(
|
||||
Arc::new(dl),
|
||||
Localization::default(),
|
||||
ContentCountry::default(),
|
||||
);
|
||||
INITIALIZED.store(true, Ordering::Release);
|
||||
log::info!("strawcore-core: downloader + localization initialized");
|
||||
}
|
||||
});
|
||||
Err(e) => {
|
||||
// Don't surface the underlying error string verbatim —
|
||||
// it can embed URLs / hosts.
|
||||
log::error!("strawcore-core: downloader init failed (will retry on next call)");
|
||||
let _ = e;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue