[Asterisk-code-review] res pjsip: Fix infinite recursion when loading transports f... (asterisk[13])

George Joseph asteriskteam at digium.com
Fri Jan 29 18:26:07 CST 2016


George Joseph has uploaded a new change for review.

  https://gerrit.asterisk.org/2129

Change subject: res_pjsip:  Fix infinite recursion when loading transports from realtime
......................................................................

res_pjsip:  Fix infinite recursion when loading transports from realtime

Attempting to load a transport from realtime was forcing asterisk into an
infinite recursion loop.  The first thing transport_apply did was to do a
sorcery retrieve by id for an existing transport of the same name. For files,
this just returns the previous object from res_sorcery_config's internal
container, if any.  For realtime, the res_sourcery_realtime driver looks in the
database and finds the existing row but now it has to rehydrate it into a
sorcery object which means calling... transport_apply.  And so it goes.

The main issue with loading from realtime (apart from the loop) was that
transport stores structures and pointers directly in the ast_sip_transport
structure instead of the separate ast_transport_state structure.  This patch
separates those items into the ast_sip_transport_state structure.  The pattern
is the same as res_pjsip_outbound_registration.

Although all current usages of ast_sip_transport and ast_sip_transport_state
were modified to use the new ast_sip_get_transport_state API, the original
items are left in ast_sip_transport and kept updated to maintain ABI
compatability for third-party modules.  They are marked as deprecated and
noted that they're now in ast_sip_transport_state.

ASTERISK-25606 #close
Reported-by: Martin Moučka

Change-Id: Ic7a836ea8e786e8def51fe3f8cce855ea54f5f19
---
M include/asterisk/res_pjsip.h
M res/res_pjsip.c
M res/res_pjsip/config_transport.c
M res/res_pjsip_endpoint_identifier_anonymous.c
M res/res_pjsip_endpoint_identifier_user.c
M res/res_pjsip_multihomed.c
M res/res_pjsip_nat.c
M res/res_pjsip_outbound_registration.c
M res/res_pjsip_sdp_rtp.c
M res/res_pjsip_session.c
M res/res_pjsip_t38.c
11 files changed, 386 insertions(+), 127 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/29/2129/1

diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 5dc16bc..908a21a 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -54,33 +54,48 @@
 struct pjsip_tls_setting;
 struct pjsip_tpselector;
 
+/*! \brief Maximum number of ciphers supported for a TLS transport */
+#define SIP_TLS_MAX_CIPHERS 64
+
 /*!
  * \brief Structure for SIP transport information
  */
 struct ast_sip_transport_state {
 	/*! \brief Transport itself */
 	struct pjsip_transport *transport;
-
 	/*! \brief Transport factory */
 	struct pjsip_tpfactory *factory;
+	/*!
+	 * Address and port to bind to
+	 * \since 13.8.0
+	 */
+	pj_sockaddr host;
+	/*!
+	 * TLS settings
+	 * \since 13.8.0
+	 */
+	pjsip_tls_setting tls;
+	/*!
+	 * Configured TLS ciphers
+	 * \since 13.8.0
+	 */
+	pj_ssl_cipher ciphers[SIP_TLS_MAX_CIPHERS];
+	/*!
+	 * Optional local network information, used for NAT purposes
+	 * \since 13.8.0
+	 */
+	struct ast_ha *localnet;
+	/*!
+	 * DNS manager for refreshing the external address
+	 * \since 13.8.0
+	 */
+	struct ast_dnsmgr_entry *external_address_refresher;
+	/*!
+	 * Optional external address information
+	 * \since 13.8.0
+	 */
+	struct ast_sockaddr external_address;
 };
-
-#define SIP_SORCERY_DOMAIN_ALIAS_TYPE "domain_alias"
-
-/*!
- * Details about a SIP domain alias
- */
-struct ast_sip_domain_alias {
-	/*! Sorcery object details */
-	SORCERY_OBJECT(details);
-	AST_DECLARE_STRING_FIELDS(
-		/*! Domain to be aliased to */
-		AST_STRING_FIELD(domain);
-	);
-};
-
-/*! \brief Maximum number of ciphers supported for a TLS transport */
-#define SIP_TLS_MAX_CIPHERS 64
 
 /*
  * \brief Transport to bind to
@@ -108,23 +123,51 @@
 		);
 	/*! Type of transport */
 	enum ast_transport type;
-	/*! Address and port to bind to */
+	/*!
+	 * \deprecated Moved to ast_sip_transport_state
+	 * \version 13.8.0 deprecated
+	 * Address and port to bind to
+	 */
 	pj_sockaddr host;
 	/*! Number of simultaneous asynchronous operations */
 	unsigned int async_operations;
 	/*! Optional external port for signaling */
 	unsigned int external_signaling_port;
-	/*! TLS settings */
+	/*!
+	 * \deprecated Moved to ast_sip_transport_state
+	 * \version 13.7.1 deprecated
+	 * TLS settings
+	 */
 	pjsip_tls_setting tls;
-	/*! Configured TLS ciphers */
+	/*!
+	 * \deprecated Moved to ast_sip_transport_state
+	 * \version 13.7.1 deprecated
+	 * Configured TLS ciphers
+	 */
 	pj_ssl_cipher ciphers[SIP_TLS_MAX_CIPHERS];
-	/*! Optional local network information, used for NAT purposes */
+	/*!
+	 * \deprecated Moved to ast_sip_transport_state
+	 * \version 13.7.1 deprecated
+	 * Optional local network information, used for NAT purposes
+	 */
 	struct ast_ha *localnet;
-	/*! DNS manager for refreshing the external address */
+	/*!
+	 * \deprecated Moved to ast_sip_transport_state
+	 * \version 13.7.1 deprecated
+	 * DNS manager for refreshing the external address
+	 */
 	struct ast_dnsmgr_entry *external_address_refresher;
-	/*! Optional external address information */
+	/*!
+	 * \deprecated Moved to ast_sip_transport_state
+	 * \version 13.7.1 deprecated
+	 * Optional external address information
+	 */
 	struct ast_sockaddr external_address;
-	/*! Transport state information */
+	/*!
+	 * \deprecated
+	 * \version 13.7.1 deprecated
+	 * Transport state information
+	 */
 	struct ast_sip_transport_state *state;
 	/*! QOS DSCP TOS bits */
 	unsigned int tos;
@@ -132,6 +175,20 @@
 	unsigned int cos;
 	/*! Write timeout */
 	int write_timeout;
+};
+
+#define SIP_SORCERY_DOMAIN_ALIAS_TYPE "domain_alias"
+
+/*!
+ * Details about a SIP domain alias
+ */
+struct ast_sip_domain_alias {
+	/*! Sorcery object details */
+	SORCERY_OBJECT(details);
+	AST_DECLARE_STRING_FIELDS(
+		/*! Domain to be aliased to */
+		AST_STRING_FIELD(domain);
+	);
 };
 
 /*!
@@ -2172,4 +2229,15 @@
  */
 long ast_sip_threadpool_queue_size(void);
 
+/*!
+ * \brief Retrieve transport state
+ * \since 13.7.1
+ *
+ * @param transport_id
+ * @returns transport_state
+ *
+ * \note ao2_cleanup(...) or ao2_ref(...,  -1) must be called on the returned object
+ */
+struct ast_sip_transport_state *ast_sip_get_transport_state(const char *transport_id);
+
 #endif /* _RES_PJSIP_H */
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index c94f6a9..d0d6ba9 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -2486,6 +2486,7 @@
 static int sip_get_tpselector_from_endpoint(const struct ast_sip_endpoint *endpoint, pjsip_tpselector *selector)
 {
 	RAII_VAR(struct ast_sip_transport *, transport, NULL, ao2_cleanup);
+	RAII_VAR(struct ast_sip_transport_state *, transport_state, NULL, ao2_cleanup);
 	const char *transport_name = endpoint->transport;
 
 	if (ast_strlen_zero(transport_name)) {
@@ -2493,19 +2494,20 @@
 	}
 
 	transport = ast_sorcery_retrieve_by_id(ast_sip_get_sorcery(), "transport", transport_name);
+	transport_state = ast_sip_get_transport_state(transport_name);
 
-	if (!transport || !transport->state) {
+	if (!transport || !transport_state) {
 		ast_log(LOG_ERROR, "Unable to retrieve PJSIP transport '%s' for endpoint '%s'\n",
 			transport_name, ast_sorcery_object_get_id(endpoint));
 		return -1;
 	}
 
-	if (transport->state->transport) {
+	if (transport_state->transport) {
 		selector->type = PJSIP_TPSELECTOR_TRANSPORT;
-		selector->u.transport = transport->state->transport;
-	} else if (transport->state->factory) {
+		selector->u.transport = transport_state->transport;
+	} else if (transport_state->factory) {
 		selector->type = PJSIP_TPSELECTOR_LISTENER;
-		selector->u.listener = transport->state->factory;
+		selector->u.listener = transport_state->factory;
 	} else if (transport->type == AST_TRANSPORT_WS || transport->type == AST_TRANSPORT_WSS) {
 		/* The WebSocket transport has no factory as it can not create outgoing connections, so
 		 * even if an endpoint is locked to a WebSocket transport we let the PJSIP logic
diff --git a/res/res_pjsip/config_transport.c b/res/res_pjsip/config_transport.c
index 840824b..43c222f 100644
--- a/res/res_pjsip/config_transport.c
+++ b/res/res_pjsip/config_transport.c
@@ -31,6 +31,67 @@
 #include "include/res_pjsip_private.h"
 #include "asterisk/http_websocket.h"
 
+/*! \brief Default number of state container buckets */
+#define DEFAULT_STATE_BUCKETS 53
+static AO2_GLOBAL_OBJ_STATIC(current_states);
+
+struct internal_state {
+	/*! \brief Transport configuration object */
+	struct ast_sip_transport *transport;
+	/*! \brief Transport state information */
+	struct ast_sip_transport_state *state;
+};
+
+/*! \brief hashing function for state objects */
+static int internal_state_hash(const void *obj, const int flags)
+{
+	const struct internal_state *object;
+	const char *key;
+
+	switch (flags & OBJ_SEARCH_MASK) {
+	case OBJ_SEARCH_KEY:
+		key = obj;
+		break;
+	case OBJ_SEARCH_OBJECT:
+		object = obj;
+		key = ast_sorcery_object_get_id(object->transport);
+		break;
+	default:
+		ast_assert(0);
+		return 0;
+	}
+	return ast_str_hash(key);
+}
+
+/*! \brief comparator function for state objects */
+static int internal_state_cmp(void *obj, void *arg, int flags)
+{
+	const struct internal_state *object_left = obj;
+	const struct internal_state *object_right = arg;
+	const char *right_key = arg;
+	int cmp;
+
+	switch (flags & OBJ_SEARCH_MASK) {
+	case OBJ_SEARCH_OBJECT:
+		right_key = ast_sorcery_object_get_id(object_right->transport);
+		/* Fall through */
+	case OBJ_SEARCH_KEY:
+		cmp = strcmp(ast_sorcery_object_get_id(object_left->transport), right_key);
+		break;
+	case OBJ_SEARCH_PARTIAL_KEY:
+		/* Not supported by container. */
+		ast_assert(0);
+		return 0;
+	default:
+		cmp = 0;
+		break;
+	}
+	if (cmp) {
+		return 0;
+	}
+	return CMP_MATCH;
+}
+
 static int sip_transport_to_ami(const struct ast_sip_transport *transport,
 				struct ast_str **buf)
 {
@@ -75,58 +136,6 @@
 	.format_ami = format_ami_endpoint_transport
 };
 
-static int destroy_transport_state(void *data)
-{
-	pjsip_transport *transport = data;
-	pjsip_transport_shutdown(transport);
-	return 0;
-}
-
-/*! \brief Destructor for transport state information */
-static void transport_state_destroy(void *obj)
-{
-	struct ast_sip_transport_state *state = obj;
-
-	if (state->transport) {
-		ast_sip_push_task_synchronous(NULL, destroy_transport_state, state->transport);
-	}
-}
-
-/*! \brief Destructor for transport */
-static void transport_destroy(void *obj)
-{
-	struct ast_sip_transport *transport = obj;
-
-	ast_string_field_free_memory(transport);
-	ast_free_ha(transport->localnet);
-
-	if (transport->external_address_refresher) {
-		ast_dnsmgr_release(transport->external_address_refresher);
-	}
-
-	ao2_cleanup(transport->state);
-}
-
-/*! \brief Allocator for transport */
-static void *transport_alloc(const char *name)
-{
-	struct ast_sip_transport *transport = ast_sorcery_generic_alloc(sizeof(*transport), transport_destroy);
-
-	if (!transport) {
-		return NULL;
-	}
-
-	if (ast_string_field_init(transport, 256)) {
-		ao2_cleanup(transport);
-		return NULL;
-	}
-
-	pjsip_tls_setting_default(&transport->tls);
-	transport->tls.ciphers = transport->ciphers;
-
-	return transport;
-}
-
 static void set_qos(struct ast_sip_transport *transport, pj_qos_params *qos)
 {
 	int tos_as_dscp = transport->tos >> 2;
@@ -141,30 +150,142 @@
 	}
 }
 
+/*! \brief Destructor for transport */
+static void sip_transport_destroy(void *obj)
+{
+	struct ast_sip_transport *transport = obj;
+
+	ast_string_field_free_memory(transport);
+}
+
+/*! \brief Allocator for transport */
+static void *sip_transport_alloc(const char *name)
+{
+	struct ast_sip_transport *transport = ast_sorcery_generic_alloc(sizeof(*transport), sip_transport_destroy);
+
+	if (!transport) {
+		return NULL;
+	}
+
+	if (ast_string_field_init(transport, 256)) {
+		ao2_cleanup(transport);
+		return NULL;
+	}
+
+	return transport;
+}
+
+static int destroy_sip_transport_state(void *data)
+{
+	struct ast_sip_transport_state *transport_state = data;
+
+	ast_free_ha(transport_state->localnet);
+
+	if (transport_state->external_address_refresher) {
+		ast_dnsmgr_release(transport_state->external_address_refresher);
+	}
+	pjsip_transport_shutdown(transport_state->transport);
+	return 0;
+}
+
+/*! \brief Destructor for ast_sip_transport state information */
+static void sip_transport_state_destroy(void *obj)
+{
+	struct ast_sip_transport_state *state = obj;
+
+	if (state->transport) {
+		ast_sip_push_task_synchronous(NULL, destroy_sip_transport_state, state);
+	}
+}
+
+/*! \brief Destructor for ast_sip_transport state information */
+static void internal_state_destroy(void *obj)
+{
+	struct internal_state *state = obj;
+
+	ao2_cleanup(state->transport);
+	ao2_cleanup(state->state);
+}
+
+static struct internal_state *internal_state_alloc(struct ast_sip_transport *transport)
+{
+	struct internal_state *internal_state;
+
+	internal_state = ao2_alloc(sizeof(*internal_state), internal_state_destroy);
+	if (!internal_state) {
+		return NULL;
+	}
+
+	internal_state->state = ao2_alloc(sizeof(*internal_state->state), sip_transport_state_destroy);
+	if (!internal_state->state) {
+		ao2_cleanup(internal_state);
+		return NULL;
+	}
+
+	internal_state->transport = transport;
+	transport->state = internal_state->state;
+	ao2_bump(transport);
+
+	return internal_state;
+}
+
+static void copy_transport_to_state(struct ast_sip_transport *transport)
+{
+	ast_assert(transport && transport->state);
+
+	memcpy(&transport->state->host, &transport->host, sizeof(transport->host));
+	memcpy(&transport->state->tls, &transport->tls, sizeof(transport->tls));
+	memcpy(&transport->state->ciphers, &transport->ciphers, sizeof(transport->ciphers));
+	transport->state->localnet = transport->localnet;
+	transport->state->external_address_refresher = transport->external_address_refresher;
+	memcpy(&transport->state->external_address, &transport->external_address, sizeof(transport->external_address));
+}
+
+static void copy_state_to_transport(struct ast_sip_transport *transport)
+{
+	ast_assert(transport && transport->state);
+
+	memcpy(&transport->host, &transport->state->host, sizeof(transport->host));
+	memcpy(&transport->tls, &transport->state->tls, sizeof(transport->tls));
+	memcpy(&transport->ciphers, &transport->state->ciphers, sizeof(transport->ciphers));
+	transport->localnet = transport->state->localnet;
+	transport->external_address_refresher = transport->state->external_address_refresher;
+	memcpy(&transport->external_address, &transport->state->external_address, sizeof(transport->external_address));
+}
+
 /*! \brief Apply handler for transports */
 static int transport_apply(const struct ast_sorcery *sorcery, void *obj)
 {
 	struct ast_sip_transport *transport = obj;
-	RAII_VAR(struct ast_sip_transport *, existing, ast_sorcery_retrieve_by_id(sorcery, "transport", ast_sorcery_object_get_id(obj)), ao2_cleanup);
+	const char *transport_id = ast_sorcery_object_get_id(obj);
+	RAII_VAR(struct ao2_container *, states, ao2_global_obj_ref(current_states), ao2_cleanup);
+	RAII_VAR(struct internal_state *, new_state, NULL, ao2_cleanup);
+	RAII_VAR(struct internal_state *, old_state, NULL, ao2_cleanup);
 	pj_status_t res = -1;
 
-	if (!existing || !existing->state) {
-		if (!(transport->state = ao2_alloc(sizeof(*transport->state), transport_state_destroy))) {
-			ast_log(LOG_ERROR, "Transport state for '%s' could not be allocated\n", ast_sorcery_object_get_id(obj));
-			return -1;
-		}
-	} else {
-		transport->state = existing->state;
-		ao2_ref(transport->state, +1);
+	if (!states) {
+		return -1;
 	}
 
-	/* Once active a transport can not be reconfigured */
-	if (transport->state->transport || transport->state->factory) {
+	old_state = ao2_find(states, transport_id, OBJ_SEARCH_KEY);
+	if (old_state) {
+		transport->state = old_state->state;
+		copy_state_to_transport(transport);
+		ao2_lock(states);
+		ao2_replace(old_state->transport, transport);
+		ao2_unlock(states);
+		return 0;
+	}
+
+	new_state = internal_state_alloc(transport);
+	if (!new_state) {
+		ast_log(LOG_ERROR, "Transport state for '%s' could not be allocated\n", transport_id);
 		return -1;
 	}
 
 	if (transport->host.addr.sa_family != PJ_AF_INET && transport->host.addr.sa_family != PJ_AF_INET6) {
 		ast_log(LOG_ERROR, "Transport '%s' could not be started as binding not specified\n", ast_sorcery_object_get_id(obj));
+		ao2_ref(new_state, -1);
 		return -1;
 	}
 
@@ -182,11 +303,13 @@
 		} else {
 			ast_log(LOG_ERROR, "Unknown address family for transport '%s', could not get external signaling address\n",
 					ast_sorcery_object_get_id(obj));
+			ao2_ref(new_state, -1);
 			return -1;
 		}
 
 		if (ast_dnsmgr_lookup(transport->external_signaling_address, &transport->external_address, &transport->external_address_refresher, NULL) < 0) {
 			ast_log(LOG_ERROR, "Could not create dnsmgr for external signaling address on '%s'\n", ast_sorcery_object_get_id(obj));
+			ao2_ref(new_state, -1);
 			return -1;
 		}
 	}
@@ -217,15 +340,20 @@
 
 		res = pjsip_tcp_transport_start3(ast_sip_get_pjsip_endpoint(), &cfg, &transport->state->factory);
 	} else if (transport->type == AST_TRANSPORT_TLS) {
+		pjsip_tls_setting_default(&transport->tls);
+		transport->tls.ciphers = transport->ciphers;
+
 		if (transport->async_operations > 1 && ast_compare_versions(pj_get_version(), "2.5.0") < 0) {
 			ast_log(LOG_ERROR, "Transport: %s: When protocol=tls and pjproject version < 2.5.0, async_operations can't be > 1\n",
 					ast_sorcery_object_get_id(obj));
+			ao2_ref(new_state, -1);
 			return -1;
 		}
 		if (!ast_strlen_zero(transport->ca_list_file)) {
 			if (!ast_file_is_readable(transport->ca_list_file)) {
 				ast_log(LOG_ERROR, "Transport: %s: ca_list_file %s is either missing or not readable\n",
 						ast_sorcery_object_get_id(obj), transport->ca_list_file);
+				ao2_ref(new_state, -1);
 				return -1;
 			}
 		}
@@ -235,6 +363,7 @@
 			if (!ast_file_is_readable(transport->ca_list_path)) {
 				ast_log(LOG_ERROR, "Transport: %s: ca_list_path %s is either missing or not readable\n",
 						ast_sorcery_object_get_id(obj), transport->ca_list_path);
+				ao2_ref(new_state, -1);
 				return -1;
 			}
 		}
@@ -249,6 +378,7 @@
 			if (!ast_file_is_readable(transport->cert_file)) {
 				ast_log(LOG_ERROR, "Transport: %s: cert_file %s is either missing or not readable\n",
 						ast_sorcery_object_get_id(obj), transport->cert_file);
+				ao2_ref(new_state, -1);
 				return -1;
 			}
 		}
@@ -257,9 +387,11 @@
 			if (!ast_file_is_readable(transport->privkey_file)) {
 				ast_log(LOG_ERROR, "Transport: %s: privkey_file %s is either missing or not readable\n",
 						ast_sorcery_object_get_id(obj), transport->privkey_file);
+				ao2_ref(new_state, -1);
 				return -1;
 			}
 		}
+
 		transport->tls.privkey_file = pj_str((char*)transport->privkey_file);
 		transport->tls.password = pj_str((char*)transport->password);
 		set_qos(transport, &transport->tls.qos_params);
@@ -277,8 +409,15 @@
 
 		pj_strerror(res, msg, sizeof(msg));
 		ast_log(LOG_ERROR, "Transport '%s' could not be started: %s\n", ast_sorcery_object_get_id(obj), msg);
+		ao2_ref(new_state, -1);
 		return -1;
 	}
+
+	copy_transport_to_state(transport);
+	ao2_lock(states);
+	ao2_link(states, new_state);
+	ao2_unlock(states);
+
 	return 0;
 }
 
@@ -770,14 +909,44 @@
 
 static struct ast_sip_cli_formatter_entry *cli_formatter;
 
+struct ast_sip_transport_state *ast_sip_get_transport_state(const char *transport_id)
+{
+	RAII_VAR(struct ao2_container *, states, ao2_global_obj_ref(current_states), ao2_cleanup);
+	RAII_VAR(struct internal_state *, state, NULL, ao2_cleanup);
+
+	if (!states) {
+		return NULL;
+	}
+
+	state = ao2_find(states, transport_id, OBJ_SEARCH_KEY);
+	if (!state || !state->state) {
+		return NULL;
+	}
+
+	ao2_bump(state->state);
+
+	return (state->state);
+}
+
 /*! \brief Initialize sorcery with transport support */
 int ast_sip_initialize_sorcery_transport(void)
 {
 	struct ast_sorcery *sorcery = ast_sip_get_sorcery();
+	struct ao2_container *new_states;
+
+	/* Create outbound registration states container. */
+	new_states = ao2_container_alloc(DEFAULT_STATE_BUCKETS,
+		internal_state_hash, internal_state_cmp);
+	if (!new_states) {
+		ast_log(LOG_ERROR, "Unable to allocate transport states container\n");
+		return AST_MODULE_LOAD_FAILURE;
+	}
+	ao2_global_obj_replace_unref(current_states, new_states);
+	ao2_ref(new_states, -1);
 
 	ast_sorcery_apply_default(sorcery, "transport", "config", "pjsip.conf,criteria=type=transport");
 
-	if (ast_sorcery_object_register_no_reload(sorcery, "transport", transport_alloc, NULL, transport_apply)) {
+	if (ast_sorcery_object_register_no_reload(sorcery, "transport", sip_transport_alloc, NULL, transport_apply)) {
 		return -1;
 	}
 
@@ -831,6 +1000,7 @@
 	ast_sip_unregister_cli_formatter(cli_formatter);
 
 	internal_sip_unregister_endpoint_formatter(&endpoint_transport_formatter);
+	ao2_global_obj_release(current_states);
 
 	return 0;
 }
diff --git a/res/res_pjsip_endpoint_identifier_anonymous.c b/res/res_pjsip_endpoint_identifier_anonymous.c
index 274c055..45f0e94 100644
--- a/res/res_pjsip_endpoint_identifier_anonymous.c
+++ b/res/res_pjsip_endpoint_identifier_anonymous.c
@@ -45,11 +45,12 @@
 static int find_transport_in_use(void *obj, void *arg, int flags)
 {
 	struct ast_sip_transport *transport = obj;
+	RAII_VAR(struct ast_sip_transport_state *, transport_state, ast_sip_get_transport_state(ast_sorcery_object_get_id(transport)), ao2_cleanup);
 	pjsip_rx_data *rdata = arg;
 
-	if ((transport->state->transport == rdata->tp_info.transport) ||
-		(transport->state->factory && !pj_strcmp(&transport->state->factory->addr_name.host, &rdata->tp_info.transport->local_name.host) &&
-			transport->state->factory->addr_name.port == rdata->tp_info.transport->local_name.port)) {
+	if (transport_state && ((transport_state->transport == rdata->tp_info.transport) ||
+		(transport_state->factory && !pj_strcmp(&transport_state->factory->addr_name.host, &rdata->tp_info.transport->local_name.host) &&
+			transport_state->factory->addr_name.port == rdata->tp_info.transport->local_name.port))) {
 		return CMP_MATCH | CMP_STOP;
 	}
 
diff --git a/res/res_pjsip_endpoint_identifier_user.c b/res/res_pjsip_endpoint_identifier_user.c
index 5abf879..d1eabe4 100644
--- a/res/res_pjsip_endpoint_identifier_user.c
+++ b/res/res_pjsip_endpoint_identifier_user.c
@@ -45,11 +45,12 @@
 static int find_transport_in_use(void *obj, void *arg, int flags)
 {
 	struct ast_sip_transport *transport = obj;
+	RAII_VAR(struct ast_sip_transport_state *, transport_state, ast_sip_get_transport_state(ast_sorcery_object_get_id(transport)), ao2_cleanup);
 	pjsip_rx_data *rdata = arg;
 
-	if ((transport->state->transport == rdata->tp_info.transport) ||
-		(transport->state->factory && !pj_strcmp(&transport->state->factory->addr_name.host, &rdata->tp_info.transport->local_name.host) &&
-			transport->state->factory->addr_name.port == rdata->tp_info.transport->local_name.port)) {
+	if (transport_state && ((transport_state->transport == rdata->tp_info.transport) ||
+		(transport_state->factory && !pj_strcmp(&transport_state->factory->addr_name.host, &rdata->tp_info.transport->local_name.host) &&
+			transport_state->factory->addr_name.port == rdata->tp_info.transport->local_name.port))) {
 		return CMP_MATCH | CMP_STOP;
 	}
 
diff --git a/res/res_pjsip_multihomed.c b/res/res_pjsip_multihomed.c
index 437a1cbe..215e5e0 100644
--- a/res/res_pjsip_multihomed.c
+++ b/res/res_pjsip_multihomed.c
@@ -44,13 +44,15 @@
 	}
 
 	for (iter = ao2_iterator_init(transports, 0); (transport = ao2_iterator_next(&iter)); ao2_ref(transport, -1)) {
-		if ((transport->type != AST_TRANSPORT_UDP) ||
-			(pj_strcmp(&transport->state->transport->local_name.host, address)) ||
-			(transport->state->transport->local_name.port != port)) {
+		RAII_VAR(struct ast_sip_transport_state *, transport_state, ast_sip_get_transport_state(ast_sorcery_object_get_id(transport)), ao2_cleanup);
+
+		if (transport_state && ((transport->type != AST_TRANSPORT_UDP) ||
+			(pj_strcmp(&transport_state->transport->local_name.host, address)) ||
+			(transport_state->transport->local_name.port != port))) {
 			continue;
 		}
 
-		sip_transport = transport->state->transport;
+		sip_transport = transport_state->transport;
 		ao2_ref(transport, -1);
 		break;
 	}
diff --git a/res/res_pjsip_nat.c b/res/res_pjsip_nat.c
index a2eb6ad..3991682 100644
--- a/res/res_pjsip_nat.c
+++ b/res/res_pjsip_nat.c
@@ -154,15 +154,16 @@
 {
 	struct ast_sip_transport *transport = obj;
 	struct request_transport_details *details = arg;
+	RAII_VAR(struct ast_sip_transport_state *, transport_state, ast_sip_get_transport_state(ast_sorcery_object_get_id(transport)), ao2_cleanup);
 
 	/* If an explicit transport or factory matches then this is what is in use, if we are unavailable
 	 * to compare based on that we make sure that the type is the same and the source IP address/port are the same
 	 */
-	if ((details->transport && details->transport == transport->state->transport) ||
-		(details->factory && details->factory == transport->state->factory) ||
-		((details->type == transport->type) && (transport->state->factory) &&
-			!pj_strcmp(&transport->state->factory->addr_name.host, &details->local_address) &&
-			transport->state->factory->addr_name.port == details->local_port)) {
+	if (transport_state && ((details->transport && details->transport == transport_state->transport) ||
+		(details->factory && details->factory == transport_state->factory) ||
+		((details->type == transport->type) && (transport_state->factory) &&
+			!pj_strcmp(&transport_state->factory->addr_name.host, &details->local_address) &&
+			transport_state->factory->addr_name.port == details->local_port))) {
 		return CMP_MATCH | CMP_STOP;
 	}
 
@@ -206,6 +207,7 @@
 {
 	RAII_VAR(struct ao2_container *, transports, NULL, ao2_cleanup);
 	RAII_VAR(struct ast_sip_transport *, transport, NULL, ao2_cleanup);
+	RAII_VAR(struct ast_sip_transport_state *, transport_state, NULL, ao2_cleanup);
 	struct request_transport_details details = { 0, };
 	pjsip_via_hdr *via = NULL;
 	struct ast_sockaddr addr = { { 0, } };
@@ -247,9 +249,19 @@
 		}
 	}
 
-	if (!(transports = ast_sorcery_retrieve_by_fields(ast_sip_get_sorcery(), "transport", AST_RETRIEVE_FLAG_MULTIPLE | AST_RETRIEVE_FLAG_ALL, NULL)) ||
-		!(transport = ao2_callback(transports, 0, find_transport_in_use, &details)) || !transport->localnet ||
-		ast_sockaddr_isnull(&transport->external_address)) {
+	if (!(transports = ast_sorcery_retrieve_by_fields(ast_sip_get_sorcery(), "transport", AST_RETRIEVE_FLAG_MULTIPLE | AST_RETRIEVE_FLAG_ALL, NULL))) {
+		return PJ_SUCCESS;
+	}
+
+	if (!(transport = ao2_callback(transports, 0, find_transport_in_use, &details))) {
+		return PJ_SUCCESS;
+	}
+
+	if (!(transport_state = ast_sip_get_transport_state(ast_sorcery_object_get_id(transport)))) {
+		return PJ_SUCCESS;
+	}
+
+	if ( !transport_state->localnet || 	ast_sockaddr_isnull(&transport_state->external_address)) {
 		return PJ_SUCCESS;
 	}
 
@@ -257,13 +269,13 @@
 	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->localnet, &addr) != AST_SENSE_ALLOW) {
+	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->external_address));
+		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);
@@ -272,7 +284,7 @@
 
 	/* 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->external_address));
+		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_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index a2fefde..daa4f3e 100644
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -1172,20 +1172,20 @@
 	pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
 
 	if (!ast_strlen_zero(registration->transport)) {
-		RAII_VAR(struct ast_sip_transport *, transport, ast_sorcery_retrieve_by_id(ast_sip_get_sorcery(), "transport", registration->transport), ao2_cleanup);
+		RAII_VAR(struct ast_sip_transport_state *, transport_state, ast_sip_get_transport_state(registration->transport), ao2_cleanup);
 
-		if (!transport || !transport->state) {
+		if (!transport_state) {
 			ast_log(LOG_ERROR, "Unable to retrieve PJSIP transport '%s' "
 				" for outbound registration", registration->transport);
 			return -1;
 		}
 
-		if (transport->state->transport) {
+		if (transport_state->transport) {
 			selector.type = PJSIP_TPSELECTOR_TRANSPORT;
-			selector.u.transport = transport->state->transport;
-		} else if (transport->state->factory) {
+			selector.u.transport = transport_state->transport;
+		} else if (transport_state->factory) {
 			selector.type = PJSIP_TPSELECTOR_LISTENER;
-			selector.u.listener = transport->state->factory;
+			selector.u.listener = transport_state->factory;
 		} else {
 			return -1;
 		}
diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index 0e1d7da..18a7f3f 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -1350,11 +1350,12 @@
 /*! \brief Function which updates the media stream with external media address, if applicable */
 static void change_outgoing_sdp_stream_media_address(pjsip_tx_data *tdata, struct pjmedia_sdp_media *stream, struct ast_sip_transport *transport)
 {
+	RAII_VAR(struct ast_sip_transport_state *, transport_state, ast_sip_get_transport_state(ast_sorcery_object_get_id(transport)), ao2_cleanup);
 	char host[NI_MAXHOST];
 	struct ast_sockaddr addr = { { 0, } };
 
 	/* If the stream has been rejected there will be no connection line */
-	if (!stream->conn) {
+	if (!stream->conn || !transport_state) {
 		return;
 	}
 
@@ -1362,7 +1363,7 @@
 	ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);
 
 	/* Is the address within the SDP inside the same network? */
-	if (ast_apply_ha(transport->localnet, &addr) == AST_SENSE_ALLOW) {
+	if (ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {
 		return;
 	}
 
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 4f5071f..aad2438 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -2773,13 +2773,14 @@
 /*! \brief Hook for modifying outgoing messages with SDP to contain the proper address information */
 static void session_outgoing_nat_hook(pjsip_tx_data *tdata, struct ast_sip_transport *transport)
 {
+	RAII_VAR(struct ast_sip_transport_state *, transport_state, ast_sip_get_transport_state(ast_sorcery_object_get_id(transport)), ao2_cleanup);
 	struct ast_sip_nat_hook *hook = ast_sip_mod_data_get(
 		tdata->mod_data, session_module.id, MOD_DATA_NAT_HOOK);
 	struct pjmedia_sdp_session *sdp;
 	int stream;
 
 	/* SDP produced by us directly will never be multipart */
-	if (hook || !tdata->msg->body || pj_stricmp2(&tdata->msg->body->content_type.type, "application") ||
+	if (!transport_state || hook || !tdata->msg->body || pj_stricmp2(&tdata->msg->body->content_type.type, "application") ||
 		pj_stricmp2(&tdata->msg->body->content_type.subtype, "sdp") || ast_strlen_zero(transport->external_media_address)) {
 		return;
 	}
@@ -2793,7 +2794,7 @@
 		ast_copy_pj_str(host, &sdp->conn->addr, sizeof(host));
 		ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);
 
-		if (ast_apply_ha(transport->localnet, &addr) != AST_SENSE_ALLOW) {
+		if (ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {
 			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 2c544bb..7534d4a 100644
--- a/res/res_pjsip_t38.c
+++ b/res/res_pjsip_t38.c
@@ -890,11 +890,12 @@
 /*! \brief Function which updates the media stream with external media address, if applicable */
 static void change_outgoing_sdp_stream_media_address(pjsip_tx_data *tdata, struct pjmedia_sdp_media *stream, struct ast_sip_transport *transport)
 {
+	RAII_VAR(struct ast_sip_transport_state *, transport_state, ast_sip_get_transport_state(ast_sorcery_object_get_id(transport)), ao2_cleanup);
 	char host[NI_MAXHOST];
 	struct ast_sockaddr addr = { { 0, } };
 
 	/* If the stream has been rejected there will be no connection line */
-	if (!stream->conn) {
+	if (!stream->conn || !transport_state) {
 		return;
 	}
 
@@ -902,7 +903,7 @@
 	ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);
 
 	/* Is the address within the SDP inside the same network? */
-	if (ast_apply_ha(transport->localnet, &addr) == AST_SENSE_ALLOW) {
+	if (ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {
 		return;
 	}
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic7a836ea8e786e8def51fe3f8cce855ea54f5f19
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>



More information about the asterisk-code-review mailing list