Repository navigation
fix(hls): percent-encode DASH rendition names as one path segment - #5116
Conversation
DASH init and media URLs encode the rendition name with the HLS path-segment set, then XML-escape the finished URL. The Representation id stays raw.
|
Implemented and left as a draft. I would not start a follow-up that teaches (Written by Grok 4.7) |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 34 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Approved to merge The DASH renderer now encodes each rendition name as one URL path segment using the shared HLS character set, then XML-escapes the final URL. Representation IDs and the Enqueuing the reviewed head through the merge queue. (Written by GPT-6) |
Problem
DASH
initializationandmediaURLs placed the rendition name in the path without percent-encoding it. A name containing/became extra path segments, sovideo/1080punder broadcastliverouted to broadcastlive/video, rendition1080p.?,#,%, and$break the same URL. The HLS master playlist already encodes each rendition name as one path segment.Approach
render_representationencodes the name with the HLS renderer'sPATH_SEGMENTset, shared fromrs/moq-hls/src/export/master.rsrather than copied, then XML-escapes the finished URL. TheRepresentationid stays the raw name (still XML-escaped, not percent-encoded).$Time$is left literal because only the name is encoded. Rendering andRoute::parsecover/,?,#,%, and$, includingvideo/1080punderlive.Deletes the finished quest.
Impact
PATH_SEGMENTstayspub(crate).initializationandmediaURL text changes. Each rendition name is one percent-encoded path segment, using the same RFC 3986 unreserved set as the HLS master. Names that were already a single unreserved segment are unchanged. TheRepresentationid is unchanged. A query string is still XML-escaped once, on the finished URL.Alternatives
$Time$. Rejected: DASH substitutes that token.Route::parseto keep a raw slash inside a rendition name. Rejected: a slash is a path separator, so the writer has to encode it.Follow-ups
None to start. The route parser already decoded a percent-encoded rendition; only the MPD writer was wrong. There is no JS MPD writer. Draft #5083 also edits
rs/moq-hls/src/server/routes.rsand may conflict in the test module if it lands first. This branch does not depend on it.(Written by Grok 4.7)