From 16b42fbd62d930f8a38283c5086fe7ac026e80e6 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Thu, 28 Aug 2014 15:46:17 -0400 Subject: [PATCH] Switch around internal fromtag/totag/endpoint representation monologue_offer_answer() had A-side and B-side reversed, resulting in incomplete dialogue association when more than 2 parties are involved. It also fixes #21 by catching errors returned by monologue_offer_answer() (e.g. when running out of ports) --- daemon/call.c | 22 +++++++++++++++------- daemon/call_interfaces.c | 36 ++++++++++++++++++++---------------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/daemon/call.c b/daemon/call.c index 089ace3a7..da290c4d9 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -1957,23 +1957,26 @@ static void __tos_change(struct call *call, const struct sdp_ng_flags *flags) { } /* called with call->master_lock held in W */ -int monologue_offer_answer(struct call_monologue *monologue, GQueue *streams, +int monologue_offer_answer(struct call_monologue *other_ml, GQueue *streams, const struct sdp_ng_flags *flags) { struct stream_params *sp; GList *media_iter, *ml_media, *other_ml_media; struct call_media *media, *other_media; unsigned int num_ports; - struct call_monologue *other_ml = monologue->active_dialogue; + struct call_monologue *monologue = other_ml->active_dialogue; struct endpoint_map *em; monologue->call->last_signal = poller_now; monologue->call->deleted = 0; - /* we must have a complete dialogue, even though the to-tag (other_ml->tag) + /* we must have a complete dialogue, even though the to-tag (monologue->tag) * may not be known yet */ - if (!other_ml) + if (!other_ml) { + ilog(LOG_ERROR, "Incomplete dialogue association"); return -1; + } + __C_DBG("this="STR_FORMAT" other="STR_FORMAT, STR_FMT(&monologue->tag), STR_FMT(&other_ml->tag)); __tos_change(monologue->call, flags); @@ -1987,9 +1990,12 @@ int monologue_offer_answer(struct call_monologue *monologue, GQueue *streams, * the dialogue */ media = __get_media(monologue, &ml_media, sp); other_media = __get_media(other_ml, &other_ml_media, sp); - /* THIS side corresponds to what's being sent to the recipient of the - * offer/answer. The OTHER side corresponds to what WILL BE sent to the - * offerer or WAS sent to the answerer. */ + /* OTHER is the side which has sent the message. SDP parameters in + * "sp" are as advertised by OTHER side. The message will be sent to + * THIS side. Parameters sent to THIS side may be overridden by + * what's in "flags". If this is an answer, or if we have talked to + * THIS side (recipient) before, then the structs will be populated with + * details already. */ /* deduct protocol from stream parameters received */ if (other_media->protocol != sp->protocol) { @@ -2014,11 +2020,13 @@ int monologue_offer_answer(struct call_monologue *monologue, GQueue *streams, if (other_media->sdes_in.params.crypto_suite) MEDIA_SET(other_media, SDES); + /* send and recv are from our POV */ bf_copy_same(&media->media_flags, &sp->sp_flags, SP_FLAG_SEND | SP_FLAG_RECV); bf_copy(&other_media->media_flags, MEDIA_FLAG_RECV, &sp->sp_flags, SP_FLAG_SEND); bf_copy(&other_media->media_flags, MEDIA_FLAG_SEND, &sp->sp_flags, SP_FLAG_RECV); + /* active and passive are also from our POV */ bf_copy(&other_media->media_flags, MEDIA_FLAG_SETUP_PASSIVE, &sp->sp_flags, SP_FLAG_SETUP_ACTIVE); bf_copy(&other_media->media_flags, MEDIA_FLAG_SETUP_ACTIVE, diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index 733ecf819..8f04bff45 100644 --- a/daemon/call_interfaces.c +++ b/daemon/call_interfaces.c @@ -126,6 +126,7 @@ static str *call_update_lookup_udp(char **out, struct callmaster *m, enum call_o GQueue q = G_QUEUE_INIT; struct stream_params sp; str *ret, callid, viabranch, fromtag, totag = STR_NULL; + int i; str_init(&callid, out[RE_UDP_UL_CALLID]); str_init(&viabranch, out[RE_UDP_UL_VIABRANCH]); @@ -147,11 +148,14 @@ static str *call_update_lookup_udp(char **out, struct callmaster *m, enum call_o goto addr_fail; g_queue_push_tail(&q, &sp); - /* XXX return value */ - monologue_offer_answer(monologue, &q, NULL); + i = monologue_offer_answer(monologue, &q, NULL); g_queue_clear(&q); - ret = streams_print(&monologue->medias, sp.index, sp.index, out[RE_UDP_COOKIE], SAF_UDP); + if (i) + goto unlock_fail; + + ret = streams_print(&monologue->active_dialogue->medias, + sp.index, sp.index, out[RE_UDP_COOKIE], SAF_UDP); rwlock_unlock_w(&c->master_lock); redis_update(c, m->conf.redis); @@ -160,16 +164,16 @@ static str *call_update_lookup_udp(char **out, struct callmaster *m, enum call_o goto out; ml_fail: - rwlock_unlock_w(&c->master_lock); - ilog(LOG_WARNING, "Invalid dialogue association"); - goto fail_out; + ilog(LOG_ERR, "Invalid dialogue association"); + goto unlock_fail; addr_fail: - rwlock_unlock_w(&c->master_lock); - ilog(LOG_WARNING, "Failed to parse a media stream: %s/%s:%s", out[RE_UDP_UL_ADDR4], out[RE_UDP_UL_ADDR6], out[RE_UDP_UL_PORT]); - goto fail_out; + ilog(LOG_ERR, "Failed to parse a media stream: %s/%s:%s", + out[RE_UDP_UL_ADDR4], out[RE_UDP_UL_ADDR6], out[RE_UDP_UL_PORT]); + goto unlock_fail; -fail_out: +unlock_fail: + rwlock_unlock_w(&c->master_lock); ret = str_sprintf("%s E8\n", out[RE_UDP_COOKIE]); out: obj_put(c); @@ -283,10 +287,10 @@ static str *call_request_lookup_tcp(char **out, struct callmaster *m, enum call_ ilog(LOG_WARNING, "Invalid dialogue association"); goto out2; } - /* XXX return value */ - monologue_offer_answer(monologue, &s, NULL); + if (monologue_offer_answer(monologue, &s, NULL)) + goto out2; - ret = streams_print(&monologue->medias, 1, s.length, NULL, SAF_TCP); + ret = streams_print(&monologue->active_dialogue->medias, 1, s.length, NULL, SAF_TCP); out2: rwlock_unlock_w(&c->master_lock); @@ -598,9 +602,9 @@ static const char *call_offer_answer_ng(bencode_item_t *input, struct callmaster chopper = sdp_chopper_new(&sdp); bencode_buffer_destroy_add(output->buffer, (free_func_t) sdp_chopper_destroy, chopper); - /* XXX return value */ - monologue_offer_answer(monologue, &streams, &flags); - ret = sdp_replace(chopper, &parsed, monologue, &flags); + ret = monologue_offer_answer(monologue, &streams, &flags); + if (!ret) + ret = sdp_replace(chopper, &parsed, monologue->active_dialogue, &flags); rwlock_unlock_w(&call->master_lock); redis_update(call, m->conf.redis);