From 8a4162683f9c0a2825089637ea3e79753915e7d8 Mon Sep 17 00:00:00 2001 From: legop3 Date: Sat, 18 Jul 2026 03:22:01 -0400 Subject: [PATCH] snapshot fixes --- server/src/services/turnService/index.js | 21 +++++++++++++++++++ .../src/services/videoAuthService/policy.js | 6 +++--- .../src/services/videoSocketService/index.js | 12 +++++------ 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/server/src/services/turnService/index.js b/server/src/services/turnService/index.js index 2c3643dc..8f3bd4c4 100644 --- a/server/src/services/turnService/index.js +++ b/server/src/services/turnService/index.js @@ -196,6 +196,26 @@ function canDrive(roverId, socket) { return activeDrivers.get(roverId) === socket.id; } +function canRequestLiveVideo(roverId, socket) { + if (!socket) return false; + if (canDrive(roverId, socket)) return true; + + const queue = driverQueues.get(roverId); + if (!queue || getMode() !== MODES.TURNS) { + return false; + } + + /* + This helper is intentionally broader than canDrive(). Bandwidth saving is a + presentation/subscription decision for normal driver clients: the UI keeps + non-current drivers on snapshots, and only asks for live video when it wants + to warm or show the stream. The server should still verify that the socket is + actually attached to this rover, but it should not reject a legitimate queued + driver because the browser and turn timer are a few milliseconds out of sync. + */ + return queue.queue.includes(socket.id); +} + function isQueuedDriver(roverId, socketId) { if (!socketId) return false; const queue = driverQueues.get(roverId); @@ -410,6 +430,7 @@ module.exports = { driverRemoved, cleanupRover, canDrive, + canRequestLiveVideo, isQueuedDriver, getActiveDrivers, turnEvents, diff --git a/server/src/services/videoAuthService/policy.js b/server/src/services/videoAuthService/policy.js index ff836161..ec92f5a0 100644 --- a/server/src/services/videoAuthService/policy.js +++ b/server/src/services/videoAuthService/policy.js @@ -104,12 +104,12 @@ function createVideoAuthPolicy(deps) { if ( !isAudio && shouldUseSnapshotsForNonTurnVideo({ controllableUserCount: countControllableUsers() }) && - !turnService.canDrive(roverId, socket) + !turnService.canRequestLiveVideo(roverId, socket) ) { /* This mirrors videoSocketService's token gate. MediaMTX can ask auth - after a token has been issued, so the active-turn bandwidth rule must - be evaluated here too instead of trusting an older browser decision. + after a token has been issued, so the same "must belong to this rover's + driver queue" rule has to be evaluated here too. */ return false; } diff --git a/server/src/services/videoSocketService/index.js b/server/src/services/videoSocketService/index.js index a0284122..31771fa3 100644 --- a/server/src/services/videoSocketService/index.js +++ b/server/src/services/videoSocketService/index.js @@ -153,15 +153,15 @@ io.on('connection', (socket) => { role !== 'spectator' && !isAdmin(socket) && shouldUseSnapshotsForNonTurnVideo({ controllableUserCount: countControllableUsers() }) && - !turnService.canDrive(baseId, socket) + !turnService.canRequestLiveVideo(baseId, socket) ) { /* - The browser also forces snapshots for non-active turn holders, but - the socket token path must enforce the same rule. Otherwise a stale - component or direct socket caller could still mint a MediaMTX token - while the UI is showing snapshots. + The browser owns the snapshot-vs-live presentation for queued rover + drivers. The server side only verifies that the socket belongs to + this rover's driver queue so legitimate warm-up/switch requests are + not rejected by small turn-timer timing differences. */ - throw new Error('Live video is limited to the active turn'); + throw new Error('Live video is limited to this rover queue'); } } else if (target.type === 'room') { throw new Error('Room cameras now use the snapshot feed');