Merge "Use std::vector to store domains of nameservers and minor change" (514e772a) · Commits · e / os / android_packages_modules_DnsResolver

ResolverController.cpp

+4 −6

Original line number	Diff line number	Diff line
		@@ -228,9 +228,7 @@ int ResolverController::setResolverConfiguration(
		server_ptrs.push_back(resolverParams.servers[i].c_str());
		}

		std::string domains_str = android::base::Join(resolverParams.domains, " ");

		// TODO: Change resolv_set_nameservers_for_net() to use ResolverParamsParcel directly.
		// TODO: Change resolv_set_nameservers() to use ResolverParamsParcel directly.
		res_params res_params = {};
		res_params.sample_validity = resolverParams.sampleValiditySeconds;
		res_params.success_threshold = resolverParams.successThreshold;
		@@ -240,10 +238,10 @@ int ResolverController::setResolverConfiguration(
		res_params.retry_count = resolverParams.retryCount;

		LOG(VERBOSE) << "setDnsServers netId = " << resolverParams.netId
		<< ", numservers = " << resolverParams.domains.size();
		<< ", numservers = " << resolverParams.servers.size();

		return -resolv_set_nameservers_for_net(resolverParams.netId, server_ptrs.data(),
		server_ptrs.size(), domains_str.c_str(), &res_params);
		return -resolv_set_nameservers(resolverParams.netId, server_ptrs.data(), server_ptrs.size(),
		resolverParams.domains, &res_params);
		}

		int ResolverController::getResolverInfo(int32_t netId, std::vector<std::string>* servers,

getaddrinfo.cpp

+3 −3

Original line number	Diff line number	Diff line
		@@ -1679,7 +1679,7 @@ static int res_queryN(const char* name, res_target* target, res_state res, int*
		* is detected. Error code, if any, is left in *herrno.
		*/
		static int res_searchN(const char* name, res_target* target, res_state res, int* herrno) {
		const char cp, const *domain;
		const char* cp;
		HEADER* hp;
		u_int dots;
		int ret, saved_herrno;
		@@ -1722,8 +1722,8 @@ static int res_searchN(const char* name, res_target* target, res_state res, int*
		*/
		_resolv_populate_res_for_net(res);

		for (domain = (const char* const) res->dnsrch; domain && !done; domain++) {
		ret = res_querydomainN(name, *domain, target, res, herrno);
		for (const auto& domain : res->search_domains) {
		ret = res_querydomainN(name, domain.c_str(), target, res, herrno);
		if (ret > 0) return ret;

		/*

libnetd_resolv_test.cpp

+33 −33

Original line number	Diff line number	Diff line
		@@ -368,7 +368,7 @@ TEST_F(ResolvGetAddrInfoTest, AlphabeticalHostname_NoData) {
		dns.addMapping(v4_host_name, ns_type::ns_t_a, "1.2.3.3");
		ASSERT_TRUE(dns.startServer());
		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));
		dns.clearQueries();

		@@ -395,7 +395,7 @@ TEST_F(ResolvGetAddrInfoTest, AlphabeticalHostname) {
		dns.addMapping(host_name, ns_type::ns_t_aaaa, v6addr);
		ASSERT_TRUE(dns.startServer());
		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		static const struct TestConfig {
		@@ -430,7 +430,7 @@ TEST_F(ResolvGetAddrInfoTest, IllegalHostname) {
		ASSERT_TRUE(dns.startServer());

		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		// Illegal hostname is verified by res_hnok() in system/netd/resolv/res_comp.cpp.
		@@ -497,7 +497,7 @@ TEST_F(ResolvGetAddrInfoTest, ServerResponseError) {
		dns.setResponseProbability(0.0); // always ignore requests and response preset rcode
		ASSERT_TRUE(dns.startServer());
		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		addrinfo* result = nullptr;
		@@ -519,7 +519,7 @@ TEST_F(ResolvGetAddrInfoTest, ServerTimeout) {
		dns.setResponseProbability(0.0); // always ignore requests and don't response
		ASSERT_TRUE(dns.startServer());
		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		addrinfo* result = nullptr;
		@@ -543,7 +543,7 @@ TEST_F(ResolvGetAddrInfoTest, CnamesNoIpAddress) {
		ASSERT_TRUE(dns.startServer());

		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		static const struct TestConfig {
		@@ -582,7 +582,7 @@ TEST_F(ResolvGetAddrInfoTest, CnamesBrokenChainByIllegalCname) {
		ASSERT_TRUE(dns.startServer());

		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		static const struct TestConfig {
		@@ -642,7 +642,7 @@ TEST_F(ResolvGetAddrInfoTest, CnamesInfiniteLoop) {
		ASSERT_TRUE(dns.startServer());

		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		for (const auto& family : {AF_INET, AF_INET6, AF_UNSPEC}) {
		@@ -670,7 +670,7 @@ TEST_F(GetHostByNameForNetContextTest, AlphabeticalHostname) {
		dns.addMapping(host_name, ns_type::ns_t_aaaa, v6addr);
		ASSERT_TRUE(dns.startServer());
		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		static const struct TestConfig {
		@@ -704,7 +704,7 @@ TEST_F(GetHostByNameForNetContextTest, IllegalHostname) {
		ASSERT_TRUE(dns.startServer());

		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		// Illegal hostname is verified by res_hnok() in system/netd/resolv/res_comp.cpp.
		@@ -749,7 +749,7 @@ TEST_F(GetHostByNameForNetContextTest, NoData) {
		dns.addMapping(v4_host_name, ns_type::ns_t_a, "1.2.3.3");
		ASSERT_TRUE(dns.startServer());
		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));
		dns.clearQueries();

		@@ -793,7 +793,7 @@ TEST_F(GetHostByNameForNetContextTest, ServerResponseError) {
		dns.setResponseProbability(0.0); // always ignore requests and response preset rcode
		ASSERT_TRUE(dns.startServer());
		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		hostent* hp = nullptr;
		@@ -814,7 +814,7 @@ TEST_F(GetHostByNameForNetContextTest, ServerTimeout) {
		dns.setResponseProbability(0.0); // always ignore requests and don't response
		ASSERT_TRUE(dns.startServer());
		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		hostent* hp = nullptr;
		@@ -836,7 +836,7 @@ TEST_F(GetHostByNameForNetContextTest, CnamesNoIpAddress) {
		ASSERT_TRUE(dns.startServer());

		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		static const struct TestConfig {
		@@ -870,7 +870,7 @@ TEST_F(GetHostByNameForNetContextTest, CnamesBrokenChainByIllegalCname) {
		ASSERT_TRUE(dns.startServer());

		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		static const struct TestConfig {
		@@ -929,7 +929,7 @@ TEST_F(GetHostByNameForNetContextTest, CnamesInfiniteLoop) {
		ASSERT_TRUE(dns.startServer());

		const char* servers[] = {listen_addr};
		ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),
		ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),
		mDefaultSearchDomains, &mDefaultParams_Binder));

		for (const auto& family : {AF_INET, AF_INET6}) {
		@@ -946,7 +946,7 @@ TEST_F(GetHostByNameForNetContextTest, CnamesInfiniteLoop) {
		// Note that local host file function, files_getaddrinfo(), of resolv_getaddrinfo()
		// is not tested because it only returns a boolean (success or failure) without any error number.

		// TODO: Simplify the DNS server configuration, DNSResponder and resolv_set_nameservers_for_net, as
		// TODO: Simplify the DNS server configuration, DNSResponder and resolv_set_nameservers, as
		// ResolverTest does.
		// TODO: Add test for resolv_getaddrinfo().
		// - DNS response message parsing.

res_cache.cpp

+42 −52

Original line number	Diff line number	Diff line
		@@ -35,7 +35,10 @@
		#include <stdlib.h>
		#include <string.h>
		#include <time.h>
		#include <algorithm>
		#include <mutex>
		#include <set>
		#include <vector>

		#include <arpa/inet.h>
		#include <arpa/nameser.h>
		@@ -46,6 +49,7 @@

		#include <android-base/logging.h>
		#include <android-base/parseint.h>
		#include <android-base/strings.h>
		#include <android-base/thread_annotations.h>
		#include <android/multinetwork.h> // ResNsendFlags

		@@ -1142,8 +1146,7 @@ struct resolv_cache_info {
		int revision_id; // # times the nameservers have been replaced
		res_params params;
		struct res_stats nsstats[MAXNS];
		char defdname[MAXDNSRCHPATH];
		int dnsrch_offset[MAXDNSRCH + 1]; // offsets into defdname
		std::vector<std::string> search_domains;
		int wait_for_pending_req_timeout_count;
		};

		@@ -1719,12 +1722,35 @@ static void resolv_set_experiment_params(res_params* params) {
		}
		}

		int resolv_set_nameservers_for_net(unsigned netid, const char** servers, const int numservers,
		int resolv_set_nameservers(unsigned netid, const char** servers, int numservers,
		const char* domains, const res_params* params) {
		char* cp;
		int* offset;
		struct addrinfo* nsaddrinfo[MAXNS];
		return resolv_set_nameservers(netid, servers, numservers, android::base::Split(domains, " "),
		params);
		}

		namespace {

		// Returns valid domains without duplicates which are limited to max size \|MAXDNSRCH\|.
		std::vector<std::string> filter_domains(const std::vector<std::string>& domains) {
		std::set<std::string> tmp_set;
		std::vector<std::string> res;

		std::copy_if(domains.begin(), domains.end(), std::back_inserter(res),
		[&tmp_set](const std::string& str) {
		return !(str.size() > MAXDNSRCHPATH - 1) && (tmp_set.insert(str).second);
		});
		if (res.size() > MAXDNSRCH) {
		LOG(WARNING) << __func__ << ": valid domains=" << res.size()
		<< ", but MAXDNSRCH=" << MAXDNSRCH;
		res.resize(MAXDNSRCH);
		}
		return res;
		}

		} // namespace

		int resolv_set_nameservers(unsigned netid, const char** servers, int numservers,
		const std::vector<std::string>& domains, const res_params* params) {
		if (numservers > MAXNS) {
		LOG(ERROR) << __func__ << ": numservers=" << numservers << ", MAXNS=" << MAXNS;
		return E2BIG;
		@@ -1732,6 +1758,8 @@ int resolv_set_nameservers_for_net(unsigned netid, const char** servers, const i

		// Parse the addresses before actually locking or changing any state, in case there is an error.
		// As a side effect this also reduces the time the lock is kept.
		// TODO: find a better way to replace addrinfo*, something like std::vector<SafeAddrinfo>
		addrinfo* nsaddrinfo[MAXNS];
		char sbuf[NI_MAXSERV];
		snprintf(sbuf, sizeof(sbuf), "%u", NAMESERVER_PORT);
		for (int i = 0; i < numservers; i++) {
		@@ -1795,31 +1823,9 @@ int resolv_set_nameservers_for_net(unsigned netid, const char** servers, const i
		}
		}

		// Always update the search paths, since determining whether they actually changed is
		// complex due to the zero-padding, and probably not worth the effort. Cache-flushing
		// however is not necessary, since the stored cache entries do contain the domain, not
		// just the host name.
		strlcpy(cache_info->defdname, domains, sizeof(cache_info->defdname));
		if ((cp = strchr(cache_info->defdname, '\n')) != NULL) *cp = '\0';
		LOG(INFO) << __func__ << ": domains=\"" << cache_info->defdname << "\"";

		cp = cache_info->defdname;
		offset = cache_info->dnsrch_offset;
		while (offset < cache_info->dnsrch_offset + MAXDNSRCH) {
		while (cp == ' ' \|\| cp == '\t') /* skip leading white space */
		cp++;
		if (cp == '\0') / stop if nothing more to do */
		break;
		offset++ = cp - cache_info->defdname; / record this search domain */
		while (cp) { / zero-terminate it */
		if (cp == ' ' \|\| cp == '\t') {
		*cp++ = '\0';
		break;
		}
		cp++;
		}
		}
		offset = -1; / cache_info->dnsrch_offset has MAXDNSRCH+1 items */
		// Always update the search paths. Cache-flushing however is not necessary,
		// since the stored cache entries do contain the domain, not just the host name.
		cache_info->search_domains = filter_domains(domains);

		return 0;
		}
		@@ -1899,15 +1905,7 @@ void _resolv_populate_res_for_net(res_state statp) {
		}
		}
		statp->nscount = nserv;
		// now do search domains. Note that we cache the offsets as this code runs alot
		// but the setting/offset-computer only runs when set/changed
		// WARNING: Don't use str*cpy() here, this string contains zeroes.
		memcpy(statp->defdname, info->defdname, sizeof(statp->defdname));
		char** pp = statp->dnsrch;
		int* p = info->dnsrch_offset;
		while (pp < statp->dnsrch + MAXDNSRCH && *p != -1) {
		pp++ = &statp->defdname[0] + p++;
		}
		statp->search_domains = info->search_domains;
		}
		}

		@@ -1979,17 +1977,9 @@ int android_net_res_stats_get_info_for_net(unsigned netid, int* nscount,
		memcpy(&servers[i], info->nsaddrinfo[i]->ai_addr, info->nsaddrinfo[i]->ai_addrlen);
		stats[i] = info->nsstats[i];
		}
		for (i = 0; i < MAXDNSRCH; i++) {
		const char* cur_domain = info->defdname + info->dnsrch_offset[i];
		// dnsrch_offset[i] can either be -1 or point to an empty string to indicate the end
		// of the search offsets. Checking for < 0 is not strictly necessary, but safer.
		// TODO: Pass in a search domain array instead of a string to
		// resolv_set_nameservers_for_net() and make this double check unnecessary.
		if (info->dnsrch_offset[i] < 0 \|\|
		((size_t) info->dnsrch_offset[i]) >= sizeof(info->defdname) \|\| !cur_domain[0]) {
		break;
		}
		strlcpy(domains[i], cur_domain, MAXDNSRCHPATH);

		for (i = 0; i < static_cast<int>(info->search_domains.size()); i++) {
		strlcpy(domains[i], info->search_domains[i].c_str(), MAXDNSRCHPATH);
		}
		*dcount = i;
		*params = info->params;

res_init.cpp

+0 −29

Original line number	Diff line number	Diff line
		@@ -125,11 +125,7 @@ int res_ninit(res_state statp) {

		/* This function has to be reachable by res_data.c but not publicly. */
		int res_vinit(res_state statp, int preinit) {
		char cp, *pp;
		char buf[BUFSIZ];
		int nserv = 0; /* number of nameserver records read from file */
		int havesearch = 0;
		int dots;
		sockaddr_union u[2];

		if ((statp->options & RES_INIT) != 0U) res_ndestroy(statp);
		@@ -162,31 +158,6 @@ int res_vinit(res_state statp, int preinit) {
		statp->nsort = 0;
		res_setservers(statp, u, nserv);

		if (statp->defdname[0] == 0 && gethostname(buf, sizeof(statp->defdname) - 1) == 0 &&
		(cp = strchr(buf, '.')) != NULL)
		strcpy(statp->defdname, cp + 1);

		/* find components of local domain that might be searched */
		if (havesearch == 0) {
		pp = statp->dnsrch;
		*pp++ = statp->defdname;
		*pp = NULL;

		dots = 0;
		for (cp = statp->defdname; cp; cp++) dots += (cp == '.');

		cp = statp->defdname;
		while (pp < statp->dnsrch + MAXDFLSRCH) {
		if (dots < LOCALDOMAINPARTS) break;
		cp = strchr(cp, '.') + 1; /* we know there is one */
		*pp++ = cp;
		dots--;
		}
		*pp = NULL;
		LOG(DEBUG) << __func__ << ": dnsrch list:";
		for (pp = statp->dnsrch; pp; pp++) LOG(DEBUG) << "\t" << pp;
		}

		if (nserv > 0) {
		statp->nscount = nserv;
		statp->options \|= RES_INIT;