From fd1e40a6573a3b7afe42f29e5e4336c5afaa3ea0 Mon Sep 17 00:00:00 2001 From: Federico Cau Date: Wed, 16 Sep 2026 23:48:18 +0200 Subject: [PATCH] fix(scraper): push cookie, pageId and authUser to the scraper as one snapshot setCookie writes the three values in one DataStore edit, but they were pushed to the scraper by three independent collectors and stored as three plain vars read one by one while building request headers. After an account switch a request could still carry the new pageId with the previous authuser or cookie, for a short window. DataStoreManager now exposes youTubeSession, a single flow over one Preferences snapshot; CommonRepositoryImpl collects it and calls YouTube.setSession. Ytmusic keeps a @Volatile immutable Session that ytClient and getAuthorizationHeader read once per request; the per-field setters and the cached cookieMap are gone, the SAPISID hash is computed from the cookie actually sent. visitorData is still refreshed only when the cookie changes, and the netscape cookie file is written once per session change instead of twice. AccountRepositoryImpl no longer writes youTube.cookie directly: both callers already wrote DataStore. Refs maxrave-dev/SimpMusic#2505 --- .../data/dataStore/DataStoreManagerImpl.kt | 12 ++++ .../data/repository/AccountRepositoryImpl.kt | 1 - .../data/repository/CommonRepositoryImpl.kt | 45 ++++++--------- .../data/model/cookie/YouTubeSession.kt | 12 ++++ .../domain/manager/DataStoreManager.kt | 2 + .../maxrave/kotlinytmusicscraper/YouTube.kt | 24 +++----- .../maxrave/kotlinytmusicscraper/Ytmusic.kt | 57 ++++++++++++------- 7 files changed, 91 insertions(+), 62 deletions(-) create mode 100644 domain/src/commonMain/kotlin/com/maxrave/domain/data/model/cookie/YouTubeSession.kt diff --git a/data/src/commonMain/kotlin/com/maxrave/data/dataStore/DataStoreManagerImpl.kt b/data/src/commonMain/kotlin/com/maxrave/data/dataStore/DataStoreManagerImpl.kt index ec6decd0..fd39a1fc 100644 --- a/data/src/commonMain/kotlin/com/maxrave/data/dataStore/DataStoreManagerImpl.kt +++ b/data/src/commonMain/kotlin/com/maxrave/data/dataStore/DataStoreManagerImpl.kt @@ -10,6 +10,7 @@ import androidx.datastore.preferences.core.stringPreferencesKey import com.maxrave.common.SELECTED_LANGUAGE import com.maxrave.common.SUPPORTED_LANGUAGE import com.maxrave.common.SponsorBlockType +import com.maxrave.domain.data.model.cookie.YouTubeSession import com.maxrave.domain.data.model.network.ProxyConfiguration import com.maxrave.domain.data.player.ReverbPreset import com.maxrave.domain.manager.DataStoreManager @@ -28,6 +29,7 @@ import com.maxrave.logger.Logger import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.IO import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map import kotlinx.coroutines.runBlocking @@ -196,6 +198,16 @@ internal class DataStoreManagerImpl( preferences[AUTH_USER] ?: 0 } + override val youTubeSession: Flow = + settingsDataStore.data + .map { preferences -> + YouTubeSession( + cookie = preferences[COOKIE] ?: "", + pageId = preferences[PAGE_ID] ?: "", + authUser = preferences[AUTH_USER] ?: 0, + ) + }.distinctUntilChanged() + override suspend fun setCookie( cookie: String, pageId: String?, diff --git a/data/src/commonMain/kotlin/com/maxrave/data/repository/AccountRepositoryImpl.kt b/data/src/commonMain/kotlin/com/maxrave/data/repository/AccountRepositoryImpl.kt index 8fe9840f..195e7a7c 100644 --- a/data/src/commonMain/kotlin/com/maxrave/data/repository/AccountRepositoryImpl.kt +++ b/data/src/commonMain/kotlin/com/maxrave/data/repository/AccountRepositoryImpl.kt @@ -24,7 +24,6 @@ internal class AccountRepositoryImpl( override fun getAccountInfo(cookie: String): Flow> = flow { - youTube.cookie = cookie delay(1000) youTube .getAccountListWithPageId(cookie) diff --git a/data/src/commonMain/kotlin/com/maxrave/data/repository/CommonRepositoryImpl.kt b/data/src/commonMain/kotlin/com/maxrave/data/repository/CommonRepositoryImpl.kt index 2cc8c347..ffb818d8 100644 --- a/data/src/commonMain/kotlin/com/maxrave/data/repository/CommonRepositoryImpl.kt +++ b/data/src/commonMain/kotlin/com/maxrave/data/repository/CommonRepositoryImpl.kt @@ -70,42 +70,35 @@ internal class CommonRepositoryImpl( ) } } - val ytCookieJob = + // cookie, pageId and authUser are written by DataStoreManager.setCookie in one + // DataStore edit; youTubeSession is one flow over that snapshot, so the scraper + // never gets a new pageId with a stale authUser or cookie. + val sessionJob = launch { - dataStoreManager.cookie.distinctUntilChanged().collectLatest { cookie -> - if (cookie.isNotEmpty()) { - youTube.cookie = cookie + var lastCookie: String? = null + dataStoreManager.youTubeSession.collectLatest { s -> + youTube.setSession( + cookie = s.cookie.ifEmpty { null }, + pageId = s.pageId.ifEmpty { null }, + authUser = s.authUser, + ) + // Same trigger as the old cookie-only collector: a channel switch on the + // same cookie must not refetch visitorData. + if (s.cookie.isNotEmpty() && s.cookie != lastCookie) { youTube.visitorData()?.let { youTube.visitorData = it + lastCookie = s.cookie } } else { - youTube.cookie = null + lastCookie = s.cookie } - Logger.d("YouTube", "New cookie") + Logger.d("YouTube", "New session") localDataSource.getUsedGoogleAccount()?.netscapeCookie?.let { writeTextToFile(it, cookiePath) Logger.w("YouTube", "Wrote cookie to file") } } } - val pageIdJob = - launch { - dataStoreManager.pageId.distinctUntilChanged().collectLatest { pageId -> - youTube.pageId = pageId.ifEmpty { null } - Logger.d("YouTube", "New pageId") - localDataSource.getUsedGoogleAccount()?.netscapeCookie?.let { - writeTextToFile(it, cookiePath) - Logger.w("YouTube", "Wrote cookie to file") - } - } - } - val authUserJob = - launch { - dataStoreManager.authUser.distinctUntilChanged().collectLatest { authUser -> - youTube.authUser = authUser - Logger.d("YouTube", "New authUser") - } - } val usingProxy = launch { combine( @@ -269,9 +262,7 @@ internal class CommonRepositoryImpl( } localeJob.join() - ytCookieJob.join() - pageIdJob.join() - authUserJob.join() + sessionJob.join() usingProxy.join() dataSyncIdJob.join() visitorDataJob.join() diff --git a/domain/src/commonMain/kotlin/com/maxrave/domain/data/model/cookie/YouTubeSession.kt b/domain/src/commonMain/kotlin/com/maxrave/domain/data/model/cookie/YouTubeSession.kt new file mode 100644 index 00000000..eccb2daf --- /dev/null +++ b/domain/src/commonMain/kotlin/com/maxrave/domain/data/model/cookie/YouTubeSession.kt @@ -0,0 +1,12 @@ +package com.maxrave.domain.data.model.cookie + +/** + * The three values the YouTube scraper needs together: a brand channel's pageId is only + * valid with the authuser that owns it, and both only with the cookie they came from. + * Read as one DataStore snapshot so no consumer ever sees a mixed pair. + */ +data class YouTubeSession( + val cookie: String, + val pageId: String, + val authUser: Int, +) diff --git a/domain/src/commonMain/kotlin/com/maxrave/domain/manager/DataStoreManager.kt b/domain/src/commonMain/kotlin/com/maxrave/domain/manager/DataStoreManager.kt index f0823a36..99c3bb1e 100644 --- a/domain/src/commonMain/kotlin/com/maxrave/domain/manager/DataStoreManager.kt +++ b/domain/src/commonMain/kotlin/com/maxrave/domain/manager/DataStoreManager.kt @@ -1,5 +1,6 @@ package com.maxrave.domain.manager +import com.maxrave.domain.data.model.cookie.YouTubeSession import com.maxrave.domain.data.model.network.ProxyConfiguration import com.maxrave.domain.data.player.ReverbPreset import kotlinx.coroutines.flow.Flow @@ -64,6 +65,7 @@ interface DataStoreManager { val cookie: Flow val pageId: Flow val authUser: Flow + val youTubeSession: Flow suspend fun setCookie( cookie: String, diff --git a/service/kotlinYtmusicScraper/src/commonMain/kotlin/com/maxrave/kotlinytmusicscraper/YouTube.kt b/service/kotlinYtmusicScraper/src/commonMain/kotlin/com/maxrave/kotlinytmusicscraper/YouTube.kt index 59915474..d3fd6111 100644 --- a/service/kotlinYtmusicScraper/src/commonMain/kotlin/com/maxrave/kotlinytmusicscraper/YouTube.kt +++ b/service/kotlinYtmusicScraper/src/commonMain/kotlin/com/maxrave/kotlinytmusicscraper/YouTube.kt @@ -169,23 +169,17 @@ class YouTube { /** * Set cookie and authentication header for client (for log in option) */ - var cookie: String? - get() = ytMusic.cookie - set(value) { - ytMusic.cookie = value - } + val cookie: String? get() = ytMusic.cookie - var pageId: String? - get() = ytMusic.pageId - set(value) { - ytMusic.pageId = value - } + val pageId: String? get() = ytMusic.pageId - var authUser: Int - get() = ytMusic.authUser - set(value) { - ytMusic.authUser = value - } + val authUser: Int get() = ytMusic.authUser + + fun setSession( + cookie: String?, + pageId: String?, + authUser: Int, + ) = ytMusic.setSession(cookie, pageId, authUser) /** * TIDAL credentials, backed by [Ytmusic]. Set by the data layer from cached remote config. diff --git a/service/kotlinYtmusicScraper/src/commonMain/kotlin/com/maxrave/kotlinytmusicscraper/Ytmusic.kt b/service/kotlinYtmusicScraper/src/commonMain/kotlin/com/maxrave/kotlinytmusicscraper/Ytmusic.kt index d347a555..d4fce0df 100644 --- a/service/kotlinYtmusicScraper/src/commonMain/kotlin/com/maxrave/kotlinytmusicscraper/Ytmusic.kt +++ b/service/kotlinYtmusicScraper/src/commonMain/kotlin/com/maxrave/kotlinytmusicscraper/Ytmusic.kt @@ -82,6 +82,7 @@ import okio.Path.Companion.toPath import okio.SYSTEM import okio.buffer import okio.use +import kotlin.concurrent.Volatile import kotlin.time.ExperimentalTime private const val TAG = "YouTubeScraperClient" @@ -122,20 +123,35 @@ class Ytmusic { var visitorData: String? = null var dataSyncId: String? = null private var poTokenChallengeRequestKey = "O43z0dpjhgX20SCx4KAo" - var cookie: String? = null - set(value) { - field = value - cookieMap = if (value == null) emptyMap() else parseCookieString(value) - extractor.logIn(value) - } + // The three request-identity values are swapped as ONE immutable snapshot and read once + // per request (see ytClient / getAuthorizationHeader): a request must never see the + // pageId of one account with the authuser or cookie of another. + private data class Session( + val cookie: String?, + val pageId: String?, + val authUser: Int, + ) - var pageId: String? = null + @Volatile + private var session = Session(cookie = null, pageId = null, authUser = 0) + + val cookie: String? get() = session.cookie + val pageId: String? get() = session.pageId // Index of the Google account inside the browser session the cookie came from // (the `authuser` query param on youtube.com). A brand channel's pageId is only // valid together with the authuser that owns it; sending 0 for a channel of the // second signed-in account makes YouTube answer as if not logged in. - var authUser: Int = 0 + val authUser: Int get() = session.authUser + + fun setSession( + cookie: String?, + pageId: String?, + authUser: Int, + ) { + session = Session(cookie, pageId, authUser) + extractor.logIn(cookie) + } // TIDAL credentials. Empty until CommonRepositoryImpl pushes the values fetched from the // remote config (cached in DataStore). Deliberately NOT hard-coded in source — while @@ -143,8 +159,6 @@ class Ytmusic { var tidalClientId: String = "" var tidalClientSecret: String = "" - private var cookieMap = emptyMap() - var proxy: ProxyConfig? = null set(value) { field = value @@ -214,13 +228,14 @@ class Ytmusic { isUsingReferer: Boolean = true, customCookie: String? = null, ) { + val s = session contentType(ContentType.Application.Json) headers { append("X-Goog-Api-Format-Version", "1") append("X-YouTube-Client-Name", "${client.xClientName ?: 1}") append("X-YouTube-Client-Version", client.clientVersion) - append("X-Goog-Authuser", authUser.toString()) - pageId?.let { + append("X-Goog-Authuser", s.authUser.toString()) + s.pageId?.let { append("X-Goog-Pageid", it) } append("x-origin", "https://music.youtube.com") @@ -228,12 +243,13 @@ class Ytmusic { append("Referer", client.referer) } if (setLogin) { - val cookie = customCookie ?: this@Ytmusic.cookie + val cookie = customCookie ?: s.cookie cookie?.let { cookie -> append("Cookie", cookie) - if ("SAPISID" !in cookieMap || "__Secure-3PAPISID" !in cookieMap) return@let + val parsedCookies = parseCookieString(cookie) + if ("SAPISID" !in parsedCookies || "__Secure-3PAPISID" !in parsedCookies) return@let val currentTime = now().toInstant(TimeZone.currentSystemDefault()).epochSeconds / 1000 - val sapisidCookie = cookieMap["SAPISID"] ?: cookieMap["__Secure-3PAPISID"] + val sapisidCookie = parsedCookies["SAPISID"] ?: parsedCookies["__Secure-3PAPISID"] val sapisidHash = sha1("$currentTime $sapisidCookie https://music.youtube.com") Logger.d(TAG, "SAPI SID Hash: SAPISIDHASH ${currentTime}_$sapisidHash") append("Authorization", "SAPISIDHASH ${currentTime}_$sapisidHash") @@ -245,15 +261,18 @@ class Ytmusic { } @OptIn(ExperimentalTime::class) - internal fun getAuthorizationHeader(): String? = - cookie?.let { cookie -> - if ("SAPISID" !in cookieMap || "__Secure-3PAPISID" !in cookieMap) null + internal fun getAuthorizationHeader(): String? { + val s = session + return s.cookie?.let { cookie -> + val parsedCookies = parseCookieString(cookie) + if ("SAPISID" !in parsedCookies || "__Secure-3PAPISID" !in parsedCookies) return@let null val currentTime = now().toInstant(TimeZone.currentSystemDefault()).epochSeconds / 1000 - val sapisidCookie = cookieMap["SAPISID"] ?: cookieMap["__Secure-3PAPISID"] + val sapisidCookie = parsedCookies["SAPISID"] ?: parsedCookies["__Secure-3PAPISID"] val sapisidHash = sha1("$currentTime $sapisidCookie https://music.youtube.com") Logger.d(TAG, "SAPI SID Hash: SAPISIDHASH ${currentTime}_$sapisidHash") "SAPISIDHASH ${currentTime}_$sapisidHash" } + } fun getNewPipePlayer(videoId: String): List> = extractor.newPipePlayer(videoId)