diff --git a/buildSrc/src/main/kotlin/ProjectConfig.kt b/buildSrc/src/main/kotlin/ProjectConfig.kt index 1255794b2..b90dbd177 100644 --- a/buildSrc/src/main/kotlin/ProjectConfig.kt +++ b/buildSrc/src/main/kotlin/ProjectConfig.kt @@ -9,6 +9,28 @@ const val STRAW_SDK_TARGET = 35 // Sulkta fork — Straw // +// 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 +// — a video that appears twice (a collab/cross-post in two subscribed +// feeds, or the same videoId across two search/channel shelves) produced a +// duplicate Compose key → IllegalArgumentException → screen crash. Now the +// subs merge + the Search/Channel FIRST page dedup by url (the continuation +// paths already did). The vc=86 key additions only deduped the append path. +// * Complete the IosSafe end-of-chunk roll: the vc=86 change set +// chunkRemaining=0 on inner EOF expecting the NEXT read() to roll, but +// Media3 stops calling read() after a -1, so a LENGTH_UNSET inner still +// truncated to the first chunk. Now the roll happens WITHIN read() (loop), +// with a progress guard so a no-progress chunk can't spin. +// * SearchCacheStore.clear() is now atomic (updateAndGet), so a concurrent +// record() can't resurrect a just-cleared entry on disk. +// * Gate the YtRelated autoplay branch through isAllowedYtUrl to match the +// stated universal-allowlist invariant; no-op-guard setLatestKnownVersion; +// drop two unused TrackSelectionParameters imports. +// (Deferred to a follow-up: SharedPreferences write-ordering serialization +// across the stores, the migration's dead Http.kt sweep, and a stale-comment +// pass — all low-risk hygiene, none a crash.) +// // vc=86 / 0.1.0-CT — audit-fix sprint (code-audit HIGH H2 + 5 MED/LOW): // * Audio-only toggle no longer drops your max-resolution cap. Both the // fullscreen button and the detail "Audio" pill rebuilt the track- @@ -225,6 +247,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 = 86 -const val STRAW_VERSION_NAME = "0.1.0-CT" +const val STRAW_VERSION_CODE = 87 +const val STRAW_VERSION_NAME = "0.1.0-CU" const val STRAW_APPLICATION_ID = "com.sulkta.straw" diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/SearchCacheStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/SearchCacheStore.kt index 4b9473fe2..79fd89c7c 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/SearchCacheStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/SearchCacheStore.kt @@ -90,7 +90,11 @@ class SearchCacheStore(context: Context) { } fun clear() { - _entries.value = emptyList() + // updateAndGet (not a plain `.value =`) so a concurrent record() + // interleaving its read→build→persist can't leave disk = [entry] after + // the user asked to clear — the same B5 race the StateFlow migration was + // meant to close (audit #2 M-3). + _entries.updateAndGet { emptyList() } sp.edit().remove(KEY).apply() } 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 e98e23607..279440f19 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt @@ -398,6 +398,10 @@ class SettingsStore(context: Context) { } fun setLatestKnownVersion(vc: Long, vname: String) { + // No-op guard to match every other setter in this file: runUpdateCheck + // calls this on every up-to-date poll, so without it we rewrite the + // same (0,"") to disk + re-emit the StateFlow each time (audit #2 L-13). + if (_latestKnownVc.value == vc && _latestKnownVname.value == vname) return _latestKnownVc.value = vc _latestKnownVname.value = vname sp.edit() diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelViewModel.kt index 1e6c86db7..be122fe68 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelViewModel.kt @@ -108,7 +108,11 @@ class ChannelViewModel : ViewModel() { viewCount = v.viewCount, uploadDateRelative = v.uploadDateRelative, ) - } + }.distinctBy { it.url } + // Dedup the FIRST page by url — the videos LazyColumn keys by + // it.url (vc=86); a duplicate key hard-crashes Compose, which a + // cross-shelf duplicate videoId can produce. loadMore already + // dedups; this closes the initial-page gap (audit #2 M-4). if (_ui.value.loadedUrl != channelUrl) return@launch _ui.update { ChannelUiState( diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailBody.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailBody.kt index a08fda2f5..b98a067b9 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailBody.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailBody.kt @@ -92,7 +92,6 @@ import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.viewmodel.compose.viewModel import androidx.media3.common.C -import androidx.media3.common.TrackSelectionParameters import androidx.media3.common.util.UnstableApi import coil3.compose.AsyncImage import com.sulkta.straw.data.ChannelRef 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 913fdd9b4..1dc9b6fd6 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 @@ -348,6 +348,13 @@ class SubscriptionFeedViewModel : ViewModel() { // memo is exact. val recencyMemo = HashMap() return channels.flatMap { ch -> channelCache[ch.url]?.items.orEmpty() } + // Dedup by url BEFORE the url-keyed LazyColumn renders it: a video + // can legitimately appear in two subscribed channels' feeds (a + // collab, or a channel + its auto-generated " - Topic" + // mirror), and StrawHome keys the feed list by it.url — a duplicate + // key hard-crashes the Subs tab (audit #2 M-1). distinctBy keeps + // the first occurrence (channel order), which is fine for the sort. + .distinctBy { it.url } .map { it.withEnrichment(enrichments) } .map { item -> item to recencyMemo.getOrPut(item.uploadDateRelative) { item.recencyScore() } } .sortedWith( diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt index 1d9cbe545..71a0c9649 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt @@ -352,12 +352,21 @@ class PlaybackService : MediaSessionService() { .firstOrNull()?.url } AutoplayMode.YtRelated -> { - val info = uniffi.strawcore.streamInfo(currentStreamUrl) - info.related - .asSequence() - .filter { it.url != currentStreamUrl } - .filter { unwatched(it.url) } - .firstOrNull()?.url + // Gate currentStreamUrl through the same allowlist the + // SameChannel/candidate paths use before it reaches the + // extractor — it can come from a MediaItem.localConfiguration + // .uri fallback, not always a freshly-revalidated YT url + // (audit #2 L-11, matches the universal-gate comment above). + if (!isAllowedYtUrl(currentStreamUrl)) { + null + } else { + val info = uniffi.strawcore.streamInfo(currentStreamUrl) + info.related + .asSequence() + .filter { it.url != currentStreamUrl } + .filter { unwatched(it.url) } + .firstOrNull()?.url + } } } } catch (c: kotlinx.coroutines.CancellationException) { diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt index 122816ecb..d47feb906 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt @@ -69,7 +69,6 @@ import androidx.lifecycle.viewmodel.compose.viewModel import androidx.media3.common.C import androidx.media3.common.PlaybackParameters import androidx.media3.common.Player -import androidx.media3.common.TrackSelectionParameters import androidx.media3.common.util.UnstableApi import androidx.media3.ui.PlayerView import com.sulkta.straw.OverlayChromeColor diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt index 385fcf904..32ea18b26 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt @@ -246,7 +246,11 @@ class SearchViewModel : ViewModel() { viewCount = r.viewCount, uploadDateRelative = r.uploadDateRelative, ) - } + }.distinctBy { it.url } + // Dedup the FIRST page by url — the results LazyColumn keys by + // it.url (vc=86) and Compose hard-crashes on a duplicate key, + // which a cross-shelf duplicate videoId can produce. loadMore + // already dedups; this closes the initial-page gap (audit #2 M-4). // Fence by job identity (ensureActive) — only a fresh // submit that called inFlight?.cancel() invalidates // me. Bare typing in the search bar (onQueryChange) diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/net/IosSafeHttpDataSource.kt b/strawApp/src/main/kotlin/com/sulkta/straw/net/IosSafeHttpDataSource.kt index 1e18b6e1a..9684f0c03 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/net/IosSafeHttpDataSource.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/net/IosSafeHttpDataSource.kt @@ -45,6 +45,13 @@ class IosSafeHttpDataSource( /** Bytes left in the current inner-open chunk. -1 = unknown end. */ private var chunkRemaining: Long = 0 + /** `totalRead` at the moment the current chunk was opened. The roll loop + * in read() uses it as a progress guard: a chunk that delivers zero new + * bytes and then EOFs is the genuine end of stream (or a stuck server), + * not a reason to re-open the same position — without this the roll could + * spin forever. */ + private var chunkStartTotalRead: Long = 0 + override fun open(dataSpec: DataSpec): Long { // When length is set, respect it but still cap the first inner-open to // chunkBytes. When length is unset, request a chunk and we'll roll @@ -84,6 +91,7 @@ class IosSafeHttpDataSource( strawLogW(TAG, t) { "open failed: ${t.javaClass.simpleName}" } throw t } + chunkStartTotalRead = totalRead strawLogD(TAG) { "open: inner chunkRemaining=$chunkRemaining" } // Report the original (potentially unbounded) length to the caller — // ExoPlayer cares about the overall length, not our internal chunking. @@ -96,50 +104,64 @@ class IosSafeHttpDataSource( override fun read(buffer: ByteArray, offset: Int, length: Int): Int { if (length == 0) return 0 - // Need a fresh chunk? - if (chunkRemaining == 0L) { - val spec = originalSpec ?: return C.RESULT_END_OF_INPUT - inner.close() - val nextPos = spec.position + totalRead - val remainingOverall = if (spec.length == C.LENGTH_UNSET.toLong()) { - Long.MAX_VALUE - } else { - spec.length - totalRead + // Loop so a chunk-boundary roll happens WITHIN this read() call. The + // earlier version set chunkRemaining=0 on inner EOF and `return read` + // (== -1), expecting the NEXT read() to roll — but Media3 treats -1 + // from read() as terminal end-of-stream and never calls read() again, + // so the deferred roll was dead code and a LENGTH_UNSET inner truncated + // to the first chunk (audit #2 M-5). + while (true) { + // Need a fresh chunk? + if (chunkRemaining == 0L) { + val spec = originalSpec ?: return C.RESULT_END_OF_INPUT + inner.close() + val nextPos = spec.position + totalRead + val remainingOverall = if (spec.length == C.LENGTH_UNSET.toLong()) { + Long.MAX_VALUE + } else { + spec.length - totalRead + } + if (remainingOverall <= 0L) return C.RESULT_END_OF_INPUT + val nextLen = remainingOverall.coerceAtMost(chunkBytes) + // Same as in open() — use buildUpon().setPosition/setLength + // rather than subrange() so the absolute position stays valid. + val nextSpec = spec.buildUpon() + .setPosition(nextPos) + .setLength(nextLen) + .build() + chunkRemaining = inner.open(nextSpec) + chunkStartTotalRead = totalRead + // A freshly-opened chunk that immediately reports 0 bytes (a + // 0-length 206, or a past-EOF range answered with an empty body) + // is the true end — don't spin re-opening the same position + // (audit #2 L-10). + if (chunkRemaining == 0L) return C.RESULT_END_OF_INPUT } - if (remainingOverall <= 0L) return C.RESULT_END_OF_INPUT - val nextLen = remainingOverall.coerceAtMost(chunkBytes) - // Same as in open() — use buildUpon().setPosition/setLength rather - // than subrange() so the absolute position stays meaningful. - val nextSpec = spec.buildUpon() - .setPosition(nextPos) - .setLength(nextLen) - .build() - chunkRemaining = inner.open(nextSpec) - } - // Cap the read against what's left in this chunk. - val toRead = if (chunkRemaining < 0L) { - // Inner doesn't know its end either; just read what was asked. - length - } else { - length.toLong().coerceAtMost(chunkRemaining).toInt() - } - val read = inner.read(buffer, offset, toRead) - if (read != C.RESULT_END_OF_INPUT) { - totalRead += read.toLong() - if (chunkRemaining > 0L) chunkRemaining -= read.toLong() - // If chunkRemaining hits 0 here, the next read() call will roll - // to the next chunk via the block at the top. - } else if (chunkRemaining != 0L) { - // Inner hit EOF. Force a chunk roll on the next read() so we - // re-open at the next position. `!= 0L` (not `> 0L`) so the - // unknown-end case (LENGTH_UNSET inner → chunkRemaining < 0) - // ALSO rolls: with `> 0L` a negative chunkRemaining never reset - // to 0, so the top-of-read() roll block (gated on `== 0L`) never - // fired and we re-read the exhausted inner forever — playback - // truncated to the first chunk. + // Cap the read against what's left in this chunk. + val toRead = if (chunkRemaining < 0L) { + // Inner doesn't know its end either; just read what was asked. + length + } else { + length.toLong().coerceAtMost(chunkRemaining).toInt() + } + val read = inner.read(buffer, offset, toRead) + if (read != C.RESULT_END_OF_INPUT) { + totalRead += read.toLong() + if (chunkRemaining > 0L) chunkRemaining -= read.toLong() + return read + } + // Inner hit EOF. If this chunk delivered no new bytes, it's the + // genuine end of the stream (or a stuck server) — stop rather than + // re-open the same position forever (the roll-loop progress guard). + if (totalRead == chunkStartTotalRead) { + return C.RESULT_END_OF_INPUT + } + // The chunk delivered some bytes then EOF'd at its boundary — roll + // to the next chunk and retry within this read() (loop back to the + // top, which re-opens at nextPos; the remainingOverall<=0 / + // 0-length-chunk guards terminate it). chunkRemaining = 0L } - return read } override fun close() { @@ -149,6 +171,7 @@ class IosSafeHttpDataSource( originalSpec = null totalRead = 0 chunkRemaining = 0 + chunkStartTotalRead = 0 } }