diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java index 1e0d544b68..c136d4c69a 100644 --- a/service-t/src/com/android/server/NsdService.java +++ b/service-t/src/com/android/server/NsdService.java @@ -19,6 +19,7 @@ package com.android.server; import static android.net.ConnectivityManager.NETID_UNSET; import static android.net.nsd.NsdManager.MDNS_DISCOVERY_MANAGER_EVENT; import static android.net.nsd.NsdManager.MDNS_SERVICE_EVENT; +import static android.net.nsd.NsdManager.RESOLVE_SERVICE_SUCCEEDED; import static android.provider.DeviceConfig.NAMESPACE_TETHERING; import static com.android.modules.utils.build.SdkLevel.isAtLeastU; @@ -83,6 +84,7 @@ import java.net.NetworkInterface; import java.net.SocketException; import java.net.UnknownHostException; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -242,14 +244,14 @@ public class NsdService extends INsdManager.Stub { public void onServiceNameDiscovered(@NonNull MdnsServiceInfo serviceInfo) { mNsdStateMachine.sendMessage(MDNS_DISCOVERY_MANAGER_EVENT, mTransactionId, NsdManager.SERVICE_FOUND, - new MdnsEvent(mClientId, mReqServiceInfo.getServiceType(), serviceInfo)); + new MdnsEvent(mClientId, serviceInfo)); } @Override public void onServiceNameRemoved(@NonNull MdnsServiceInfo serviceInfo) { mNsdStateMachine.sendMessage(MDNS_DISCOVERY_MANAGER_EVENT, mTransactionId, NsdManager.SERVICE_LOST, - new MdnsEvent(mClientId, mReqServiceInfo.getServiceType(), serviceInfo)); + new MdnsEvent(mClientId, serviceInfo)); } } @@ -264,7 +266,7 @@ public class NsdService extends INsdManager.Stub { public void onServiceFound(MdnsServiceInfo serviceInfo) { mNsdStateMachine.sendMessage(MDNS_DISCOVERY_MANAGER_EVENT, mTransactionId, NsdManager.RESOLVE_SERVICE_SUCCEEDED, - new MdnsEvent(mClientId, mReqServiceInfo.getServiceType(), serviceInfo)); + new MdnsEvent(mClientId, serviceInfo)); } } @@ -279,21 +281,21 @@ public class NsdService extends INsdManager.Stub { public void onServiceFound(@NonNull MdnsServiceInfo serviceInfo) { mNsdStateMachine.sendMessage(MDNS_DISCOVERY_MANAGER_EVENT, mTransactionId, NsdManager.SERVICE_UPDATED, - new MdnsEvent(mClientId, mReqServiceInfo.getServiceType(), serviceInfo)); + new MdnsEvent(mClientId, serviceInfo)); } @Override public void onServiceUpdated(@NonNull MdnsServiceInfo serviceInfo) { mNsdStateMachine.sendMessage(MDNS_DISCOVERY_MANAGER_EVENT, mTransactionId, NsdManager.SERVICE_UPDATED, - new MdnsEvent(mClientId, mReqServiceInfo.getServiceType(), serviceInfo)); + new MdnsEvent(mClientId, serviceInfo)); } @Override public void onServiceRemoved(@NonNull MdnsServiceInfo serviceInfo) { mNsdStateMachine.sendMessage(MDNS_DISCOVERY_MANAGER_EVENT, mTransactionId, NsdManager.SERVICE_UPDATED_LOST, - new MdnsEvent(mClientId, mReqServiceInfo.getServiceType(), serviceInfo)); + new MdnsEvent(mClientId, serviceInfo)); } } @@ -303,14 +305,10 @@ public class NsdService extends INsdManager.Stub { private static class MdnsEvent { final int mClientId; @NonNull - final String mRequestedServiceType; - @NonNull final MdnsServiceInfo mMdnsServiceInfo; - MdnsEvent(int clientId, @NonNull String requestedServiceType, - @NonNull MdnsServiceInfo mdnsServiceInfo) { + MdnsEvent(int clientId, @NonNull MdnsServiceInfo mdnsServiceInfo) { mClientId = clientId; - mRequestedServiceType = requestedServiceType; mMdnsServiceInfo = mdnsServiceInfo; } } @@ -1104,9 +1102,38 @@ public class NsdService extends INsdManager.Stub { return true; } - private NsdServiceInfo buildNsdServiceInfoFromMdnsEvent(final MdnsEvent event) { + @Nullable + private NsdServiceInfo buildNsdServiceInfoFromMdnsEvent( + final MdnsEvent event, int code) { final MdnsServiceInfo serviceInfo = event.mMdnsServiceInfo; - final String serviceType = event.mRequestedServiceType; + final String[] typeArray = serviceInfo.getServiceType(); + final String joinedType; + if (typeArray.length == 0 + || !typeArray[typeArray.length - 1].equals(LOCAL_DOMAIN_NAME)) { + Log.wtf(TAG, "MdnsServiceInfo type does not end in .local: " + + Arrays.toString(typeArray)); + return null; + } else { + joinedType = TextUtils.join(".", + Arrays.copyOfRange(typeArray, 0, typeArray.length - 1)); + } + final String serviceType; + switch (code) { + case NsdManager.SERVICE_FOUND: + case NsdManager.SERVICE_LOST: + // For consistency with historical behavior, discovered service types have + // a dot at the end. + serviceType = joinedType + "."; + break; + case RESOLVE_SERVICE_SUCCEEDED: + // For consistency with historical behavior, resolved service types have + // a dot at the beginning. + serviceType = "." + joinedType; + break; + default: + serviceType = joinedType; + break; + } final String serviceName = serviceInfo.getServiceInstanceName(); final NsdServiceInfo servInfo = new NsdServiceInfo(serviceName, serviceType); final Network network = serviceInfo.getNetwork(); @@ -1131,7 +1158,9 @@ public class NsdService extends INsdManager.Stub { final MdnsEvent event = (MdnsEvent) obj; final int clientId = event.mClientId; - final NsdServiceInfo info = buildNsdServiceInfoFromMdnsEvent(event); + final NsdServiceInfo info = buildNsdServiceInfoFromMdnsEvent(event, code); + // Errors are already logged if null + if (info == null) return false; if (DBG) { Log.d(TAG, String.format("MdnsDiscoveryManager event code=%s transactionId=%d", NsdManager.nameOf(code), transactionId)); @@ -1150,8 +1179,6 @@ public class NsdService extends INsdManager.Stub { break; } final MdnsServiceInfo serviceInfo = event.mMdnsServiceInfo; - // Add '.' in front of the service type that aligns with historical behavior - info.setServiceType("." + event.mRequestedServiceType); info.setPort(serviceInfo.getPort()); Map attrs = serviceInfo.getAttributes(); diff --git a/tests/cts/net/src/android/net/cts/NsdManagerTest.kt b/tests/cts/net/src/android/net/cts/NsdManagerTest.kt index db7f38cb79..88b9baff1e 100644 --- a/tests/cts/net/src/android/net/cts/NsdManagerTest.kt +++ b/tests/cts/net/src/android/net/cts/NsdManagerTest.kt @@ -282,13 +282,17 @@ class NsdManagerTest { fun waitForServiceDiscovered( serviceName: String, + serviceType: String, expectedNetwork: Network? = null ): NsdServiceInfo { - return expectCallbackEventually { + val serviceFound = expectCallbackEventually { it.serviceInfo.serviceName == serviceName && (expectedNetwork == null || expectedNetwork == nsdShim.getNetwork(it.serviceInfo)) }.serviceInfo + // Discovered service types have a dot at the end + assertEquals("$serviceType.", serviceFound.serviceType) + return serviceFound } } @@ -497,6 +501,10 @@ class NsdManagerTest { val registeredInfo = registrationRecord.expectCallback( REGISTRATION_TIMEOUT_MS).serviceInfo + // Only service name is included in ServiceRegistered callbacks + assertNull(registeredInfo.serviceType) + assertEquals(si.serviceName, registeredInfo.serviceName) + val discoveryRecord = NsdDiscoveryRecord() // Test discovering without an Executor nsdManager.discoverServices(serviceType, NsdManager.PROTOCOL_DNS_SD, discoveryRecord) @@ -505,12 +513,15 @@ class NsdManagerTest { discoveryRecord.expectCallback() // Expect a service record to be discovered - val foundInfo = discoveryRecord.waitForServiceDiscovered(registeredInfo.serviceName) + val foundInfo = discoveryRecord.waitForServiceDiscovered( + registeredInfo.serviceName, serviceType) // Test resolving without an Executor val resolveRecord = NsdResolveRecord() nsdManager.resolveService(foundInfo, resolveRecord) val resolvedService = resolveRecord.expectCallback().serviceInfo + assertEquals(".$serviceType", resolvedService.serviceType) + assertEquals(registeredInfo.serviceName, resolvedService.serviceName) // Check Txt attributes assertEquals(8, resolvedService.attributes.size) @@ -538,9 +549,11 @@ class NsdManagerTest { registrationRecord.expectCallback() // Expect a callback for service lost - discoveryRecord.expectCallbackEventually { + val lostCb = discoveryRecord.expectCallbackEventually { it.serviceInfo.serviceName == serviceName } + // Lost service types have a dot at the end + assertEquals("$serviceType.", lostCb.serviceInfo.serviceType) // Register service again to see if NsdManager can discover it val si2 = NsdServiceInfo() @@ -554,7 +567,8 @@ class NsdManagerTest { // Expect a service record to be discovered (and filter the ones // that are unrelated to this test) - val foundInfo2 = discoveryRecord.waitForServiceDiscovered(registeredInfo2.serviceName) + val foundInfo2 = discoveryRecord.waitForServiceDiscovered( + registeredInfo2.serviceName, serviceType) // Resolve the service val resolveRecord2 = NsdResolveRecord() @@ -591,7 +605,7 @@ class NsdManagerTest { testNetwork1.network, Executor { it.run() }, discoveryRecord) val foundInfo = discoveryRecord.waitForServiceDiscovered( - serviceName, testNetwork1.network) + serviceName, serviceType, testNetwork1.network) assertEquals(testNetwork1.network, nsdShim.getNetwork(foundInfo)) // Rewind to ensure the service is not found on the other interface @@ -638,6 +652,8 @@ class NsdManagerTest { val serviceDiscovered = discoveryRecord.expectCallback() assertEquals(registeredInfo1.serviceName, serviceDiscovered.serviceInfo.serviceName) + // Discovered service types have a dot at the end + assertEquals("$serviceType.", serviceDiscovered.serviceInfo.serviceType) assertEquals(testNetwork1.network, nsdShim.getNetwork(serviceDiscovered.serviceInfo)) // Unregister, then register the service back: it should be lost and found again @@ -650,6 +666,7 @@ class NsdManagerTest { val registeredInfo2 = registerService(registrationRecord, si, executor) val serviceDiscovered2 = discoveryRecord.expectCallback() assertEquals(registeredInfo2.serviceName, serviceDiscovered2.serviceInfo.serviceName) + assertEquals("$serviceType.", serviceDiscovered2.serviceInfo.serviceType) assertEquals(testNetwork1.network, nsdShim.getNetwork(serviceDiscovered2.serviceInfo)) // Teardown, then bring back up a network on the test interface: the service should @@ -665,6 +682,7 @@ class NsdManagerTest { val newNetwork = newAgent.network ?: fail("Registered agent should have a network") val serviceDiscovered3 = discoveryRecord.expectCallback() assertEquals(registeredInfo2.serviceName, serviceDiscovered3.serviceInfo.serviceName) + assertEquals("$serviceType.", serviceDiscovered3.serviceInfo.serviceType) assertEquals(newNetwork, nsdShim.getNetwork(serviceDiscovered3.serviceInfo)) } cleanupStep { nsdManager.stopServiceDiscovery(discoveryRecord) @@ -740,12 +758,12 @@ class NsdManagerTest { nsdManager.discoverServices(serviceType, NsdManager.PROTOCOL_DNS_SD, discoveryRecord) val foundInfo1 = discoveryRecord.waitForServiceDiscovered( - serviceName, testNetwork1.network) + serviceName, serviceType, testNetwork1.network) assertEquals(testNetwork1.network, nsdShim.getNetwork(foundInfo1)) // Rewind as the service could be found on each interface in any order discoveryRecord.nextEvents.rewind(0) val foundInfo2 = discoveryRecord.waitForServiceDiscovered( - serviceName, testNetwork2.network) + serviceName, serviceType, testNetwork2.network) assertEquals(testNetwork2.network, nsdShim.getNetwork(foundInfo2)) nsdShim.resolveService(nsdManager, foundInfo1, Executor { it.run() }, resolveRecord) @@ -790,7 +808,7 @@ class NsdManagerTest { testNetwork1.network, Executor { it.run() }, discoveryRecord) // Expect that service is found on testNetwork1 val foundInfo = discoveryRecord.waitForServiceDiscovered( - serviceName, testNetwork1.network) + serviceName, serviceType, testNetwork1.network) assertEquals(testNetwork1.network, nsdShim.getNetwork(foundInfo)) // Discover service on testNetwork2. @@ -805,7 +823,7 @@ class NsdManagerTest { null as Network? /* network */, Executor { it.run() }, discoveryRecord3) // Expect that service is found on testNetwork1 val foundInfo3 = discoveryRecord3.waitForServiceDiscovered( - serviceName, testNetwork1.network) + serviceName, serviceType, testNetwork1.network) assertEquals(testNetwork1.network, nsdShim.getNetwork(foundInfo3)) } cleanupStep { nsdManager.stopServiceDiscovery(discoveryRecord2) @@ -835,7 +853,7 @@ class NsdManagerTest { nsdManager.discoverServices( serviceType, NsdManager.PROTOCOL_DNS_SD, discoveryRecord ) - val foundInfo = discoveryRecord.waitForServiceDiscovered(serviceNames) + val foundInfo = discoveryRecord.waitForServiceDiscovered(serviceNames, serviceType) // Expect that resolving the service name works properly even service name contains // non-standard characters. @@ -985,7 +1003,7 @@ class NsdManagerTest { nsdShim.discoverServices(nsdManager, serviceType, NsdManager.PROTOCOL_DNS_SD, testNetwork1.network, Executor { it.run() }, discoveryRecord) val foundInfo = discoveryRecord.waitForServiceDiscovered( - serviceName, testNetwork1.network) + serviceName, serviceType, testNetwork1.network) // Register service callback and check the addresses are the same as network addresses nsdShim.registerServiceInfoCallback(nsdManager, foundInfo, { it.run() }, cbRecord) diff --git a/tests/unit/java/com/android/server/NsdServiceTest.java b/tests/unit/java/com/android/server/NsdServiceTest.java index 14b4280029..5ca393475f 100644 --- a/tests/unit/java/com/android/server/NsdServiceTest.java +++ b/tests/unit/java/com/android/server/NsdServiceTest.java @@ -908,7 +908,8 @@ public class NsdServiceTest { listener.onServiceNameDiscovered(foundInfo); verify(discListener, timeout(TIMEOUT_MS)).onServiceFound(argThat(info -> info.getServiceName().equals(SERVICE_NAME) - && info.getServiceType().equals(SERVICE_TYPE) + // Service type in discovery callbacks has a dot at the end + && info.getServiceType().equals(SERVICE_TYPE + ".") && info.getNetwork().equals(network))); final MdnsServiceInfo removedInfo = new MdnsServiceInfo( @@ -927,7 +928,8 @@ public class NsdServiceTest { listener.onServiceNameRemoved(removedInfo); verify(discListener, timeout(TIMEOUT_MS)).onServiceLost(argThat(info -> info.getServiceName().equals(SERVICE_NAME) - && info.getServiceType().equals(SERVICE_TYPE) + // Service type in discovery callbacks has a dot at the end + && info.getServiceType().equals(SERVICE_TYPE + ".") && info.getNetwork().equals(network))); client.stopServiceDiscovery(discListener); @@ -982,6 +984,8 @@ public class NsdServiceTest { client.resolveService(request, resolveListener); waitForIdle(); verify(mSocketProvider).startMonitoringSockets(); + // TODO(b/266167702): this is a bug, as registerListener should be done _service._tcp, and + // _sub should be in the list of subtypes in the options. verify(mDiscoveryManager).registerListener(eq(constructedServiceType), listenerCaptor.capture(), argThat(options -> network.equals(options.getNetwork()) @@ -1009,7 +1013,8 @@ public class NsdServiceTest { verify(resolveListener, timeout(TIMEOUT_MS)).onServiceResolved(infoCaptor.capture()); final NsdServiceInfo info = infoCaptor.getValue(); assertEquals(SERVICE_NAME, info.getServiceName()); - assertEquals("." + serviceType, info.getServiceType()); + // TODO(b/266167702): this should be ._service._tcp (as per legacy behavior) + assertEquals("._nsd._sub._service._tcp", info.getServiceType()); assertEquals(PORT, info.getPort()); assertTrue(info.getAttributes().containsKey("key")); assertEquals(1, info.getAttributes().size());