Ensure service type ends with a dot in discovery
Historical behavior of onServiceFound is to include a dot at the end of
the service type. To avoid inconsistencies between behavior of new and
older backend, which would be hard to predict/handle for apps, ensure
that the same format is used. This means a dot must be added when using
the new backend.
Implement this by creating the callback service types based on
MdnsServiceInfo obtained from MdnsDiscoveryManager, rather than the
service type that was provided by the calling app. This avoids
inconsistencies as the calling app may or may not include dots
originally, and it may use subtypes ("_sub._type._tcp.local" or even
"_type._tcp.local,_sub"), which would result in callbacks giving invalid
service types, or inconsistent with S/T behavior.
Fixes: 281776253
Test: atest
Change-Id: I677ee885d1ee8e6a09773f21e4dd53e96faee748
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());