Skip to content

Commit 0cf67fa

Browse files
Copilotfinger563
andauthored
Fix RTSP server crash on TCP transport connect (#602)
* Initial plan * Fix RTSP server crash on TCP transport connect Three fixes: 1. Fix race condition on sessions_ map - accept_task_function now locks session_mutex_ when modifying sessions_, matching the locking in session_task_function that iterates/erases sessions. 2. Teardown session after failed SETUP - when TCP transport is rejected (or any SETUP parse failure), the session is properly torn down so it gets cleaned up instead of remaining as a zombie. 3. Include CSeq in 461 response - the "Unsupported Transport" response now includes the sequence number per RTSP protocol. Co-authored-by: finger563 <213467+finger563@users.noreply.github.com> * Fix RTSP server crash on TCP transport connect Co-authored-by: finger563 <213467+finger563@users.noreply.github.com> * Remove codeql artifact Co-authored-by: finger563 <213467+finger563@users.noreply.github.com> * ensure all false return paths from `parse_rtsp_setup_request` respond with an error to the client * update to use existing function modified instead of new function --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: finger563 <213467+finger563@users.noreply.github.com> Co-authored-by: William Emfinger <waemfinger@gmail.com>
1 parent e95cd45 commit 0cf67fa

3 files changed

Lines changed: 26 additions & 8 deletions

File tree

components/rtsp/include/rtsp_session.hpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,11 @@ class RtspSession : public BaseComponent {
136136

137137
/// Handle an invalid RTSP request
138138
/// @param request The request to handle
139+
/// @param code The response code to send (default: 400)
140+
/// @param message The response message to send (default: "Bad Request")
139141
/// @return True if the request was handled successfully, false otherwise
140-
bool handle_rtsp_invalid_request(std::string_view request);
142+
bool handle_rtsp_invalid_request(std::string_view request, int code = 400,
143+
std::string_view message = "Bad Request");
141144

142145
/// Handle a single RTSP request
143146
/// @note Requests are of the form "METHOD RTSP_PATH RTSP_VERSION"

components/rtsp/src/rtsp_server.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,10 @@ bool RtspServer::accept_task_function(std::mutex &m, std::condition_variable &cv
186186

187187
// add the session to the list of sessions
188188
auto session_id = session->get_session_id();
189-
sessions_.emplace(session_id, std::move(session));
189+
{
190+
std::lock_guard<std::mutex> lk(session_mutex_);
191+
sessions_.emplace(session_id, std::move(session));
192+
}
190193

191194
// start the session task if it is not already running
192195
using namespace std::placeholders;

components/rtsp/src/rtsp_session.cpp

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,9 @@ bool RtspSession::handle_rtsp_setup(std::string_view request) {
152152
int client_rtp_port;
153153
int client_rtcp_port;
154154
if (!parse_rtsp_setup_request(request, rtsp_path, client_rtp_port, client_rtcp_port)) {
155-
// the parse function will send the response, so we just need to return
155+
// the parse function will send the response, so we just need to
156+
// teardown the session since setup failed and streaming cannot proceed
157+
teardown();
156158
return false;
157159
}
158160
// parse the sequence number from the request
@@ -215,11 +217,10 @@ bool RtspSession::handle_rtsp_teardown(std::string_view request) {
215217
return send_response(code, message, sequence_number, headers);
216218
}
217219

218-
bool RtspSession::handle_rtsp_invalid_request(std::string_view request) {
220+
bool RtspSession::handle_rtsp_invalid_request(std::string_view request, int code,
221+
std::string_view message) {
219222
logger_.info("RTSP invalid request");
220223
// create a response
221-
int code = 400;
222-
std::string message = "Bad Request";
223224
int sequence_number = 0;
224225
if (!parse_rtsp_command_sequence(request, sequence_number)) {
225226
return send_response(code, message);
@@ -357,29 +358,36 @@ bool RtspSession::parse_rtsp_setup_request(std::string_view request, std::string
357358
// parse the rtsp path from the request
358359
rtsp_path = parse_rtsp_path(request);
359360
if (rtsp_path.empty()) {
361+
logger_.error("Failed to parse RTSP path from request");
362+
handle_rtsp_invalid_request(request);
360363
return false;
361364
}
362365
logger_.debug("Parsing setup request:\n{}", request);
363366
// parse the transport header from the request
364367
auto transport_index = request.find("Transport: ");
365368
if (transport_index == std::string::npos) {
369+
logger_.error("Failed to parse Transport header (start) from request");
370+
handle_rtsp_invalid_request(request);
366371
return false;
367372
}
368373
auto transport_end_index = request.find('\r', transport_index);
369374
if (transport_end_index == std::string::npos) {
375+
logger_.error("Failed to parse Transport header (end) from request");
376+
handle_rtsp_invalid_request(request);
370377
return false;
371378
}
372379
std::string_view transport =
373380
request.substr(transport_index + 11, transport_end_index - transport_index - 11);
374381
if (transport.empty()) {
382+
logger_.error("Transport header is empty");
383+
handle_rtsp_invalid_request(request);
375384
return false;
376385
}
377386
logger_.debug("Transport header: {}", transport);
378387
// we don't support TCP, so return an error if the transport is not RTP/AVP/UDP
379388
if (transport.find("RTP/AVP/TCP") != std::string::npos) {
380389
logger_.error("TCP transport is not supported");
381-
// TODO: this doesn't send the sequence number back to the client
382-
send_response(461, "Unsupported Transport");
390+
handle_rtsp_invalid_request(request, 461, "Unsupported Transport");
383391
return false;
384392
}
385393

@@ -389,12 +397,16 @@ bool RtspSession::parse_rtsp_setup_request(std::string_view request, std::string
389397
std::string_view rtp_port =
390398
request.substr(client_port_index + 12, dash_index - client_port_index - 12);
391399
if (rtp_port.empty()) {
400+
logger_.error("Failed to parse client RTP port from request");
401+
handle_rtsp_invalid_request(request);
392402
return false;
393403
}
394404
// parse the rtcp port from the request
395405
std::string_view rtcp_port =
396406
request.substr(dash_index + 1, request.find('\r', client_port_index) - dash_index - 1);
397407
if (rtcp_port.empty()) {
408+
logger_.error("Empty client RTCP port in request");
409+
handle_rtsp_invalid_request(request);
398410
return false;
399411
}
400412
// convert the rtp and rtcp ports to integers

0 commit comments

Comments
 (0)