feat(stream): expose upstream metadata headers#684
Merged
Conversation
stackia
marked this pull request as ready for review
July 19, 2026 11:29
Contributor
Documentation previewThe documentation preview has been deployed for this pull request. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 620d21eb22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A response to HEAD must not carry content (RFC 9110 9.3.2), but the short error responses always queued their HTML body. This became visible on this branch because RTSP HEAD can now answer 503, so `HEAD /rtsp/...` against an unreachable upstream returned 52 stray bytes after the header block. Route all five senders through http_send_error(), which drops the body when the request method is HEAD. Also fixes the pre-existing 401 case.
Three related defects in the metadata-probe handling: - recv() returning 0 was allowed to be swallowed whenever a complete response was already buffered, so a server that closed right after its last response left the session waiting for a reply that could never arrive. peer_closed is now sticky on the session and always terminates the exchange once that final response has been handled. - The "drain before treating HUP as fatal" path was special-cased to metadata probes. Generalized it: any session with an outstanding response reads first, then reports the close through the new rtsp_handle_terminal_socket_event(), which is shared with the direct HUP/ERR path so the PLAYING drain and the TEARDOWN-tolerance behaviour stay identical either way. - A response that replaces the control connection (redirect, TEARDOWN reconnect) must not have the old connection's close applied to it. Added a connect_generation counter for this; fd numbers cannot be compared because close()+socket() commonly reuses them. Also stop moving the session to DESCRIBED before the metadata-probe and r2h-duration returns. Neither case advances past DESCRIBE, and DESCRIBED has no handshake timeout and is eligible for the STUN tick to start a SETUP, so a duration query could issue an unwanted SETUP/PLAY upstream after its response had already been produced. Consolidates the metadata plumbing while here: a single rtsp_metadata() accessor replaces nine conn->stream.metadata reach-throughs with their repeated frozen checks, and the per-state auth-retry reset now shares stream_metadata_forget() with the redirect path instead of duplicating a slightly different field list.
stream_metadata_format_number() ran its trailing-zero stripper over a possibly truncated snprintf result, so a large magnitude produced a silently wrong value instead of no value: an upstream `Scale: 1e300` was reported as a 63-digit number roughly 238 orders of magnitude off. It now checks for truncation and the caller omits the field, matching the documented "unknown fields are omitted" contract. Alongside that, two DRY cleanups in the same function: - The eight R2H-* header names lived in three places (the emission switches, the Access-Control-Expose-Headers literal, and implicitly in the field order). They now come from one stream_metadata_header_names[] table, which also generates the expose list, and the five enum fields are emitted through a shared helper instead of five near-identical switch blocks. ~110 lines -> ~50. - stream_metadata_forget() replaces reset_rtsp_negotiation() with an explicit stage bitmask, so the redirect and auth-retry callers express what they forget rather than encoding two slightly different field lists. Replaces the -1/-2/-3 return protocol with named STREAM_EVENT_* constants now that a third value exists.
The two HEAD 503 assertions were vacuous: http.client never reads a HEAD body, so the stray content the server was sending went undetected. Added raw_http_request(), which uses a bare socket, and asserted both that HEAD sends no body and that an ordinary GET error still does. Fills the coverage gaps around the metadata headers: - Redirect and 401-retry both reset learned metadata; neither path had any test. Added redirect_describe_to and challenge_describe_once to the mock RTSP server for this. The redirect test caught a real bug (fixed in the parent commit) where the pre-redirect connection's close failed the probe. - Invalid upstream values are dropped, not echoed: non-numeric Scale, a Range containing a control character, `npt=now-`, and an unrepresentable duration. - mp2t-direct over RTSP was only exercised via multicast. MockRTSPServer gained encapsulate_rtp=False (bare MPEG-TS in the interleaved frames) plus a configurable setup_transport, so MP2T/TCP upstreams are now covered, with a companion test asserting that a non-RTP SDP alone does not veto mp2t-rtp when the media turns out to be RTP after all. - Access-Control-Expose-Headers is asserted absent when cors-allow-origin is unset; only the present case was checked before. Renames the TS null packet helper to TS_NULL_PACKET so the RTSP mock can reuse it instead of duplicating the definition.
Condenses the R2H-* documentation down to the header table and syncs the English translation, dropping the snapshot/security, HEAD-behaviour and CORS paragraphs along with the per-page notes in fcc-setup and video-snapshot. Records the repo's admonition convention (GitHub alerts, not VitePress ::: containers) and the new terminology in the translation skill's memory.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b436e3a9da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
Summary
R2H-*upstream metadata headers for RTSP, multicast, FCC, MPEG-TS, and JPEG snapshot responsesMotivation
Downstream HTTP clients need a consistent way to understand how a stream was sourced and negotiated without exposing sensitive upstream RTSP details or requiring a media parser. HEAD requests should provide all metadata available from static configuration or SDP without creating a media stream.
Implementation notes
Metadata is stored in a fixed-size
stream_metadata_tand frozen when response headers are emitted. Unknown or unconfirmed values are omitted. RTSP HEAD probes stop after DESCRIBE and synchronously release the control connection before queuing the HTTP response, preventing later upstream hangups from dropping the response. The implementation intentionally does not expose FEC state orR2H-Metadata-Version.Validation