Merge "Ensure service type ends with a dot in discovery" into udc-dev
diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java
index 1e0d544..c136d4c 100644
--- a/service-t/src/com/android/server/NsdService.java
+++ b/service-t/src/com/android/server/NsdService.java
@@ -19,6 +19,7 @@
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.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 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 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 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 @@
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 @@
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 @@
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 @@
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<String, String> 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 db7f38c..88b9baf 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 @@
fun waitForServiceDiscovered(
serviceName: String,
+ serviceType: String,
expectedNetwork: Network? = null
): NsdServiceInfo {
- return expectCallbackEventually<ServiceFound> {
+ val serviceFound = expectCallbackEventually<ServiceFound> {
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 @@
val registeredInfo = registrationRecord.expectCallback<ServiceRegistered>(
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 @@
discoveryRecord.expectCallback<DiscoveryStarted>()
// 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<ServiceResolved>().serviceInfo
+ assertEquals(".$serviceType", resolvedService.serviceType)
+ assertEquals(registeredInfo.serviceName, resolvedService.serviceName)
// Check Txt attributes
assertEquals(8, resolvedService.attributes.size)
@@ -538,9 +549,11 @@
registrationRecord.expectCallback<ServiceUnregistered>()
// Expect a callback for service lost
- discoveryRecord.expectCallbackEventually<ServiceLost> {
+ val lostCb = discoveryRecord.expectCallbackEventually<ServiceLost> {
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 @@
// 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 @@
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 @@
val serviceDiscovered = discoveryRecord.expectCallback<ServiceFound>()
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 @@
val registeredInfo2 = registerService(registrationRecord, si, executor)
val serviceDiscovered2 = discoveryRecord.expectCallback<ServiceFound>()
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 @@
val newNetwork = newAgent.network ?: fail("Registered agent should have a network")
val serviceDiscovered3 = discoveryRecord.expectCallback<ServiceFound>()
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 @@
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 @@
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 @@
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 @@
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 @@
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 14b4280..5ca3934 100644
--- a/tests/unit/java/com/android/server/NsdServiceTest.java
+++ b/tests/unit/java/com/android/server/NsdServiceTest.java
@@ -908,7 +908,8 @@
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 @@
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 @@
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 @@
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());