[asterisk-commits] DNS: Fix some corner cases. (asterisk[master])

[SVN commits to the Asterisk project]

Joshua Colp has submitted this change and it was merged.

Change subject: DNS: Fix some corner cases.
......................................................................


DNS: Fix some corner cases.

* Fix query_set destruction before we are done kicking the queries off.

* Fixed no queries requested handling.

* Add empty queries request unit test.

* Added missing allocation check in ast_dns_query_set_add().

* Made initial pjsip resolving query vector slightly larger.

ASTERISK-25115
Reported by: John Bigelow

Change-Id: Ie8be8347d0992e93946d72b6e7b1299727b038f2
---
M main/dns_query_set.c
M res/res_pjsip/pjsip_resolver.c
M tests/test_dns_query_set.c
3 files changed, 49 insertions(+), 2 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  Ashley Sanders: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/main/dns_query_set.c b/main/dns_query_set.c
index 147c737..40a89e1 100644
--- a/main/dns_query_set.c
+++ b/main/dns_query_set.c
@@ -130,7 +130,10 @@
 		return -1;
 	}
 
-	AST_VECTOR_APPEND(&query_set->queries, query);
+	if (AST_VECTOR_APPEND(&query_set->queries, query)) {
+		ao2_ref(query.query, -1);
+		return -1;
+	}
 
 	return 0;
 }
@@ -175,6 +178,11 @@
 	query_set->callback = callback;
 	query_set->user_data = ao2_bump(data);
 
+	/*
+	 * Bump the query_set ref in case all queries complete
+	 * before we are done kicking them off.
+	 */
+	ao2_ref(query_set, +1);
 	for (idx = 0; idx < AST_VECTOR_SIZE(&query_set->queries); ++idx) {
 		struct dns_query_set_query *query = AST_VECTOR_GET_ADDR(&query_set->queries, idx);
 
@@ -187,6 +195,17 @@
 
 		dns_query_set_callback(query->query);
 	}
+	if (!idx) {
+		/*
+		 * There were no queries in the set;
+		 * therefore all queries are "completed".
+		 * Invoke the final callback.
+		 */
+		query_set->callback(query_set);
+		ao2_cleanup(query_set->user_data);
+		query_set->user_data = NULL;
+	}
+	ao2_ref(query_set, -1);
 }
 
 /*! \brief Structure used for signaling back for synchronous resolution completion */
diff --git a/res/res_pjsip/pjsip_resolver.c b/res/res_pjsip/pjsip_resolver.c
index 4573e4c..915d1d9 100644
--- a/res/res_pjsip/pjsip_resolver.c
+++ b/res/res_pjsip/pjsip_resolver.c
@@ -516,7 +516,7 @@
 	resolve->callback = cb;
 	resolve->token = token;
 
-	if (AST_VECTOR_INIT(&resolve->resolving, 2)) {
+	if (AST_VECTOR_INIT(&resolve->resolving, 4)) {
 		ao2_ref(resolve, -1);
 		cb(PJ_ENOMEM, token, NULL);
 		return;
@@ -565,6 +565,12 @@
 		cb(PJ_ENOMEM, token, NULL);
 		return;
 	}
+	if (!resolve->queries) {
+		ast_debug(2, "[%p] No resolution queries for target '%s'\n", resolve, host);
+		ao2_ref(resolve, -1);
+		cb(PJLIB_UTIL_EDNSNOANSWERREC, token, NULL);
+		return;
+	}
 
 	ast_debug(2, "[%p] Starting initial resolution using parallel queries for target '%s'\n", resolve, host);
 	ast_dns_query_set_resolve_async(resolve->queries, sip_resolve_callback, resolve);
diff --git a/tests/test_dns_query_set.c b/tests/test_dns_query_set.c
index 2efc881..98a6051 100644
--- a/tests/test_dns_query_set.c
+++ b/tests/test_dns_query_set.c
@@ -306,6 +306,26 @@
 	return query_set_test(test, 4, 0);
 }
 
+AST_TEST_DEFINE(query_set_empty)
+{
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "query_set_empty";
+		info->category = "/main/dns/query_set/";
+		info->summary = "Test nominal asynchronous empty DNS query set";
+		info->description =
+			"This tests nominal query set in the following ways:\n"
+			"\t* No queries are added to a query set\n"
+			"\t* Asynchronous resolution of the query set is started\n"
+			"\t* We ensure that the query set callback is invoked upon completion";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	return query_set_test(test, 0, 0);
+}
+
 AST_TEST_DEFINE(query_set_nominal_cancel)
 {
 	switch (cmd) {
@@ -352,6 +372,7 @@
 static int unload_module(void)
 {
 	AST_TEST_UNREGISTER(query_set);
+	AST_TEST_UNREGISTER(query_set_empty);
 	AST_TEST_UNREGISTER(query_set_nominal_cancel);
 	AST_TEST_UNREGISTER(query_set_off_nominal_cancel);
 
@@ -361,6 +382,7 @@
 static int load_module(void)
 {
 	AST_TEST_REGISTER(query_set);
+	AST_TEST_REGISTER(query_set_empty);
 	AST_TEST_REGISTER(query_set_nominal_cancel);
 	AST_TEST_REGISTER(query_set_off_nominal_cancel);
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie8be8347d0992e93946d72b6e7b1299727b038f2
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>