fix(assets): bound asset duration to prevent viewer crash-loop - #3128
Merged
Conversation
An out-of-range duration (e.g. 9999999999999, typed to mean "forever") reached threading.Event.wait in the viewer, raising OverflowError past C PyTime_t range and crash-looping the device (Sentry ANTHIAS-3E). - Add DURATION_S_MAX (1 year) + clamp_duration to app.models, mirroring the refresh-interval guard - Viewer: clamp duration on read so pre-existing rows can't take the screen down - v2 API: bound duration and default durations via IntegerField min/max - v1/v1.1/v1.2: bound the CharField duration in the parse paths - HTML forms: clamp asset duration and default durations on save Fixes #3122 Co-Authored-By: Claude Fable 5 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR prevents viewer crash-loops caused by out-of-range Asset.duration values reaching threading.Event.wait() (OverflowError), by introducing a shared duration cap and enforcing/clamping it across viewer read paths, server HTML form handlers, and API serializers (v1–v2), with regression tests.
Changes:
- Add
DURATION_S_MAX(1 year) andclamp_duration()helper, and clamp duration in the viewer loop to protect against pre-existing bad DB rows. - Enforce duration bounds on API write paths (v2 via
IntegerField(min_value, max_value), v1-family via serializer parse/prepare paths) and clamp in HTML POST handlers. - Add unit/integration tests covering viewer clamp behavior, HTML form clamping, and API rejection of out-of-range durations.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_viewer.py | Adds regression test asserting viewer clamps oversized duration before calling Event.wait(). |
| tests/test_template_views.py | Adds template-view test ensuring HTML assets_update clamps oversized duration instead of 400’ing. |
| src/anthias_viewer/init.py | Clamps asset['duration'] on read in asset_loop before any waits/playback. |
| src/anthias_server/app/views.py | Clamps posted asset duration and default durations in settings save handler. |
| src/anthias_server/app/models.py | Introduces DURATION_S_MAX and clamp_duration() helper alongside existing refresh-interval clamp. |
| src/anthias_server/api/tests/test_assets.py | Adds API tests ensuring out-of-range durations are rejected across create/update paths. |
| src/anthias_server/api/serializers/v2.py | Adds min/max bounds to v2 duration/default duration integer fields. |
| src/anthias_server/api/serializers/v1_1.py | Adds explicit duration range validation in v1.1 prepare paths. |
| src/anthias_server/api/serializers/mixins.py | Adds duration range validation for v1.2 create paths (with a remaining unhandled non-int parse case). |
| src/anthias_server/api/serializers/init.py | Adds shared parse_duration() + standardized range error message and hooks duration validation into updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot review: int(duration) in the v1.2 create path let a non-integer escape as an unhandled ValueError (500) instead of a validation error keyed on duration like the other versions. Co-Authored-By: Claude Fable 5 <[email protected]>
Copilot review: get_video_duration output bypassed the new bound, so a corrupted container header could still persist an out-of-range duration. Clamp rather than reject — the file itself is playable. Co-Authored-By: Claude Fable 5 <[email protected]>
Copilot review: int(duration_raw) == 0 let a non-integer CharField value escape as an unhandled ValueError (500) instead of a 400 keyed on duration. Co-Authored-By: Claude Fable 5 <[email protected]>
Copilot review: 292 years is ~9.2e9 seconds, not ~2.9e11. Co-Authored-By: Claude Fable 5 <[email protected]>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues Fixed
Description
An out-of-range asset duration — 9999999999999, typed by an operator to mean "forever" — reached
threading.Event.waitin the viewer'sasset_loop, which raisesOverflowErrorfor timeouts past CPyTime_trange. The exception propagates out ofmain(), so the viewer crash-looped and took the screen down (875 Sentry events from one pi5 in under a day).The fix bounds duration at every layer, mirroring the existing
REFRESH_INTERVAL_S_MAX/clamp_refresh_intervalpattern:DURATION_S_MAX(1 year — effectively "forever" for signage) +clamp_duration()inapp.models.duration,default_duration, anddefault_streaming_durationviaIntegerField(min_value, max_value).Validation on real hardware
Exercised on the physical testbeds (Pi 4 aarch64 and x86), not just unit tests:
9999999999999) enabled asset straight into the DB, bypassing the API to simulate a legacy / hand-edited row, then let the liveasset_looprun. The viewer displayed the asset and loggedSleeping for 31536000(the clamped 365 days, not the raw value) with zero crashes — the container'sRestartCountstayed at0. Before the fix this exact scenario crash-looped the viewer.POSTandPATCHto/api/v2/assetswithduration=9999999999999both returned HTTP 400 (Ensure this value is less than or equal to 31536000); a sane duration still returned201.threading.Event.wait(timeout=9999999999999)→OverflowError: timestamp too large to convert to C PyTime_t(the ANTHIAS-3E crash). With the fix,clamp_duration(9999999999999)→31536000and the wait succeeds; confirmedasset_loopcallsclamp_durationbefore the wait. (The Pi 4 testbed is display-headless so itsasset_loopis display-gated — the arch-specific crash-and-fix was confirmed in the real container, and the full live-rotation e2e above ran on x86.)Both testbeds were restored to their pinned images afterward.
Checklist
🤖 Generated with Claude Code