[Asterisk-code-review] res pjsip: Log IPv6 addresses correctly (asterisk[13])

George Joseph asteriskteam at digium.com
Mon Sep 17 08:33:41 CDT 2018


George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/10058 )

Change subject: res_pjsip: Log IPv6 addresses correctly
......................................................................

res_pjsip: Log IPv6 addresses correctly

Both pjsip_tx_data.tp_info.dst_name and pjsip_rx_data.pkt_info.src_name
store IPv6 addresses without enclosing brackets. This causes some log
output to be confusing because it is difficult to separate the IPv6
address from a port specification.

* Use pj_sockaddr_print() along with pjsip_tx_data.tp_info.dst_addr and
  pjsip_rx_data.pkt_info.src_addr where possible for consistent IPv6
  output.

* When a pj_sockaddr is not available, explicitly wrap IPv6 addresses
  in brackets.

* When assigning pjsip_rx_data.pkt_info.src_name ourselves, make sure
  to also set pjsip_rx_data.pkt_info.src_addr.

Change-Id: I5cfe997ced7883862a12b9c7d8551d76ae02fcf8
---
M include/asterisk/netsock2.h
M res/res_pjsip.c
M res/res_pjsip/pjsip_distributor.c
M res/res_pjsip_logger.c
M res/res_pjsip_outbound_authenticator_digest.c
M res/res_pjsip_session.c
M res/res_pjsip_transport_websocket.c
7 files changed, 62 insertions(+), 23 deletions(-)

Approvals:
  Michael L. Young: Looks good to me, but someone else must approve
  Richard Mudgett: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved
  George Joseph: Approved for Submit



diff --git a/include/asterisk/netsock2.h b/include/asterisk/netsock2.h
index 5c6c733..b6f2858 100644
--- a/include/asterisk/netsock2.h
+++ b/include/asterisk/netsock2.h
@@ -31,6 +31,18 @@
 
 #include <netinet/in.h>
 
+/*
+ * String buffer size that can accommodate a fully stringified representation of a
+ * supported IP address & port:
+ *
+ * - 45 bytes for an IPv6 address
+ * -  2 bytes for brackets around an IPv6 address
+ * -  1 byte for the port separator (a colon)
+ * -  5 bytes for the port
+ * -  1 byte for the zero-terminator
+ */
+#define AST_SOCKADDR_BUFLEN (45 + 2 + 1 + 5 + 1)
+
 /*!
  * Values for address families that we support. This is reproduced from socket.h
  * because we do not want users to include that file. Only netsock2.c should
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index ece03d3..93fa6a3 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -3450,6 +3450,8 @@
 	ast_copy_string(rdata->pkt_info.packet, packet, sizeof(rdata->pkt_info.packet));
 	ast_copy_string(rdata->pkt_info.src_name, src_name, sizeof(rdata->pkt_info.src_name));
 	rdata->pkt_info.src_port = src_port;
+	pj_sockaddr_parse(pj_AF_UNSPEC(), 0, pj_cstr(&tmp, src_name), &rdata->pkt_info.src_addr);
+	pj_sockaddr_set_port(&rdata->pkt_info.src_addr, src_port);
 
 	pjsip_parse_rdata(packet, strlen(packet), rdata);
 	if (!rdata->msg_info.msg || !pj_list_empty(&rdata->msg_info.parse_err)) {
diff --git a/res/res_pjsip/pjsip_distributor.c b/res/res_pjsip/pjsip_distributor.c
index f9e2997..9b3fcdc 100644
--- a/res/res_pjsip/pjsip_distributor.c
+++ b/res/res_pjsip/pjsip_distributor.c
@@ -638,16 +638,21 @@
 	char from_buf[PJSIP_MAX_URL_SIZE];
 	char callid_buf[PJSIP_MAX_URL_SIZE];
 	char method_buf[PJSIP_MAX_URL_SIZE];
+	char src_addr_buf[AST_SOCKADDR_BUFLEN];
 	pjsip_uri_print(PJSIP_URI_IN_FROMTO_HDR, rdata->msg_info.from->uri, from_buf, PJSIP_MAX_URL_SIZE);
 	ast_copy_pj_str(callid_buf, &rdata->msg_info.cid->id, PJSIP_MAX_URL_SIZE);
 	ast_copy_pj_str(method_buf, &rdata->msg_info.msg->line.req.method.name, PJSIP_MAX_URL_SIZE);
 	if (count) {
-		ast_log(LOG_NOTICE, "Request '%s' from '%s' failed for '%s:%d' (callid: %s) - %s"
+		ast_log(LOG_NOTICE, "Request '%s' from '%s' failed for '%s' (callid: %s) - %s"
 			" after %u tries in %.3f ms\n",
-			method_buf, from_buf, rdata->pkt_info.src_name, rdata->pkt_info.src_port, callid_buf, msg, count, period / 1000.0);
+			method_buf, from_buf,
+			pj_sockaddr_print(&rdata->pkt_info.src_addr, src_addr_buf, sizeof(src_addr_buf), 3),
+			callid_buf, msg, count, period / 1000.0);
 	} else {
-		ast_log(LOG_NOTICE, "Request '%s' from '%s' failed for '%s:%d' (callid: %s) - %s\n",
-			method_buf, from_buf, rdata->pkt_info.src_name, rdata->pkt_info.src_port, callid_buf, msg);
+		ast_log(LOG_NOTICE, "Request '%s' from '%s' failed for '%s' (callid: %s) - %s\n",
+			method_buf, from_buf,
+			pj_sockaddr_print(&rdata->pkt_info.src_addr, src_addr_buf, sizeof(src_addr_buf), 3),
+			callid_buf, msg);
 	}
 }
 
diff --git a/res/res_pjsip_logger.c b/res/res_pjsip_logger.c
index 2e5c76c..fe23392 100644
--- a/res/res_pjsip_logger.c
+++ b/res/res_pjsip_logger.c
@@ -101,22 +101,25 @@
 
 static pj_status_t logging_on_tx_msg(pjsip_tx_data *tdata)
 {
+	char buffer[AST_SOCKADDR_BUFLEN];
+
 	if (!pjsip_log_test_addr(tdata->tp_info.dst_name, tdata->tp_info.dst_port)) {
 		return PJ_SUCCESS;
 	}
 
-	ast_verbose("<--- Transmitting SIP %s (%d bytes) to %s:%s:%d --->\n%.*s\n",
+	ast_verbose("<--- Transmitting SIP %s (%d bytes) to %s:%s --->\n%.*s\n",
 		    tdata->msg->type == PJSIP_REQUEST_MSG ? "request" : "response",
 		    (int) (tdata->buf.cur - tdata->buf.start),
 		    tdata->tp_info.transport->type_name,
-		    tdata->tp_info.dst_name,
-		    tdata->tp_info.dst_port,
+		    pj_sockaddr_print(&tdata->tp_info.dst_addr, buffer, sizeof(buffer), 3),
 		    (int) (tdata->buf.end - tdata->buf.start), tdata->buf.start);
 	return PJ_SUCCESS;
 }
 
 static pj_bool_t logging_on_rx_msg(pjsip_rx_data *rdata)
 {
+	char buffer[AST_SOCKADDR_BUFLEN];
+
 	if (!pjsip_log_test_addr(rdata->pkt_info.src_name, rdata->pkt_info.src_port)) {
 		return PJ_FALSE;
 	}
@@ -125,12 +128,11 @@
 		return PJ_FALSE;
 	}
 
-	ast_verbose("<--- Received SIP %s (%d bytes) from %s:%s:%d --->\n%s\n",
+	ast_verbose("<--- Received SIP %s (%d bytes) from %s:%s --->\n%s\n",
 		    rdata->msg_info.msg->type == PJSIP_REQUEST_MSG ? "request" : "response",
 		    rdata->msg_info.len,
 		    rdata->tp_info.transport->type_name,
-		    rdata->pkt_info.src_name,
-		    rdata->pkt_info.src_port,
+		    pj_sockaddr_print(&rdata->pkt_info.src_addr, buffer, sizeof(buffer), 3),
 		    rdata->pkt_info.packet);
 	return PJ_FALSE;
 }
diff --git a/res/res_pjsip_outbound_authenticator_digest.c b/res/res_pjsip_outbound_authenticator_digest.c
index 9440a87..a94e0a2 100644
--- a/res/res_pjsip_outbound_authenticator_digest.c
+++ b/res/res_pjsip_outbound_authenticator_digest.c
@@ -118,8 +118,8 @@
 	}
 	/* If there was no dialog, then this is probably a REGISTER so no endpoint */
 	if (!id) {
-		id = ast_alloca(strlen(challenge->pkt_info.src_name) + 7 /* ':' + port + NULL */);
-		sprintf(id, "%s:%d", challenge->pkt_info.src_name, challenge->pkt_info.src_port);
+		id = ast_alloca(AST_SOCKADDR_BUFLEN);
+		pj_sockaddr_print(&challenge->pkt_info.src_addr, id, AST_SOCKADDR_BUFLEN, 3);
 		id_type = "Host";
 	}
 
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index ed637cd..ff40e55 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -2101,6 +2101,7 @@
 	pjsip_timer_setting timer;
 	pjsip_rdata_sdp_info *sdp_info;
 	pjmedia_sdp_session *local = NULL;
+	char buffer[AST_SOCKADDR_BUFLEN];
 
 	/* From this point on, any calls to pjsip_inv_terminate have the last argument as PJ_TRUE
 	 * so that we will be notified so we can destroy the session properly
@@ -2128,8 +2129,11 @@
 		}
 		goto end;
 	case SIP_GET_DEST_EXTEN_PARTIAL:
-		ast_debug(1, "Call from '%s' (%s:%s:%d) to extension '%s' - partial match\n", ast_sorcery_object_get_id(invite->session->endpoint),
-			invite->rdata->tp_info.transport->type_name, invite->rdata->pkt_info.src_name, invite->rdata->pkt_info.src_port, invite->session->exten);
+		ast_debug(1, "Call from '%s' (%s:%s) to extension '%s' - partial match\n",
+			ast_sorcery_object_get_id(invite->session->endpoint),
+			invite->rdata->tp_info.transport->type_name,
+			pj_sockaddr_print(&invite->rdata->pkt_info.src_addr, buffer, sizeof(buffer), 3),
+			invite->session->exten);
 
 		if (pjsip_inv_initial_answer(invite->session->inv_session, invite->rdata, 484, NULL, NULL, &tdata) == PJ_SUCCESS) {
 			ast_sip_session_send_response(invite->session, tdata);
@@ -2139,9 +2143,12 @@
 		goto end;
 	case SIP_GET_DEST_EXTEN_NOT_FOUND:
 	default:
-		ast_log(LOG_NOTICE, "Call from '%s' (%s:%s:%d) to extension '%s' rejected because extension not found in context '%s'.\n",
-			ast_sorcery_object_get_id(invite->session->endpoint), invite->rdata->tp_info.transport->type_name, invite->rdata->pkt_info.src_name,
-			invite->rdata->pkt_info.src_port, invite->session->exten, invite->session->endpoint->context);
+		ast_log(LOG_NOTICE, "Call from '%s' (%s:%s) to extension '%s' rejected because extension not found in context '%s'.\n",
+			ast_sorcery_object_get_id(invite->session->endpoint),
+			invite->rdata->tp_info.transport->type_name,
+			pj_sockaddr_print(&invite->rdata->pkt_info.src_addr, buffer, sizeof(buffer), 3),
+			invite->session->exten,
+			invite->session->endpoint->context);
 
 		if (pjsip_inv_initial_answer(invite->session->inv_session, invite->rdata, 404, NULL, NULL, &tdata) == PJ_SUCCESS) {
 			ast_sip_session_send_response(invite->session, tdata);
diff --git a/res/res_pjsip_transport_websocket.c b/res/res_pjsip_transport_websocket.c
index d71575d..2cbff33 100644
--- a/res/res_pjsip_transport_websocket.c
+++ b/res/res_pjsip_transport_websocket.c
@@ -429,12 +429,23 @@
 		pjsip_sip_uri *uri = pjsip_uri_get_uri(contact->uri);
 		const pj_str_t *txp_str = &STR_WS;
 
-		ast_debug(4, "%s re-writing Contact URI from %.*s:%d%s%.*s to %s:%d;transport=%s\n",
-			pjsip_rx_data_get_info(rdata),
-			(int)pj_strlen(&uri->host), pj_strbuf(&uri->host), uri->port,
-			pj_strlen(&uri->transport_param) ? ";transport=" : "",
-			(int)pj_strlen(&uri->transport_param), pj_strbuf(&uri->transport_param),
-			rdata->pkt_info.src_name ?: "", rdata->pkt_info.src_port, pj_strbuf(txp_str));
+		if (DEBUG_ATLEAST(4)) {
+			char src_addr_buffer[AST_SOCKADDR_BUFLEN];
+			const char *ipv6_s = "", *ipv6_e = "";
+
+			if (pj_strchr(&uri->host, ':')) {
+				ipv6_s = "[";
+				ipv6_e = "]";
+			}
+
+			ast_log(LOG_DEBUG, "%s re-writing Contact URI from %s%.*s%s:%d%s%.*s to %s;transport=%s\n",
+				pjsip_rx_data_get_info(rdata),
+				ipv6_s, (int) pj_strlen(&uri->host), pj_strbuf(&uri->host), ipv6_e, uri->port,
+				pj_strlen(&uri->transport_param) ? ";transport=" : "",
+				(int) pj_strlen(&uri->transport_param), pj_strbuf(&uri->transport_param),
+				pj_sockaddr_print(&rdata->pkt_info.src_addr, src_addr_buffer, sizeof(src_addr_buffer), 3),
+				pj_strbuf(txp_str));
+		}
 
 		pj_cstr(&uri->host, rdata->pkt_info.src_name);
 		uri->port = rdata->pkt_info.src_port;

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: I5cfe997ced7883862a12b9c7d8551d76ae02fcf8
Gerrit-Change-Number: 10058
Gerrit-PatchSet: 3
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Michael L. Young <elgueromexicano at gmail.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180917/d8450b87/attachment-0001.html>


More information about the asterisk-code-review mailing list