From c428e18e823da8a3db9db9eb5a695daad5eb451b Mon Sep 17 00:00:00 2001 From: KrX3D Date: Tue, 1 Sep 2026 20:31:40 +0200 Subject: [PATCH 1/2] fix: stop the playlist cache serving stale watched state on re-entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported: after watching videos and re-entering a playlist, the watched ones came back looking unwatched. That is a regression from the full-list cache, and the diagnosis offered with the report was right — re-entry was being served from cache. getWatchPercent() reads watched state primarily from data embedded in the renderer itself (thumbnailOverlayResumePlaybackRenderer, played/status overlays, watched badges), falling back to the live _ttVideoProgressCache only when none of those are present. The cache holds raw renderers captured at collection time, so their overlays are frozen at that moment. Injecting them on a later visit replays whatever progress they had then, and overwrites the fresh response that did carry the updated state. So the cache is now strictly a hand-off from the pre-reload pass to the reloaded page, consumed on injection. Every visit runs its own collect+reload against fresh data. That costs a cycle per visit (~1.7s) instead of a 0.38s cache hit, which is the right trade for showing correct watched state. Two guards keep that from looping, since consuming the cache mid-visit would otherwise let the reloaded page collect and reload again: - __ttServedFromCache marks the page that was served from cache, and the scheduler stands down for it — it already holds the full list and has no continuation token. - The reload guard is cleared in _clearState (real navigation) rather than at injection, so it persists across SOFT_RELOAD_PAGE, which does not navigate, while still letting the next genuine visit run its own cycle. Not addressed, as agreed: stopping playback returns to the playlist without reprocessing it, so videos watched in that session stay visible until the page is re-entered. With this fix that re-entry now filters them correctly, which was the practical problem. --- mods/features/adblock.js | 8 ++++++- mods/features/playlistBatchCollect.js | 31 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/mods/features/adblock.js b/mods/features/adblock.js index 9ce30b03..b74c2462 100644 --- a/mods/features/adblock.js +++ b/mods/features/adblock.js @@ -5,7 +5,7 @@ import { timelyAction, longPressData, MenuServiceItemRenderer, ShelfRenderer, Ti import { PatchSettings } from '../ui/customYTSettings.js'; import { t } from 'i18next'; import './logServer.js'; -import { scheduleCollectAfterNativeSettles, getCachedFullPlaylist, noteInitialPlaylistContents, noteContinuationBatch } from './playlistBatchCollect.js'; +import { scheduleCollectAfterNativeSettles, getCachedFullPlaylist, consumeCachedFullPlaylist, noteInitialPlaylistContents, noteContinuationBatch } from './playlistBatchCollect.js'; import { appendFileOnlyLog, detectAndStorePage, @@ -944,6 +944,12 @@ JSON.parse = function () { }); topPlaylistRenderer.contents = cachedFull.slice(); topPlaylistRenderer.continuations = null; + // One-shot: the cache exists only to carry the full list across the + // reload it triggered. Keeping it would serve these same renderers on + // every later visit, and their embedded watch-progress overlays are + // frozen at collection time — so anything watched since would come + // back looking unwatched. + consumeCachedFullPlaylist(playlistKey); } else { // Hand the raw first batch to the collector: _collectAll only returns // the CONTINUATION batches, so the cached full list needs this to be diff --git a/mods/features/playlistBatchCollect.js b/mods/features/playlistBatchCollect.js index 099a2a4c..57e0d28e 100644 --- a/mods/features/playlistBatchCollect.js +++ b/mods/features/playlistBatchCollect.js @@ -143,6 +143,11 @@ function _clearState() { clearTimeout(window.__ttNativeSettleTimer); window.__ttNativeSettleTimer = null; window.__ttLatestContinuations = null; + // Per-visit guards: cleared on real navigation so the next visit can run + // its own collect+reload, while staying set across a SOFT_RELOAD_PAGE + // (which does not navigate) so that reload cannot repeat. + window.__ttServedFromCache = null; + window.__ttFullReloadDone = {}; } window.addEventListener('hashchange', _clearState); window.addEventListener('popstate', _clearState); @@ -385,6 +390,31 @@ export function getCachedFullPlaylist(key) { return entry && Array.isArray(entry.contents) ? entry.contents : null; } +// Drop a cached playlist once it has been injected. The cache is strictly a +// hand-off from the pre-reload pass to the reloaded page, NOT a store to reuse +// on later visits. +// +// It holds raw renderer objects captured at collection time, and +// getWatchPercent() reads watched state primarily from data embedded in those +// renderers (thumbnailOverlayResumePlaybackRenderer and friends), consulting +// the live _ttVideoProgressCache only as a fallback. So a cached item keeps +// whatever progress it had when it was captured: watch a video, come back, and +// the injected copy still presents it as unwatched, which is exactly what was +// reported. Serving a fresh response instead costs one more collect+reload +// cycle per visit, which is worth it for correct watched state. +export function consumeCachedFullPlaylist(key) { + const cache = getFullCache(); + if (!cache[key]) return; + delete cache[key]; + // Mark this page load as already complete so the scheduler stands down: + // it has the whole playlist and no continuation token, and letting it + // collect again would store a new cache entry and reload a second time. + // The reload guard is NOT cleared here — it is cleared on real navigation + // (_clearState), which is what lets the next visit run its own cycle + // without allowing a loop within this one. + window.__ttServedFromCache = key; +} + // adblock.js hands us the raw, unfiltered contents of the initial response so // the cached list can start with batch 1 — _collectAll only ever returns the // CONTINUATION batches (53 of 68 on the measured playlist), never the first. @@ -533,6 +563,7 @@ export function scheduleCollectAfterNativeSettles(continuations, reason) { const key = playlistKeyFromHash(); if (getCachedFullPlaylist(key)) return; // reloaded pass: already complete + if (window.__ttServedFromCache === key) return; // page already holds the full list // Newest token wins — each native continuation supersedes the last. if (_getToken(continuations)) window.__ttLatestContinuations = continuations; From f6564f7aeb7e62508b95eaa71b4c1e698c5a9151 Mon Sep 17 00:00:00 2001 From: KrX3D Date: Tue, 1 Sep 2026 20:35:45 +0200 Subject: [PATCH 2/2] fix: key the playlist cache with Map/Set, not plain objects (CodeQL) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Valid finding — remote property injection on the cache write. Both the full-playlist cache and the one-shot reload guard were plain objects keyed by playlistKeyFromHash(), i.e. window.location.hash. That is attacker-influenceable through a crafted URL, so cache[key] = ... with a hash of #__proto__ would write through to Object.prototype and pollute every object in the page. Converted both to the collection types that have no prototype chain to reach: - __ttPlaylistFullCache: object -> Map (get/set/has/delete, plus size and iteration for the eviction pass, which previously used Object.keys and a reduce over cache[a].ts). - __ttFullReloadDone: object -> Set, since it only ever stored true as a membership marker. Both are guarded with an instanceof check on read, so a stale plain object left on window by an earlier build is replaced rather than misused. Set is already used in four places in adblock.js, so these types are known to work on the target devices. No behaviour change. --- mods/features/playlistBatchCollect.js | 33 +++++++++++++++------------ 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/mods/features/playlistBatchCollect.js b/mods/features/playlistBatchCollect.js index 57e0d28e..89e7591a 100644 --- a/mods/features/playlistBatchCollect.js +++ b/mods/features/playlistBatchCollect.js @@ -147,7 +147,7 @@ function _clearState() { // its own collect+reload, while staying set across a SOFT_RELOAD_PAGE // (which does not navigate) so that reload cannot repeat. window.__ttServedFromCache = null; - window.__ttFullReloadDone = {}; + window.__ttFullReloadDone = new Set(); } window.addEventListener('hashchange', _clearState); window.addEventListener('popstate', _clearState); @@ -380,13 +380,18 @@ function playlistKeyFromHash() { try { return String(window.location?.hash || ''); } catch (_) { return ''; } } +// Backed by a Map, not a plain object, because the key is the location hash. +// That is attacker-influenceable via a crafted URL, and writing cache[key] on +// a plain object would let a hash of #__proto__ reach Object.prototype +// (flagged by CodeQL as remote property injection). A Map treats every key as +// ordinary data, with no prototype chain to walk into. function getFullCache() { - if (!window.__ttPlaylistFullCache) window.__ttPlaylistFullCache = {}; + if (!(window.__ttPlaylistFullCache instanceof Map)) window.__ttPlaylistFullCache = new Map(); return window.__ttPlaylistFullCache; } export function getCachedFullPlaylist(key) { - const entry = getFullCache()[key]; + const entry = getFullCache().get(key); return entry && Array.isArray(entry.contents) ? entry.contents : null; } @@ -404,8 +409,8 @@ export function getCachedFullPlaylist(key) { // cycle per visit, which is worth it for correct watched state. export function consumeCachedFullPlaylist(key) { const cache = getFullCache(); - if (!cache[key]) return; - delete cache[key]; + if (!cache.has(key)) return; + cache.delete(key); // Mark this page load as already complete so the scheduler stands down: // it has the whole playlist and no continuation token, and letting it // collect again would store a new cache entry and reload a second time. @@ -431,17 +436,17 @@ function storeFullPlaylist(key, collectedContents) { } const cache = getFullCache(); // Bound the cache: these are full renderer objects and this runs on a TV. - const keys = Object.keys(cache); - if (keys.length >= FULL_CACHE_MAX_PLAYLISTS && !cache[key]) { - const oldest = keys.reduce((a, b) => (cache[a].ts <= cache[b].ts ? a : b)); - delete cache[oldest]; + if (cache.size >= FULL_CACHE_MAX_PLAYLISTS && !cache.has(key)) { + let oldestKey = null, oldestTs = Infinity; + for (const [k2, v] of cache) { if (v.ts < oldestTs) { oldestTs = v.ts; oldestKey = k2; } } + if (oldestKey !== null) cache.delete(oldestKey); } - cache[key] = { contents: initial.contents.concat(collectedContents), ts: Date.now() }; + cache.set(key, { contents: initial.contents.concat(collectedContents), ts: Date.now() }); _log('playlist.full_cache.stored', { key, initial: initial.contents.length, collected: collectedContents.length, - total: cache[key].contents.length, + total: cache.get(key).contents.length, }); return true; } @@ -450,8 +455,8 @@ function storeFullPlaylist(key, collectedContents) { // list would render as a truncated playlist with no way to load the rest, // since the injected response carries no continuation token. function maybeReloadForFullPlaylist(key) { - if (!window.__ttFullReloadDone) window.__ttFullReloadDone = {}; - if (window.__ttFullReloadDone[key]) { + if (!(window.__ttFullReloadDone instanceof Set)) window.__ttFullReloadDone = new Set(); + if (window.__ttFullReloadDone.has(key)) { _log('playlist.full_cache.reload_skipped', { key, reason: 'already_reloaded' }); return; } @@ -459,7 +464,7 @@ function maybeReloadForFullPlaylist(key) { _log('playlist.full_cache.reload_skipped', { key, reason: 'navigated_away' }); return; } - window.__ttFullReloadDone[key] = true; + window.__ttFullReloadDone.add(key); _log('playlist.full_cache.reloading', { key }); try { // Reached through a window global rather than an ES import. This module