From 35dce2b7f8c61dec918daf5344d858e6539bf81a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Marcoux?= Date: Thu, 9 Jul 2026 08:40:31 +0200 Subject: [PATCH] fix(timeshift): drop stale byte state and harden descriptor update on seek MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review follow-ups on the position fix: - When the position actually moves, drop content_length/serving_range from the pool entry — they describe the PREVIOUS position's file, and keeping them would feed the near-EOF/displacement heuristics another programme's size (a metadata probe near the new file's EOF could displace live playback, or a genuine scrub could be misjudged as a probe). The next successful open repopulates both. - Guard the update under the pool lock and skip it when the entry has vanished (Redis restart/eviction) — a bare HSET would resurrect a partial, TTL-less hash that answers every later request for that session_id with 503. - Align the reuse-path timezone lookup with the fresh path (is_active=True on the default-profile filter). Three more tests: byte state dropped on move / kept on same-position update, vanished entry never resurrected, tz fallback to the reserved profile when no active default exists. --- apps/timeshift/tests/test_views.py | 80 ++++++++++++++++++++++++++++++ apps/timeshift/views.py | 26 +++++++--- 2 files changed, 100 insertions(+), 6 deletions(-) diff --git a/apps/timeshift/tests/test_views.py b/apps/timeshift/tests/test_views.py index 3bbd77e1..ebe3069e 100644 --- a/apps/timeshift/tests/test_views.py +++ b/apps/timeshift/tests/test_views.py @@ -453,6 +453,12 @@ class _FakeRedis: hash_value = self.store.get(key) return hash_value.get(field) if isinstance(hash_value, dict) else None + def hdel(self, key, *fields): + hash_value = self.store.get(key) + if not isinstance(hash_value, dict): + return 0 + return sum(1 for f in fields if hash_value.pop(f, None) is not None) + def expire(self, key, ttl): return 1 if key in self.store else 0 @@ -1678,6 +1684,80 @@ class TimeshiftSessionReuseTests(TestCase): "2026-06-08:19-30", ) + def test_position_move_drops_previous_byte_state(self): + # content_length / serving_range describe ONE provider file; carrying + # them across a seek would feed the near-EOF/displacement heuristics + # another programme's size. A same-position update must keep them. + _seed_pool_session(self.redis, session_id=self.SESSION) + self.redis.hset(self._pool_key(), mapping={ + "content_length": "2000000000", "serving_range": "start", + }) + self._call_reused_session( + "2026-06-08:17-30", "timeshift_8_2026-06-08-17-30", + ) + self.assertIsNone(self.redis.hget(self._pool_key(), "content_length")) + self.assertIsNone(self.redis.hget(self._pool_key(), "serving_range")) + + # Same-position call (media unchanged): byte state survives. + self.redis.hset(self._pool_key(), mapping={ + "content_length": "1000000", "serving_range": "range", + }) + self._call_reused_session( + "2026-06-08:17-30", "timeshift_8_2026-06-08-17-30", + ) + self.assertEqual( + self.redis.hget(self._pool_key(), "content_length"), "1000000", + ) + + def test_position_update_never_resurrects_vanished_entry(self): + # If the pool key expired/vanished mid-request, writing to it would + # recreate a partial TTL-less hash that 503-wedges the session_id. + views._update_pool_position( + self.redis, self.SESSION, + media_id="timeshift_8_2026-06-08-17-30", + provider_timestamp="2026-06-08:19-30", + ) + self.assertFalse(self.redis.exists(self._pool_key())) + + def test_reused_session_tz_falls_back_to_reserved_profile(self): + # No active default profile -> the reserved profile's server_info is + # the only tz source left; conversion must still apply. + _seed_pool_session(self.redis, session_id=self.SESSION) + profile = MagicMock(id=31, custom_properties={ + "server_info": {"timezone": "Europe/Brussels"}, + }) + account = MagicMock(id=1) + account.profiles.filter.return_value.first.return_value = None + ok = MagicMock(status_code=200) + with patch.object(views.M3UAccount.objects, "get", return_value=account), \ + patch.object(views, "_attempt_timeshift_stream", + return_value=ok) as attempt_mock: + views._stream_reused_session( + self.redis, + session_id=self.SESSION, + descriptor={ + "account_id": "1", + "stream_id": "111", + "provider_timestamp": "2026-06-08:19-00", + }, + profile=profile, + channel=self.channel, + media_id="timeshift_8_2026-06-08-17-30", + safe_ts="2026-06-08-17-30", + timestamp="2026-06-08:17-30", + duration_minutes=40, + client_id=self.SESSION, + client_ip="1.2.3.4", + range_header=None, + channel_logo_id=None, + user=self.user, + debug=False, + ) + self.assertEqual( + attempt_mock.call_args.kwargs["provider_timestamp"], + "2026-06-08:19-30", + ) + def test_seek_same_session_new_timestamp_serves_new_position(self): # End-to-end repro of the field report: idle session anchored at # 17-00, then a timestamp-jump request for 17-30 carrying the SAME diff --git a/apps/timeshift/views.py b/apps/timeshift/views.py index 832e831c..77454664 100644 --- a/apps/timeshift/views.py +++ b/apps/timeshift/views.py @@ -597,15 +597,28 @@ def _update_pool_position(redis_client, session_id, *, media_id, provider_timest A seek keeps the session (provider-slot continuity) but changes the catch-up position; the descriptor must follow it so later fingerprint matches and same-channel displacement compare against the position the - session is really on. + session is really on. When the position actually moves, the byte state of + the PREVIOUS position's file (content_length, serving_range) is dropped — + keeping it would feed the near-EOF/displacement heuristics another + programme's size; the next successful open repopulates both. """ if redis_client is None or not session_id: return + key = _pool_key(session_id) try: - redis_client.hset(_pool_key(session_id), mapping={ - "media_id": str(media_id), - "provider_timestamp": str(provider_timestamp), - }) + with _pool_lock(redis_client, session_id): + entry = redis_client.hgetall(key) + if not entry: + # Entry vanished (Redis restart/eviction): writing here would + # resurrect a partial, TTL-less hash that wedges the session. + return + position_changed = entry.get("media_id") != str(media_id) + redis_client.hset(key, mapping={ + "media_id": str(media_id), + "provider_timestamp": str(provider_timestamp), + }) + if position_changed: + redis_client.hdel(key, "content_length", "serving_range") except Exception as exc: logger.debug("Timeshift pool position update failed: %s", exc) @@ -940,7 +953,8 @@ def _stream_reused_session( # conversion. provider_tz_name = None tz_profile = ( - m3u_account.profiles.filter(is_default=True).first() or profile + m3u_account.profiles.filter(is_active=True, is_default=True).first() + or profile ) server_info = (tz_profile.custom_properties or {}).get("server_info") or {} if isinstance(server_info, dict):