Winlin 3422d61cfb Claude: Report on_play and on_publish rejection to the client. v8.0.34 (#4737) ...

Reimplements the fix proposed in #4736, designed from its problem
statement rather than its diff.

When an `on_play` or `on_publish` HTTP callback rejects a client, SRS
blocked it but never reported why — and on RTMP it reported the opposite
first. Every hook rejection arrives as the same `ERROR_RESPONSE_CODE`,
so each protocol now translates that one error into whatever status its
transport can carry.

- **HTTP-FLV and HLS**: add a single root error handler,
`srs_http_stream_serve_error()`, which maps `ERROR_SYSTEM_AUTH` onto
`401` and leaves every other error closing the connection as SRS has
always done. A failing branch only transforms the error code; the root
of the handler decides the status, so no branch responds on its own. It
sends the error code and its short name only — never `srs_error_desc`,
which carries the callback URL and the backend's response. This required
exposing `header_wrote()` on `ISrsHttpResponseWriter`, so the handler
never rewrites a response already on the wire.
- **WHEP**: reuse the status mapping WHIP already had. The play path
used `srs_error_wrap()` where publish used
`srs_error_transform(ERROR_SYSTEM_AUTH, ...)`, which is the entire
reason a rejected viewer got `500` instead of `401`. The deprecated JSON
play and publish APIs are annotated, since they answer HTTP 200 with a
fixed `{"code":400}` for every failure and must not be extended.
- **RTSP**: map the same transformed error onto `401 Unauthorized` in
`DESCRIBE`.
- **RTMP**: no status to fix, only an ordering one. `start_play()` and
`start_fmle_publish()` sent `onStatus(NetStream.Play.Start /
Publish.Start)` before the hook was consulted, so a rejected client was
told it was live and then silently disconnected. Authorization now runs
first on both sides, with the publish check extracted into
`SrsRtmpConn::authorize_publish()`.
- **SRT**: unchanged. MPEG-TS over SRT has no application-level status
channel, so refusal without a reason is the final, correct behavior, and
a test locks that limitation in.
- **Testability**: `SrsHlsStream` and `SrsHlsVirtualConn` were not
mockable, so they were converted to the codebase's standard shape —
globals captured in the constructor, `security_` typed as the interface,
and the timer subscription moved out of the constructor into
`assemble()`, forwarded by `SrsVodStream` and called at every
construction site. Behavior-preserving.
- **Skill**: `references/testable-code.md` becomes the single owner of
the test-first contract and the C++ mockability patterns this change
followed, plus a rule to reimplement a rejected reference PR from its
problem statement rather than its diff.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

2026-09-13 17:02:25 -04:00

Symbolic link

12 B

Raw Permalink Blame History

View Raw