Repository navigation
stream.hls: fix 0 and <0 DATERANGE DURATION values - #7074
Conversation
is_date_in_daterange used `daterange.duration or daterange.planned_duration`, so an explicit DURATION of 0 (a valid non-negative value per RFC 8216) was falsy and fell back to PLANNED-DURATION, making a zero-length range match dates instead of none. Fall back only when duration is None.
bastimeyer
left a comment
bastimeyer
left a comment
There was a problem hiding this comment.
Thanks for the PR.
I used an AI assistant to help locate this bug
Please don't run AI agents to try to find arbitrary bugs in FOSS projects (according to your GH profile, you're doing this on all kinds of projects). This creates unnecessary work for maintainers if the bug is neglible, like in this case, even though it's a valid bug. But please don't misunderstand me, fixing bugs is always appreciated. I only have a problem with the intention of submitting PRs like this.
Only the Twitch plugin interacts with HLS dateranges and calls the is_date_in_daterange() method where a duration of 0 and a planned duration greater than 0 could cause an issue. This method is called for checking ads, and ads don't have a duration of 0 with a planned duration that's greater than 0.
The fix isn't fully correct:
DURATION
The duration of the Date Range expressed as a decimal-floating-point number of seconds. It MUST NOT be negative. A single instant in time (e.g., crossing a finish line) SHOULD be represented with a duration of 0. This attribute is OPTIONAL.PLANNED-DURATION
The expected duration of the Date Range expressed as a decimal-floating-point number of seconds. It MUST NOT be negative. This attribute SHOULD be used to indicate the expected duration of a Date Range whose actual duration is not yet known. It is OPTIONAL.
While a protocol specification must be strict, parsers should be lenient with the input data. We must therefore detect negative durations and handle this accordingly, even if the spec forbids these values to exist.
Please check for negative durations and negative planned durations as well, and handle this like the value doesn't exist in this case. Make sure to get the value from total_seconds().
A DURATION of 0 is a valid single instant in time and must not fall back to PLANNED-DURATION. Negative durations are forbidden by RFC 8216, but a parser should be lenient about them, so both DURATION and PLANNED-DURATION are now ignored when negative rather than producing an end date before the start date.
|
Fixed in 67340be. Both DURATION and PLANNED-DURATION are now ignored when negative, checked via Tests are parametrized over zero duration, negative duration with and without a planned fallback, negative planned duration, and both negative. Five of the six fail on current master. On your other point, that's fair. A valid bug that can't actually bite anyone still costs you review time, and I should be weighing that before opening a PR rather than treating 'it's a real bug' as enough on its own. Point taken. |
bastimeyer
changed the title
|
Thanks for fix. Appreciated. |
M3U8.is_date_in_daterangederives a DATERANGE's end from:An explicit
DURATION=0is a valid non-negative decimal-floating-point value (RFC 8216 §4.3.2.7), buttimedelta(0)is falsy, soordiscards it and falls back toPLANNED-DURATION— or, if that's also unset, treats the range as open-ended (start <= date). A zero-length DATERANGE therefore matches dates instead of matching none.Fix
Fall back to
planned_durationonly whenduration is None.Test
Added
test_is_date_in_daterange_zero_duration. Onmastera date after the start is wrongly reported as inside a zero-duration range; it passes with this change. The existing daterange tests stay green, and ruff is clean.AI-assisted contributions
I used an AI assistant (Claude Code) to help locate this bug and draft the test. I verified it myself: reproduced the wrong result red→green against the real
is_date_in_daterange, ran the m3u8 test suite, and confirmed ruff/style pass. I understand the change and take responsibility for it.