Skip to content

Commit

Permalink
Fix off by 1 issue in when relying on keySystems[].maxSessionCacheSize
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
peaBerberian committed Oct 4, 2024
1 parent 4956854 commit 608ecbb
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
7 changes: 6 additions & 1 deletion src/main_thread/decrypt/create_or_load_session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion src/main_thread/decrypt/utils/clean_old_loaded_sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default async function cleanOldLoadedSessions(
loadedSessionsStore: LoadedSessionsStore,
limit: number,
): Promise<void> {
if (limit < 0 || limit >= loadedSessionsStore.getLength()) {
if (limit < 0 || limit > loadedSessionsStore.getLength()) {
return;
}
log.info("DRM: LSS cache limit exceeded", limit, loadedSessionsStore.getLength());
Expand Down

0 comments on commit 608ecbb

Please sign in to comment.