[Asterisk-code-review] res/res pjsip session: Only check localnet if it is defined (asterisk[master])

[Joshua Colp]

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

Change subject: res/res_pjsip_session: Only check localnet if it is defined
......................................................................


res/res_pjsip_session: Only check localnet if it is defined

If local_net is not defined on a transport, transport_state->localnet
will be NULL. ast_apply_ha will, be default, return AST_SENSE_ALLOW in
this case, causing the external_media_address, if set, to be skipped.

This patch causes us to only check if we are sending within a network if
local_net is defined.

ASTERISK-26879 #close

Change-Id: Ib661c31a954cabc9c99f1f25c9c9a5c5b82cbbfb
---
M res/res_pjsip_nat.c
M res/res_pjsip_sdp_rtp.c
M res/res_pjsip_session.c
M res/res_pjsip_t38.c
4 files changed, 33 insertions(+), 27 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/res/res_pjsip_nat.c b/res/res_pjsip_nat.c
index a855298..a26180b 100644
--- a/res/res_pjsip_nat.c
+++ b/res/res_pjsip_nat.c
@@ -262,32 +262,33 @@
 		return PJ_SUCCESS;
 	}
 
-	if ( !transport_state->localnet || 	ast_sockaddr_isnull(&transport_state->external_address)) {
-		return PJ_SUCCESS;
-	}
+	if (transport_state->localnet) {
+		ast_sockaddr_parse(&addr, tdata->tp_info.dst_name, PARSE_PORT_FORBID);
+		ast_sockaddr_set_port(&addr, tdata->tp_info.dst_port);
 
-	ast_sockaddr_parse(&addr, tdata->tp_info.dst_name, PARSE_PORT_FORBID);
-	ast_sockaddr_set_port(&addr, tdata->tp_info.dst_port);
-
-	/* See if where we are sending this request is local or not, and if not that we can get a Contact URI to modify */
-	if (ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {
-		return PJ_SUCCESS;
-	}
-
-	/* Update the contact header with the external address */
-	if (uri || (uri = nat_get_contact_sip_uri(tdata))) {
-		pj_strdup2(tdata->pool, &uri->host, ast_sockaddr_stringify_host(&transport_state->external_address));
-		if (transport->external_signaling_port) {
-			uri->port = transport->external_signaling_port;
-			ast_debug(4, "Re-wrote Contact URI port to %d\n", uri->port);
+		/* See if where we are sending this request is local or not, and if not that we can get a Contact URI to modify */
+		if (ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {
+			ast_debug(5, "Request is being sent to local address, skipping NAT manipulation\n");
+			return PJ_SUCCESS;
 		}
 	}
 
-	/* Update the via header if relevant */
-	if ((tdata->msg->type == PJSIP_REQUEST_MSG) && (via || (via = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_VIA, NULL)))) {
-		pj_strdup2(tdata->pool, &via->sent_by.host, ast_sockaddr_stringify_host(&transport_state->external_address));
-		if (transport->external_signaling_port) {
-			via->sent_by.port = transport->external_signaling_port;
+	if (!ast_sockaddr_isnull(&transport_state->external_address)) {
+		/* Update the contact header with the external address */
+		if (uri || (uri = nat_get_contact_sip_uri(tdata))) {
+			pj_strdup2(tdata->pool, &uri->host, ast_sockaddr_stringify_host(&transport_state->external_address));
+			if (transport->external_signaling_port) {
+				uri->port = transport->external_signaling_port;
+				ast_debug(4, "Re-wrote Contact URI port to %d\n", uri->port);
+			}
+		}
+
+		/* Update the via header if relevant */
+		if ((tdata->msg->type == PJSIP_REQUEST_MSG) && (via || (via = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_VIA, NULL)))) {
+			pj_strdup2(tdata->pool, &via->sent_by.host, ast_sockaddr_stringify_host(&transport_state->external_address));
+			if (transport->external_signaling_port) {
+				via->sent_by.port = transport->external_signaling_port;
+			}
 		}
 	}
 
diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index 9f39256..75dc839 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -1473,10 +1473,11 @@
 	ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);
 
 	/* Is the address within the SDP inside the same network? */
-	if (ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {
+	if (transport_state->localnet
+		&& ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {
 		return;
 	}
-
+	ast_debug(5, "Setting media address to %s\n", transport->external_media_address);
 	pj_strdup2(tdata->pool, &stream->conn->addr, transport->external_media_address);
 }
 
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index ad9670c..609d08d 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -3148,7 +3148,10 @@
 		ast_copy_pj_str(host, &sdp->conn->addr, sizeof(host));
 		ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);
 
-		if (ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {
+		if (!transport_state->localnet
+			|| (transport_state->localnet
+				&& ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW)) {
+			ast_debug(5, "Setting external media address to %s\n", transport->external_media_address);
 			pj_strdup2(tdata->pool, &sdp->conn->addr, transport->external_media_address);
 		}
 	}
diff --git a/res/res_pjsip_t38.c b/res/res_pjsip_t38.c
index 79dc9c3..bae4ff1 100644
--- a/res/res_pjsip_t38.c
+++ b/res/res_pjsip_t38.c
@@ -867,10 +867,11 @@
 	ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);
 
 	/* Is the address within the SDP inside the same network? */
-	if (ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {
+	if (transport_state->localnet
+		&& ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {
 		return;
 	}
-
+	ast_debug(5, "Setting media address to %s\n", transport->external_media_address);
 	pj_strdup2(tdata->pool, &stream->conn->addr, transport->external_media_address);
 }
 

-- 
To view, visit https://gerrit.asterisk.org/5231
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib661c31a954cabc9c99f1f25c9c9a5c5b82cbbfb
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>