[Asterisk-code-review] res pjsip t38: Decline T.38 stream on failure case. (asterisk[15])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/9353 )

Change subject: res_pjsip_t38: Decline T.38 stream on failure case.
......................................................................

res_pjsip_t38: Decline T.38 stream on failure case.

When negotiating an incoming T.38 stream the code incorrectly
returned failure instead of a decline for the stream when a
problem occurred or the configuration didn't allow it. This
resulted in SDP offers being rejected with a 488 response
in all cases, even when another valid stream was present.

This change makes it so the stream is now declined. If no
streams are accepted a 488 response is sent while if at least
one stream is accepted all the declined streams are, well,
declined.

ASTERISK-27763

Change-Id: I88bcf793788c412a9839d111a5c736bf6867807c
---
M res/res_pjsip_t38.c
1 file changed, 5 insertions(+), 5 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Approved for Submit



diff --git a/res/res_pjsip_t38.c b/res/res_pjsip_t38.c
index 8091bb3..24d1a0e 100644
--- a/res/res_pjsip_t38.c
+++ b/res/res_pjsip_t38.c
@@ -749,17 +749,17 @@
 
 	if (!session->endpoint->media.t38.enabled) {
 		ast_debug(3, "Declining; T.38 not enabled on session\n");
-		return -1;
+		return 0;
 	}
 
 	if (!(state = t38_state_get_or_alloc(session))) {
-		return -1;
+		return 0;
 	}
 
 	if ((session->t38state == T38_REJECTED) || (session->t38state == T38_DISABLED)) {
 		ast_debug(3, "Declining; T.38 state is rejected or declined\n");
 		t38_change_state(session, session_media, state, T38_DISABLED);
-		return -1;
+		return 0;
 	}
 
 	ast_copy_pj_str(host, stream->conn ? &stream->conn->addr : &sdp->conn->addr, sizeof(host));
@@ -768,7 +768,7 @@
 	if (ast_sockaddr_resolve(&addrs, host, PARSE_PORT_FORBID, AST_AF_INET) <= 0) {
 		/* The provided host was actually invalid so we error out this negotiation */
 		ast_debug(3, "Declining; provided host is invalid\n");
-		return -1;
+		return 0;
 	}
 
 	/* Check the address family to make sure it matches configured */
@@ -776,7 +776,7 @@
 		(ast_sockaddr_is_ipv4(addrs) && session->endpoint->media.t38.ipv6)) {
 		/* The address does not match configured */
 		ast_debug(3, "Declining, provided host does not match configured address family\n");
-		return -1;
+		return 0;
 	}
 
 	return 1;

-- 
To view, visit https://gerrit.asterisk.org/9353
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: I88bcf793788c412a9839d111a5c736bf6867807c
Gerrit-Change-Number: 9353
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180709/73749094/attachment.html>