From 971548405bd82746229cad4958b10707e759816e Mon Sep 17 00:00:00 2001 From: George Joseph Date: Wed, 20 Sep 2017 09:45:16 -0600 Subject: [PATCH] res_pjsip_session: Change some asserts to warning/debug messages There was an issue reported where an SDP received on a 183 Session Progress message caused a crash because the pending streams had already been processed when the OK was received. In that case the pending topology was legitimately NULL. There was an assert for an incorrect number of streams in the topology but not one for topology being NULL. In any case, if you're not in dev-mode the asserts don't do anything and since the scenario is legit, the asserts weren't appropriate anyway. * Changed several asserts to warning or debug messages and return codes as appropriate. ASTERISK-27264 Reported by: Daniel Heckl Change-Id: I58daaa9d2938fa980857ab3ec41925ab5ff9c848 --- res/res_pjsip_session.c | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c index 64416a0636..978c95a0c4 100644 --- a/res/res_pjsip_session.c +++ b/res/res_pjsip_session.c @@ -277,7 +277,11 @@ int ast_sip_session_is_pending_stream_default(const struct ast_sip_session *sess { int index; - ast_assert(session->pending_media_state->topology != NULL); + if (!session->pending_media_state->topology) { + ast_log(LOG_WARNING, "Pending topology was NULL for channel '%s'\n", + session->channel ? ast_channel_name(session->channel) : "unknown"); + return 0; + } if (ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) { return 0; @@ -766,6 +770,30 @@ static int handle_negotiated_sdp(struct ast_sip_session *session, const pjmedia_ int i; struct ast_stream_topology *topology; + /* This situation can legitimately happen when an SDP is received in a + * 183 Session Progress message. In that case, everything's been done + * by the time this function is called and there are no more pending + * streams. + */ + if (!session->pending_media_state->topology) { + ast_debug(1, "Pending topology was NULL for channel '%s'\n", + session->channel ? ast_channel_name(session->channel) : "unknown"); + return 0; + } + + /* If we're handling negotiated streams, then we should already have set + * up session media instances (and Asterisk streams) that correspond to + * the local SDP, and there should be the same number of session medias + * and streams as there are local SDP streams + */ + if (ast_stream_topology_get_count(session->pending_media_state->topology) != local->media_count + || AST_VECTOR_SIZE(&session->pending_media_state->sessions) != local->media_count) { + ast_log(LOG_WARNING, "Local SDP for channel '%s' contains %d media streams while we expected it to contain %u\n", + session->channel ? ast_channel_name(session->channel) : "unknown", + ast_stream_topology_get_count(session->pending_media_state->topology), local->media_count); + return -1; + } + for (i = 0; i < local->media_count; ++i) { struct ast_sip_session_media *session_media; struct ast_stream *stream; @@ -774,14 +802,6 @@ static int handle_negotiated_sdp(struct ast_sip_session *session, const pjmedia_ continue; } - /* If we're handling negotiated streams, then we should already have set - * up session media instances (and Asterisk streams) that correspond to - * the local SDP, and there should be the same number of session medias - * and streams as there are local SDP streams - */ - ast_assert(i < AST_VECTOR_SIZE(&session->pending_media_state->sessions)); - ast_assert(i < ast_stream_topology_get_count(session->pending_media_state->topology)); - session_media = AST_VECTOR_GET(&session->pending_media_state->sessions, i); stream = ast_stream_topology_get_stream(session->pending_media_state->topology, i); @@ -816,9 +836,6 @@ static int handle_negotiated_sdp(struct ast_sip_session *session, const pjmedia_ continue; } - ast_assert(i < AST_VECTOR_SIZE(&session->pending_media_state->sessions)); - ast_assert(i < ast_stream_topology_get_count(session->pending_media_state->topology)); - session_media = AST_VECTOR_GET(&session->pending_media_state->sessions, i); stream = ast_stream_topology_get_stream(session->pending_media_state->topology, i);