From 608ecbb4ae2610bb59c832c0412c676c7fdfe832 Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Fri, 4 Oct 2024 15:28:50 +0200 Subject: [PATCH 1/2] Fix off by 1 issue in when relying on `keySystems[].maxSessionCacheSize` `keySystems[].maxSessionCacheSize', which allows to limit the number of created `MediaKeySession`, was relied on before creating a new `MediaKeySession` (this is good I guess). However it considered if we were over the limit at that instant, were in reality the logic should probably also account for the `MediaKeySession` that will be created just after. The current behavior leads to having a supplementary `MediaKeySession` than what `keySystems[].maxSessionCacheSize` authorizes. To fix it, I simply remove `1` from the limit we want to enforce in that call, unless removing `1` would lead to a negative number as this has a special semantic (which is to have no limit). This still works for `0` like it worked before (which is strangely equivalent to setting it to `1` in our logic, but setting it to `0` is just asking for bugs anyway I guess :p). NOTE: We could have a plausible justification for our precedent behavior that it was only setting the "cache size" and thus excluding the last "actually relied on" one, but this is not how it is documented in the API documentation, so to me this is a bug. --- src/main_thread/decrypt/create_or_load_session.ts | 7 ++++++- src/main_thread/decrypt/utils/clean_old_loaded_sessions.ts | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main_thread/decrypt/create_or_load_session.ts b/src/main_thread/decrypt/create_or_load_session.ts index 23587597c2..8bb8cfaab8 100644 --- a/src/main_thread/decrypt/create_or_load_session.ts +++ b/src/main_thread/decrypt/create_or_load_session.ts @@ -82,7 +82,12 @@ export default async function createOrLoadSession( } } - await cleanOldLoadedSessions(loadedSessionsStore, maxSessionCacheSize); + await cleanOldLoadedSessions( + loadedSessionsStore, + // Account for the next session we will be creating + // Note that `maxSessionCacheSize < 0 has special semantic (no limit)` + maxSessionCacheSize <= 0 ? maxSessionCacheSize : maxSessionCacheSize - 1, + ); if (cancelSignal.cancellationError !== null) { throw cancelSignal.cancellationError; // stop here if cancelled since } diff --git a/src/main_thread/decrypt/utils/clean_old_loaded_sessions.ts b/src/main_thread/decrypt/utils/clean_old_loaded_sessions.ts index c01a00fe19..9cc16138e6 100644 --- a/src/main_thread/decrypt/utils/clean_old_loaded_sessions.ts +++ b/src/main_thread/decrypt/utils/clean_old_loaded_sessions.ts @@ -30,7 +30,7 @@ export default async function cleanOldLoadedSessions( loadedSessionsStore: LoadedSessionsStore, limit: number, ): Promise { - if (limit < 0 || limit >= loadedSessionsStore.getLength()) { + if (limit < 0 || limit > loadedSessionsStore.getLength()) { return; } log.info("DRM: LSS cache limit exceeded", limit, loadedSessionsStore.getLength()); From 04d66cc77fc68c2a916a7e549e6dda6ef8a34b47 Mon Sep 17 00:00:00 2001 From: Florent Date: Tue, 15 Oct 2024 14:22:11 +0200 Subject: [PATCH 2/2] fix: clean old loaded session so the limit is correctly respected --- src/main_thread/decrypt/utils/clean_old_loaded_sessions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main_thread/decrypt/utils/clean_old_loaded_sessions.ts b/src/main_thread/decrypt/utils/clean_old_loaded_sessions.ts index 9cc16138e6..c01a00fe19 100644 --- a/src/main_thread/decrypt/utils/clean_old_loaded_sessions.ts +++ b/src/main_thread/decrypt/utils/clean_old_loaded_sessions.ts @@ -30,7 +30,7 @@ export default async function cleanOldLoadedSessions( loadedSessionsStore: LoadedSessionsStore, limit: number, ): Promise { - if (limit < 0 || limit > loadedSessionsStore.getLength()) { + if (limit < 0 || limit >= loadedSessionsStore.getLength()) { return; } log.info("DRM: LSS cache limit exceeded", limit, loadedSessionsStore.getLength());