[Asterisk-code-review] chan_sip: Always process updated SDP on media source change (asterisk[master])
Friendly Automation
asteriskteam at digium.com
Mon Jan 27 18:29:45 CST 2020
Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13692 )
Change subject: chan_sip: Always process updated SDP on media source change
......................................................................
chan_sip: Always process updated SDP on media source change
Fixes no-audio issues when the media source is changed and
strictrtp is enabled (default).
If the peer media source changes, the SDP session version also changes.
If it is lower than the one we had stored, chan_sip would ignore it.
This changeset keeps track of the remote media origin identifier,
comparing that as well. If it changes, the session version needn't be
higher for us to accept the SDP.
Common scenario where this would've caused problems: a separate media
gateway that informs the caller about premium rates before handing off
the call to the final destination.
(An alternative fix would be to set ignoresdpversion=yes on the peer.)
ASTERISK-28686
Change-Id: I88fdbc5aeb777b583e7738c084254c482a7776ee
---
M channels/chan_sip.c
M channels/sip/include/sip.h
2 files changed, 47 insertions(+), 18 deletions(-)
Approvals:
Joshua Colp: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved
Friendly Automation: Approved for Submit
diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index e3b8afd..717e62d 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -11249,9 +11249,12 @@
static int process_sdp_o(const char *o, struct sip_pvt *p)
{
+ const char *o_copy_start;
char *o_copy;
char *token;
- int64_t rua_version;
+ int offset;
+ int64_t sess_version;
+ char unique[128];
/* Store the SDP version number of remote UA. This will allow us to
distinguish between session modifications and session refreshes. If
@@ -11269,35 +11272,49 @@
return FALSE;
}
- o_copy = ast_strdupa(o);
- token = strsep(&o_copy, " "); /* Skip username */
+ /* o=<username> <sess-id> <sess-version> <nettype> <addrtype>
+ <unicast-address> */
+
+ o_copy_start = o_copy = ast_strdupa(o);
+ token = strsep(&o_copy, " "); /* Skip username */
if (!o_copy) {
ast_log(LOG_WARNING, "SDP syntax error in o= line username\n");
return FALSE;
}
- token = strsep(&o_copy, " "); /* Skip session-id */
+ token = strsep(&o_copy, " "); /* sess-id */
if (!o_copy) {
- ast_log(LOG_WARNING, "SDP syntax error in o= line session-id\n");
+ ast_log(LOG_WARNING, "SDP syntax error in o= line sess-id\n");
return FALSE;
}
- token = strsep(&o_copy, " "); /* Version */
- if (!o_copy) {
- ast_log(LOG_WARNING, "SDP syntax error in o= line\n");
- return FALSE;
- }
- if (!sscanf(token, "%30" SCNd64, &rua_version)) {
- ast_log(LOG_WARNING, "SDP syntax error in o= line version\n");
+ token = strsep(&o_copy, " "); /* sess-version */
+ if (!o_copy || !sscanf(token, "%30" SCNd64, &sess_version)) {
+ ast_log(LOG_WARNING, "SDP syntax error in o= line sess-version\n");
return FALSE;
}
- /* we need to check the SDP version number the other end sent us;
+ /* Copy all after sess-version on top of sess-version into unique.
+ * <sess-id> is a numeric string such that the tuple of <username>,
+ * <sess-id>, <nettype>, <addrtype>, and <unicast-address> forms a
+ * globally unique identifier for the session.
+ * I.e. all except the <sess-version> */
+ ast_copy_string(unique, o, sizeof(unique)); /* copy all of o= contents */
+ offset = (o_copy - o_copy_start); /* after sess-version */
+ if (offset < sizeof(unique)) {
+ /* copy all after sess-version on top of sess-version */
+ int sess_version_start = token - o_copy_start;
+ ast_copy_string(unique + sess_version_start, o + offset, sizeof(unique) - sess_version_start);
+ }
+
+ /* We need to check the SDP version number the other end sent us;
* our rules for deciding what to accept are a bit complex.
*
* 1) if 'ignoresdpversion' has been set for this dialog, then
* we will just accept whatever they sent and assume it is
* a modification of the session, even if it is not
* 2) otherwise, if this is the first SDP we've seen from them
- * we accept it
+ * we accept it;
+ * note that _them_ may change, in which case the
+ * sessionunique_remote will be different
* 3) otherwise, if the new SDP version number is higher than the
* old one, we accept it
* 4) otherwise, if this SDP is in response to us requesting a switch
@@ -11307,14 +11324,25 @@
* not request a switch to T.38, then we stop parsing the SDP, as it
* has not changed from the previous version
*/
+ if (sip_debug_test_pvt(p)) {
+ if (ast_strlen_zero(p->sessionunique_remote)) {
+ ast_verbose("Got SDP version %" PRId64 " and unique parts [%s]\n",
+ sess_version, unique);
+ } else {
+ ast_verbose("Comparing SDP version %" PRId64 " -> %" PRId64 " and unique parts [%s] -> [%s]\n",
+ p->sessionversion_remote, sess_version, p->sessionunique_remote, unique);
+ }
+ }
if (ast_test_flag(&p->flags[1], SIP_PAGE2_IGNORESDPVERSION) ||
- (p->sessionversion_remote < 0) ||
- (p->sessionversion_remote < rua_version)) {
- p->sessionversion_remote = rua_version;
+ sess_version > p->sessionversion_remote ||
+ strcmp(unique, S_OR(p->sessionunique_remote, ""))) {
+ p->sessionversion_remote = sess_version;
+ ast_string_field_set(p, sessionunique_remote, unique);
} else {
if (p->t38.state == T38_LOCAL_REINVITE) {
- p->sessionversion_remote = rua_version;
+ p->sessionversion_remote = sess_version;
+ ast_string_field_set(p, sessionunique_remote, unique);
ast_log(LOG_WARNING, "Call %s responded to our T.38 reinvite without changing SDP version; 'ignoresdpversion' should be set for this peer.\n", p->callid);
} else {
p->session_modify = FALSE;
diff --git a/channels/sip/include/sip.h b/channels/sip/include/sip.h
index 4ee2041..ca26fa3 100644
--- a/channels/sip/include/sip.h
+++ b/channels/sip/include/sip.h
@@ -1052,6 +1052,7 @@
AST_STRING_FIELD(last_presence_message); /*!< The last presence message for a subscription */
AST_STRING_FIELD(msg_body); /*!< Text for a MESSAGE body */
AST_STRING_FIELD(tel_phone_context); /*!< The phone-context portion of a TEL URI */
+ AST_STRING_FIELD(sessionunique_remote); /*!< Remote UA's SDP Session unique parts */
);
char via[128]; /*!< Via: header */
int maxforwards; /*!< SIP Loop prevention */
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13692
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I88fdbc5aeb777b583e7738c084254c482a7776ee
Gerrit-Change-Number: 13692
Gerrit-PatchSet: 1
Gerrit-Owner: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200127/0016d35e/attachment.html>
More information about the asterisk-code-review
mailing list