[adb] Fix crash in mdns service removal.
If std::remove_if is a no-op (no service name match), then
std::vector::erase would crash.
Bug: 160025305
Test: test_adb.py
Test: Manual.
1) Enable wireless debugging on multiple devices
2) Disable wireless debugging on all devices
Change-Id: If58b0dda0bb698fd3fd225d9b6a6726a88ae47e0
diff --git a/adb/client/transport_mdns.cpp b/adb/client/transport_mdns.cpp
index 9611212..a0fc9ca 100644
--- a/adb/client/transport_mdns.cpp
+++ b/adb/client/transport_mdns.cpp
@@ -249,6 +249,9 @@
return false;
}
+ // Remove any services with the same instance name, as it may be a stale registration.
+ removeDNSService(regType_.c_str(), serviceName_.c_str());
+
// Add to the service registry before trying to auto-connect, since socket_spec_connect will
// check these registries for the ip address when connecting via mdns instance name.
int adbSecureServiceType = serviceIndex();
@@ -268,13 +271,6 @@
return false;
}
- if (!services->empty()) {
- // Remove the previous resolved service, if any.
- services->erase(std::remove_if(services->begin(), services->end(),
- [&](std::unique_ptr<ResolvedService>& service) {
- return (serviceName_ == service->serviceName());
- }));
- }
services->push_back(std::unique_ptr<ResolvedService>(this));
if (adb_DNSServiceShouldAutoConnect(regType_.c_str(), serviceName_.c_str())) {
@@ -327,6 +323,8 @@
static bool connectByServiceName(const ServiceRegistry& services,
const std::string& service_name);
+ static void removeDNSService(const char* regType, const char* serviceName);
+
private:
int clientVersion_ = ADB_SECURE_CLIENT_VERSION;
std::string addr_format_;
@@ -396,6 +394,37 @@
return false;
}
+// static
+void ResolvedService::removeDNSService(const char* regType, const char* serviceName) {
+ D("%s: regType=[%s] serviceName=[%s]", __func__, regType, serviceName);
+ int index = adb_DNSServiceIndexByName(regType);
+ ServiceRegistry* services;
+ switch (index) {
+ case kADBTransportServiceRefIndex:
+ services = sAdbTransportServices;
+ break;
+ case kADBSecurePairingServiceRefIndex:
+ services = sAdbSecurePairingServices;
+ break;
+ case kADBSecureConnectServiceRefIndex:
+ services = sAdbSecureConnectServices;
+ break;
+ default:
+ return;
+ }
+
+ if (services->empty()) {
+ return;
+ }
+
+ std::string sName(serviceName);
+ services->erase(std::remove_if(services->begin(), services->end(),
+ [&sName](std::unique_ptr<ResolvedService>& service) {
+ return (sName == service->serviceName());
+ }),
+ services->end());
+}
+
void adb_secure_foreach_pairing_service(const char* service_name,
adb_secure_foreach_service_callback cb) {
ResolvedService::forEachService(*ResolvedService::sAdbSecurePairingServices, service_name, cb);
@@ -481,35 +510,6 @@
std::string regType_;
};
-static void adb_RemoveDNSService(const char* regType, const char* serviceName) {
- D("%s: regType=[%s] serviceName=[%s]", __func__, regType, serviceName);
- int index = adb_DNSServiceIndexByName(regType);
- ResolvedService::ServiceRegistry* services;
- switch (index) {
- case kADBTransportServiceRefIndex:
- services = ResolvedService::sAdbTransportServices;
- break;
- case kADBSecurePairingServiceRefIndex:
- services = ResolvedService::sAdbSecurePairingServices;
- break;
- case kADBSecureConnectServiceRefIndex:
- services = ResolvedService::sAdbSecureConnectServices;
- break;
- default:
- return;
- }
-
- if (services->empty()) {
- return;
- }
-
- std::string sName(serviceName);
- services->erase(std::remove_if(services->begin(), services->end(),
- [&sName](std::unique_ptr<ResolvedService>& service) {
- return (sName == service->serviceName());
- }));
-}
-
// Returns the version the device wanted to advertise,
// or -1 if parsing fails.
static int parse_version_from_txt_record(uint16_t txtLen, const unsigned char* txtRecord) {
@@ -612,7 +612,7 @@
} else {
D("%s: Discover lost serviceName=[%s] regtype=[%s] domain=[%s]", __func__, serviceName,
regtype, domain);
- adb_RemoveDNSService(regtype, serviceName);
+ ResolvedService::removeDNSService(regtype, serviceName);
}
}
diff --git a/adb/test_adb.py b/adb/test_adb.py
index b9f0d54..a32d875 100755
--- a/adb/test_adb.py
+++ b/adb/test_adb.py
@@ -618,21 +618,37 @@
finally:
zeroconf_ctx.unregister_service(info)
+@contextlib.contextmanager
+def zeroconf_register_services(zeroconf_ctx, infos):
+ """Context manager for multiple zeroconf services
+
+ Registers all services given and unregisters all on cleanup. Returns the ServiceInfo
+ list supplied.
+ """
+
+ try:
+ for info in infos:
+ zeroconf_ctx.register_service(info)
+ yield infos
+ finally:
+ for info in infos:
+ zeroconf_ctx.unregister_service(info)
+
"""Should match the service names listed in adb_mdns.h"""
class MdnsTest:
"""Tests for adb mdns."""
+ @staticmethod
+ def _mdns_services(port):
+ output = subprocess.check_output(["adb", "-P", str(port), "mdns", "services"])
+ return [x.split("\t") for x in output.decode("utf8").strip().splitlines()[1:]]
+
+ @staticmethod
+ def _devices(port):
+ output = subprocess.check_output(["adb", "-P", str(port), "devices"])
+ return [x.split("\t") for x in output.decode("utf8").strip().splitlines()[1:]]
+
class Base(unittest.TestCase):
- @staticmethod
- def _mdns_services(port):
- output = subprocess.check_output(["adb", "-P", str(port), "mdns", "services"])
- return [x.split("\t") for x in output.decode("utf8").strip().splitlines()[1:]]
-
- @staticmethod
- def _devices(port):
- output = subprocess.check_output(["adb", "-P", str(port), "devices"])
- return [x.split("\t") for x in output.decode("utf8").strip().splitlines()[1:]]
-
@contextlib.contextmanager
def _adb_mdns_connect(self, server_port, mdns_instance, serial, should_connect):
"""Context manager for an ADB connection.
@@ -691,6 +707,50 @@
for line in MdnsTest._mdns_services(server_port)))
@unittest.skipIf(not is_zeroconf_installed(), "zeroconf library not installed")
+ def test_mdns_services_register_unregister_multiple(self):
+ """Ensure that `adb mdns services` correctly adds and removes multiple services
+ """
+ from zeroconf import IPVersion, ServiceInfo
+
+ with adb_server() as server_port:
+ output = subprocess.check_output(["adb", "-P", str(server_port),
+ "mdns", "services"]).strip()
+ self.assertTrue(output.startswith(b"List of discovered mdns services"))
+
+ """TODO(joshuaduong): Add ipv6 tests once we have it working in adb"""
+ """Register/Unregister a service"""
+ with zeroconf_context(IPVersion.V4Only) as zc:
+ srvs = {
+ 'mdns_name': ["testservice0", "testservice1", "testservice2"],
+ 'mdns_type': "_" + self.service_name + "._tcp.",
+ 'ipaddr': [
+ socket.inet_aton("192.168.0.1"),
+ socket.inet_aton("10.0.0.255"),
+ socket.inet_aton("172.16.1.100")],
+ 'port': [10000, 20000, 65535]}
+ srv_infos = []
+ for i in range(len(srvs['mdns_name'])):
+ srv_infos.append(ServiceInfo(
+ srvs['mdns_type'] + "local.",
+ name=srvs['mdns_name'][i] + "." + srvs['mdns_type'] + "local.",
+ addresses=[srvs['ipaddr'][i]],
+ port=srvs['port'][i]))
+
+ """ Register all devices, then unregister"""
+ with zeroconf_register_services(zc, srv_infos) as infos:
+ """Give adb some time to register the service"""
+ time.sleep(1)
+ for i in range(len(srvs['mdns_name'])):
+ self.assertTrue(any((srvs['mdns_name'][i] in line and srvs['mdns_type'] in line)
+ for line in MdnsTest._mdns_services(server_port)))
+
+ """Give adb some time to unregister the service"""
+ time.sleep(1)
+ for i in range(len(srvs['mdns_name'])):
+ self.assertFalse(any((srvs['mdns_name'][i] in line and srvs['mdns_type'] in line)
+ for line in MdnsTest._mdns_services(server_port)))
+
+ @unittest.skipIf(not is_zeroconf_installed(), "zeroconf library not installed")
def test_mdns_connect(self):
"""Ensure that `adb connect` by mdns instance name works (for non-pairing services)
"""