resolver: add some additional test coverage

While here, remove some code paths that do not occur by definition. (For example, if the resolver succeeds, we will definitely have a valid set of addresses, but if it fails, we will definitely not.)
nanomsg · Dec 29, 2024 · b5462f6 · b5462f6
1 parent a1bc59a
commit b5462f6
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 18 deletions.
diff --git a/src/platform/posix/posix_resolv_gai.c b/src/platform/posix/posix_resolv_gai.c
@@ -238,7 +238,7 @@ resolv_worker(void *index)
 		if ((aio = resolv_active[tid]) == NULL) {
 			// no more interest (canceled), so ignore the result
 			// and carry on
-			if (results != NULL) {
+			if (rv == 0) {
 				freeaddrinfo(results);
 			}
 			continue;
@@ -248,16 +248,12 @@ resolv_worker(void *index)
 		if (rv != 0) {
 			rv = posix_gai_errno(rv);
 			nni_aio_finish_error(aio, rv);
-			if (results != NULL) {
-				freeaddrinfo(results);
-			}
 			continue;
 		}
 
 		// We only take the first matching address.  Presumably
 		// DNS load balancing is done by the resolver/server.
 
-		rv = NNG_EADDRINVAL;
 		for (probe = results; probe != NULL; probe = probe->ai_next) {
 			if (probe->ai_addr->sa_family == AF_INET) {
 				break;
@@ -271,10 +267,8 @@ resolv_worker(void *index)
 
 		if (probe == NULL) {
 			// no match
-			nni_aio_finish_error(aio, rv);
-			if (results != NULL) {
-				freeaddrinfo(results);
-			}
+			nni_aio_finish_error(aio, NNG_EADDRINVAL);
+			freeaddrinfo(results);
 			continue;
 		}
 

diff --git a/src/platform/resolver_test.c b/src/platform/resolver_test.c
@@ -262,6 +262,45 @@ test_null_not_passive(void)
 	nng_aio_free(aio);
 }
 
+void
+test_bad_family(void)
+{
+	nng_aio        *aio;
+	nng_sockaddr    sa;
+	nni_resolv_item item = { 0 };
+
+	NUTS_PASS(nng_aio_alloc(&aio, NULL, NULL));
+	item.ri_family  = NNG_AF_INPROC;
+	item.ri_host    = "/abcdef";
+	item.ri_port    = 80;
+	item.ri_passive = false;
+	item.ri_sa      = &sa;
+	nni_resolv(&item, aio);
+	nng_aio_wait(aio);
+	NUTS_FAIL(nng_aio_result(aio), NNG_ENOTSUP);
+	nng_aio_free(aio);
+}
+
+void
+test_aio_stopped(void)
+{
+	nng_aio        *aio;
+	nng_sockaddr    sa;
+	nni_resolv_item item = { 0 };
+
+	NUTS_PASS(nng_aio_alloc(&aio, NULL, NULL));
+	item.ri_family  = NNG_AF_INPROC;
+	item.ri_host    = "/abcdef";
+	item.ri_port    = 80;
+	item.ri_passive = false;
+	item.ri_sa      = &sa;
+	nng_aio_stop(aio);
+	nni_resolv(&item, aio);
+	nng_aio_wait(aio);
+	NUTS_FAIL(nng_aio_result(aio), NNG_ESTOPPED);
+	nng_aio_free(aio);
+}
+
 NUTS_TESTS = {
 	{ "resolve google dns", test_google_dns },
 	{ "resolve hostname too long", test_hostname_too_long },
@@ -274,5 +313,7 @@ NUTS_TESTS = {
 	{ "resolve localhost unspecified", test_localhost_unspecified },
 	{ "resolve null passive", test_null_passive },
 	{ "resolve null not passive", test_null_not_passive },
+	{ "resolve bad family", test_bad_family },
+	{ "resolve aio stopped", test_bad_family },
 	{ NULL, NULL },
 };
diff --git a/src/platform/windows/win_resolv.c b/src/platform/windows/win_resolv.c
@@ -196,7 +196,7 @@ resolv_worker(void *index)
 		if ((aio = resolv_active[tid]) == NULL) {
 			// no more interest (canceled), so ignore the result
 			// and carry on
-			if (results != NULL) {
+			if (rv == 0) {
 				freeaddrinfo(results);
 			}
 			continue;
@@ -206,16 +206,12 @@ resolv_worker(void *index)
 		if (rv != 0) {
 			rv = resolv_errno(rv);
 			nni_aio_finish_error(aio, rv);
-			if (results != NULL) {
-				freeaddrinfo(results);
-			}
 			continue;
 		}
 
 		// We only take the first matching address.  Presumably
 		// DNS load balancing is done by the resolver/server.
 
-		rv = NNG_EADDRINVAL;
 		for (probe = results; probe != NULL; probe = probe->ai_next) {
 			if (probe->ai_addr->sa_family == AF_INET) {
 				break;
@@ -229,10 +225,8 @@ resolv_worker(void *index)
 
 		if (probe == NULL) {
 			// no match
-			nni_aio_finish_error(aio, rv);
-			if (results != NULL) {
-				freeaddrinfo(results);
-			}
+			nni_aio_finish_error(aio, NNG_EADDRINVAL);
+			freeaddrinfo(results);
 			continue;
 		}