Implement proper subtype advertising

Implement subtype advertising by advertising an additional PTR record if subtype advertising was requested. For an advertiser a subtype is just an additional PTR record, so this just involves plumbing the subtype down to MdnsRecordRepository, which includes the additional record in the service registration. Bug: 266167702 Test: atest Change-Id: I09e780af25149162f16bd75410ddc50f160a0dab
2023-05-11 20:42:26 +09:00
parent f2d064112c
commit ce44beb7aa
8 changed files with 183 additions and 71 deletions
--- a/tests/unit/java/com/android/server/NsdServiceTest.java
+++ b/tests/unit/java/com/android/server/NsdServiceTest.java
@@ -985,10 +985,9 @@ public class NsdServiceTest {
        final RegistrationListener regListener = mock(RegistrationListener.class);
        client.registerService(regInfo, NsdManager.PROTOCOL_DNS_SD, Runnable::run, regListener);
        waitForIdle();
-        // TODO: also pass the subtype to MdnsAdvertiser
        verify(mAdvertiser).addService(anyInt(), argThat(s ->
                "Instance".equals(s.getServiceName())
-                        && SERVICE_TYPE.equals(s.getServiceType())));
+                        && SERVICE_TYPE.equals(s.getServiceType())), eq("_subtype"));

        final DiscoveryListener discListener = mock(DiscoveryListener.class);
        client.discoverServices(typeWithSubtype, PROTOCOL, network, Runnable::run, discListener);
@@ -1090,7 +1089,7 @@ public class NsdServiceTest {

        final ArgumentCaptor<Integer> serviceIdCaptor = ArgumentCaptor.forClass(Integer.class);
        verify(mAdvertiser).addService(serviceIdCaptor.capture(),
-                argThat(info -> matches(info, regInfo)));
+                argThat(info -> matches(info, regInfo)), eq(null) /* subtype */);

        client.unregisterService(regListenerWithoutFeature);
        waitForIdle();
@@ -1147,8 +1146,10 @@ public class NsdServiceTest {
        waitForIdle();

        // The advertiser is enabled for _type2 but not _type1
-        verify(mAdvertiser, never()).addService(anyInt(), argThat(info -> matches(info, service1)));
-        verify(mAdvertiser).addService(anyInt(), argThat(info -> matches(info, service2)));
+        verify(mAdvertiser, never()).addService(
+                anyInt(), argThat(info -> matches(info, service1)), eq(null) /* subtype */);
+        verify(mAdvertiser).addService(
+                anyInt(), argThat(info -> matches(info, service2)), eq(null) /* subtype */);
    }

    @Test
@@ -1173,7 +1174,7 @@ public class NsdServiceTest {
        verify(mSocketProvider).startMonitoringSockets();
        final ArgumentCaptor<Integer> idCaptor = ArgumentCaptor.forClass(Integer.class);
        verify(mAdvertiser).addService(idCaptor.capture(), argThat(info ->
-                matches(info, regInfo)));
+                matches(info, regInfo)), eq(null) /* subtype */);

        // Verify onServiceRegistered callback
        final MdnsAdvertiser.AdvertiserCallback cb = cbCaptor.getValue();
@@ -1209,7 +1210,7 @@ public class NsdServiceTest {

        client.registerService(regInfo, NsdManager.PROTOCOL_DNS_SD, Runnable::run, regListener);
        waitForIdle();
-        verify(mAdvertiser, never()).addService(anyInt(), any());
+        verify(mAdvertiser, never()).addService(anyInt(), any(), any());

        verify(regListener, timeout(TIMEOUT_MS)).onRegistrationFailed(
                argThat(info -> matches(info, regInfo)), eq(FAILURE_INTERNAL_ERROR));
@@ -1237,7 +1238,8 @@ public class NsdServiceTest {
        final ArgumentCaptor<Integer> idCaptor = ArgumentCaptor.forClass(Integer.class);
        // Service name is truncated to 63 characters
        verify(mAdvertiser).addService(idCaptor.capture(),
-                argThat(info -> info.getServiceName().equals("a".repeat(63))));
+                argThat(info -> info.getServiceName().equals("a".repeat(63))),
+                eq(null) /* subtype */);

        // Verify onServiceRegistered callback
        final MdnsAdvertiser.AdvertiserCallback cb = cbCaptor.getValue();
@@ -1319,7 +1321,7 @@ public class NsdServiceTest {
        client.registerService(regInfo, NsdManager.PROTOCOL_DNS_SD, Runnable::run, regListener);
        waitForIdle();
        verify(mSocketProvider).startMonitoringSockets();
-        verify(mAdvertiser).addService(anyInt(), any());
+        verify(mAdvertiser).addService(anyInt(), any(), any());

        // Verify the discovery uses MdnsDiscoveryManager
        final DiscoveryListener discListener = mock(DiscoveryListener.class);
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt
@@ -57,6 +57,7 @@ private val TEST_LINKADDR = LinkAddress(TEST_ADDR, 64 /* prefixLength */)
 private val TEST_NETWORK_1 = mock(Network::class.java)
 private val TEST_NETWORK_2 = mock(Network::class.java)
 private val TEST_HOSTNAME = arrayOf("Android_test", "local")
+private const val TEST_SUBTYPE = "_subtype"

 private val SERVICE_1 = NsdServiceInfo("TestServiceName", "_advertisertest._tcp").apply {
    port = 12345
@@ -130,7 +131,7 @@ class MdnsAdvertiserTest {
    @Test
    fun testAddService_OneNetwork() {
        val advertiser = MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog)
-        postSync { advertiser.addService(SERVICE_ID_1, SERVICE_1) }
+        postSync { advertiser.addService(SERVICE_ID_1, SERVICE_1, null /* subtype */) }

        val socketCbCaptor = ArgumentCaptor.forClass(SocketCallback::class.java)
        verify(socketProvider).requestSocket(eq(TEST_NETWORK_1), socketCbCaptor.capture())
@@ -161,7 +162,7 @@ class MdnsAdvertiserTest {
    @Test
    fun testAddService_AllNetworks() {
        val advertiser = MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog)
-        postSync { advertiser.addService(SERVICE_ID_1, ALL_NETWORKS_SERVICE) }
+        postSync { advertiser.addService(SERVICE_ID_1, ALL_NETWORKS_SERVICE, TEST_SUBTYPE) }

        val socketCbCaptor = ArgumentCaptor.forClass(SocketCallback::class.java)
        verify(socketProvider).requestSocket(eq(ALL_NETWORKS_SERVICE.network),
@@ -179,6 +180,10 @@ class MdnsAdvertiserTest {
        verify(mockDeps).makeAdvertiser(eq(mockSocket2), eq(listOf(TEST_LINKADDR)),
                eq(thread.looper), any(), intAdvCbCaptor2.capture(), eq(TEST_HOSTNAME), any()
        )
+        verify(mockInterfaceAdvertiser1).addService(
+                anyInt(), eq(ALL_NETWORKS_SERVICE), eq(TEST_SUBTYPE))
+        verify(mockInterfaceAdvertiser2).addService(
+                anyInt(), eq(ALL_NETWORKS_SERVICE), eq(TEST_SUBTYPE))

        doReturn(false).`when`(mockInterfaceAdvertiser1).isProbing(SERVICE_ID_1)
        postSync { intAdvCbCaptor1.value.onRegisterServiceSucceeded(
@@ -207,20 +212,21 @@ class MdnsAdvertiserTest {
    @Test
    fun testAddService_Conflicts() {
        val advertiser = MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog)
-        postSync { advertiser.addService(SERVICE_ID_1, SERVICE_1) }
+        postSync { advertiser.addService(SERVICE_ID_1, SERVICE_1, null /* subtype */) }

        val oneNetSocketCbCaptor = ArgumentCaptor.forClass(SocketCallback::class.java)
        verify(socketProvider).requestSocket(eq(TEST_NETWORK_1), oneNetSocketCbCaptor.capture())
        val oneNetSocketCb = oneNetSocketCbCaptor.value

        // Register a service with the same name on all networks (name conflict)
-        postSync { advertiser.addService(SERVICE_ID_2, ALL_NETWORKS_SERVICE) }
+        postSync { advertiser.addService(SERVICE_ID_2, ALL_NETWORKS_SERVICE, null /* subtype */) }
        val allNetSocketCbCaptor = ArgumentCaptor.forClass(SocketCallback::class.java)
        verify(socketProvider).requestSocket(eq(null), allNetSocketCbCaptor.capture())
        val allNetSocketCb = allNetSocketCbCaptor.value

-        postSync { advertiser.addService(LONG_SERVICE_ID_1, LONG_SERVICE_1) }
-        postSync { advertiser.addService(LONG_SERVICE_ID_2, LONG_ALL_NETWORKS_SERVICE) }
+        postSync { advertiser.addService(LONG_SERVICE_ID_1, LONG_SERVICE_1, null /* subtype */) }
+        postSync { advertiser.addService(LONG_SERVICE_ID_2, LONG_ALL_NETWORKS_SERVICE,
+                null /* subtype */) }

        // Callbacks for matching network and all networks both get the socket
        postSync {
@@ -248,13 +254,13 @@ class MdnsAdvertiserTest {
                eq(thread.looper), any(), intAdvCbCaptor.capture(), eq(TEST_HOSTNAME), any()
        )
        verify(mockInterfaceAdvertiser1).addService(eq(SERVICE_ID_1),
-                argThat { it.matches(SERVICE_1) })
+                argThat { it.matches(SERVICE_1) }, eq(null))
        verify(mockInterfaceAdvertiser1).addService(eq(SERVICE_ID_2),
-                argThat { it.matches(expectedRenamed) })
+                argThat { it.matches(expectedRenamed) }, eq(null))
        verify(mockInterfaceAdvertiser1).addService(eq(LONG_SERVICE_ID_1),
-                argThat { it.matches(LONG_SERVICE_1) })
+                argThat { it.matches(LONG_SERVICE_1) }, eq(null))
        verify(mockInterfaceAdvertiser1).addService(eq(LONG_SERVICE_ID_2),
-            argThat { it.matches(expectedLongRenamed) })
+            argThat { it.matches(expectedLongRenamed) }, eq(null))

        doReturn(false).`when`(mockInterfaceAdvertiser1).isProbing(SERVICE_ID_1)
        postSync { intAdvCbCaptor.value.onRegisterServiceSucceeded(
@@ -278,7 +284,7 @@ class MdnsAdvertiserTest {
    fun testRemoveService_whenAllServiceRemoved_thenUpdateHostName() {
        val advertiser = MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog)
        verify(mockDeps, times(1)).generateHostname()
-        postSync { advertiser.addService(SERVICE_ID_1, SERVICE_1) }
+        postSync { advertiser.addService(SERVICE_ID_1, SERVICE_1, null /* subtype */) }
        postSync { advertiser.removeService(SERVICE_ID_1) }
        verify(mockDeps, times(2)).generateHostname()
    }
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiserTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiserTest.kt
@@ -117,7 +117,7 @@ class MdnsInterfaceAdvertiserTest {
            knownServices.add(inv.getArgument(0))

            -1
-        }.`when`(repository).addService(anyInt(), any())
+        }.`when`(repository).addService(anyInt(), any(), any())
        doAnswer { inv ->
            knownServices.remove(inv.getArgument(0))
            null
@@ -278,8 +278,8 @@ class MdnsInterfaceAdvertiserTest {
        doReturn(serviceId).`when`(testProbingInfo).serviceId
        doReturn(testProbingInfo).`when`(repository).setServiceProbing(serviceId)

-        advertiser.addService(serviceId, serviceInfo)
-        verify(repository).addService(serviceId, serviceInfo)
+        advertiser.addService(serviceId, serviceInfo, null /* subtype */)
+        verify(repository).addService(serviceId, serviceInfo, null /* subtype */)
        verify(prober).startProbing(testProbingInfo)

        // Simulate probing success: continues to announcing
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
@@ -44,6 +44,7 @@ import org.junit.runner.RunWith
 private const val TEST_SERVICE_ID_1 = 42
 private const val TEST_SERVICE_ID_2 = 43
 private const val TEST_PORT = 12345
+private const val TEST_SUBTYPE = "_subtype"
 private val TEST_HOSTNAME = arrayOf("Android_000102030405060708090A0B0C0D0E0F", "local")
 private val TEST_ADDRESSES = listOf(
        LinkAddress(parseNumericAddress("192.0.2.111"), 24),
@@ -86,7 +87,8 @@ class MdnsRecordRepositoryTest {
    fun testAddServiceAndProbe() {
        val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME)
        assertEquals(0, repository.servicesCount)
-        assertEquals(-1, repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1))
+        assertEquals(-1, repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1,
+                null /* subtype */))
        assertEquals(1, repository.servicesCount)

        val probingInfo = repository.setServiceProbing(TEST_SERVICE_ID_1)
@@ -118,18 +120,18 @@ class MdnsRecordRepositoryTest {
    @Test
    fun testAddAndConflicts() {
        val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME)
-        repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1)
+        repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1, null /* subtype */)
        assertFailsWith(NameConflictException::class) {
-            repository.addService(TEST_SERVICE_ID_2, TEST_SERVICE_1)
+            repository.addService(TEST_SERVICE_ID_2, TEST_SERVICE_1, null /* subtype */)
        }
    }

    @Test
    fun testInvalidReuseOfServiceId() {
        val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME)
-        repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1)
+        repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1, null /* subtype */)
        assertFailsWith(IllegalArgumentException::class) {
-            repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_2)
+            repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_2, null /* subtype */)
        }
    }

@@ -138,7 +140,7 @@ class MdnsRecordRepositoryTest {
        val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME)
        assertFalse(repository.hasActiveService(TEST_SERVICE_ID_1))

-        repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1)
+        repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1, null /* subtype */)
        assertTrue(repository.hasActiveService(TEST_SERVICE_ID_1))

        val probingInfo = repository.setServiceProbing(TEST_SERVICE_ID_1)
@@ -179,6 +181,41 @@ class MdnsRecordRepositoryTest {
        assertEquals(0, repository.servicesCount)
    }

+    @Test
+    fun testExitAnnouncements_WithSubtype() {
+        val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME)
+        repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1, TEST_SUBTYPE)
+        repository.onAdvertisementSent(TEST_SERVICE_ID_1)
+
+        val exitAnnouncement = repository.exitService(TEST_SERVICE_ID_1)
+        assertNotNull(exitAnnouncement)
+        assertEquals(1, repository.servicesCount)
+        val packet = exitAnnouncement.getPacket(0)
+
+        assertEquals(0x8400 /* response, authoritative */, packet.flags)
+        assertEquals(0, packet.questions.size)
+        assertEquals(0, packet.authorityRecords.size)
+        assertEquals(0, packet.additionalRecords.size)
+
+        assertContentEquals(listOf(
+                MdnsPointerRecord(
+                        arrayOf("_testservice", "_tcp", "local"),
+                        0L /* receiptTimeMillis */,
+                        true /* cacheFlush */,
+                        0L /* ttlMillis */,
+                        arrayOf("MyTestService", "_testservice", "_tcp", "local")),
+                MdnsPointerRecord(
+                        arrayOf("_subtype", "_sub", "_testservice", "_tcp", "local"),
+                        0L /* receiptTimeMillis */,
+                        true /* cacheFlush */,
+                        0L /* ttlMillis */,
+                        arrayOf("MyTestService", "_testservice", "_tcp", "local")),
+        ), packet.answers)
+
+        repository.removeService(TEST_SERVICE_ID_1)
+        assertEquals(0, repository.servicesCount)
+    }
+
    @Test
    fun testExitingServiceReAdded() {
        val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME)
@@ -186,7 +223,8 @@ class MdnsRecordRepositoryTest {
        repository.onAdvertisementSent(TEST_SERVICE_ID_1)
        repository.exitService(TEST_SERVICE_ID_1)

-        assertEquals(TEST_SERVICE_ID_1, repository.addService(TEST_SERVICE_ID_2, TEST_SERVICE_1))
+        assertEquals(TEST_SERVICE_ID_1,
+                repository.addService(TEST_SERVICE_ID_2, TEST_SERVICE_1, null /* subtype */))
        assertEquals(1, repository.servicesCount)

        repository.removeService(TEST_SERVICE_ID_2)
@@ -196,7 +234,8 @@ class MdnsRecordRepositoryTest {
    @Test
    fun testOnProbingSucceeded() {
        val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME)
-        val announcementInfo = repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1)
+        val announcementInfo = repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1,
+                TEST_SUBTYPE)
        repository.onAdvertisementSent(TEST_SERVICE_ID_1)
        val packet = announcementInfo.getPacket(0)

@@ -205,6 +244,7 @@ class MdnsRecordRepositoryTest {
        assertEquals(0, packet.authorityRecords.size)

        val serviceType = arrayOf("_testservice", "_tcp", "local")
+        val serviceSubtype = arrayOf(TEST_SUBTYPE, "_sub", "_testservice", "_tcp", "local")
        val serviceName = arrayOf("MyTestService", "_testservice", "_tcp", "local")
        val v4AddrRev = getReverseDnsAddress(TEST_ADDRESSES[0].address)
        val v6Addr1Rev = getReverseDnsAddress(TEST_ADDRESSES[1].address)
@@ -250,6 +290,13 @@ class MdnsRecordRepositoryTest {
                        false /* cacheFlush */,
                        4500000L /* ttlMillis */,
                        serviceName),
+                MdnsPointerRecord(
+                        serviceSubtype,
+                        0L /* receiptTimeMillis */,
+                        // Not a unique name owned by the announcer, so cacheFlush=false
+                        false /* cacheFlush */,
+                        4500000L /* ttlMillis */,
+                        serviceName),
                MdnsServiceRecord(
                        serviceName,
                        0L /* receiptTimeMillis */,
@@ -319,9 +366,21 @@ class MdnsRecordRepositoryTest {

    @Test
    fun testGetReply() {
+        doGetReplyTest(subtype = null)
+    }
+
+    @Test
+    fun testGetReply_WithSubtype() {
+        doGetReplyTest(TEST_SUBTYPE)
+    }
+
+    private fun doGetReplyTest(subtype: String?) {
        val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME)
-        repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1)
-        val questions = listOf(MdnsPointerRecord(arrayOf("_testservice", "_tcp", "local"),
+        repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1, subtype)
+        val queriedName = if (subtype == null) arrayOf("_testservice", "_tcp", "local")
+        else arrayOf(subtype, "_sub", "_testservice", "_tcp", "local")
+
+        val questions = listOf(MdnsPointerRecord(queriedName,
                0L /* receiptTimeMillis */,
                false /* cacheFlush */,
                // TTL and data is empty for a question
@@ -344,7 +403,7 @@ class MdnsRecordRepositoryTest {

        assertEquals(listOf(
                MdnsPointerRecord(
-                        arrayOf("_testservice", "_tcp", "local"),
+                        queriedName,
                        0L /* receiptTimeMillis */,
                        false /* cacheFlush */,
                        longTtl,
@@ -405,8 +464,8 @@ class MdnsRecordRepositoryTest {
    @Test
    fun testGetConflictingServices() {
        val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME)
-        repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1)
-        repository.addService(TEST_SERVICE_ID_2, TEST_SERVICE_2)
+        repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1, null /* subtype */)
+        repository.addService(TEST_SERVICE_ID_2, TEST_SERVICE_2, null /* subtype */)

        val packet = MdnsPacket(
                0 /* flags */,
@@ -433,8 +492,8 @@ class MdnsRecordRepositoryTest {
    @Test
    fun testGetConflictingServices_IdenticalService() {
        val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME)
-        repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1)
-        repository.addService(TEST_SERVICE_ID_2, TEST_SERVICE_2)
+        repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1, null /* subtype */)
+        repository.addService(TEST_SERVICE_ID_2, TEST_SERVICE_2, null /* subtype */)

        val otherTtlMillis = 1234L
        val packet = MdnsPacket(
@@ -460,10 +519,13 @@ class MdnsRecordRepositoryTest {
    }
 }

-private fun MdnsRecordRepository.initWithService(serviceId: Int, serviceInfo: NsdServiceInfo):
-        AnnouncementInfo {
+private fun MdnsRecordRepository.initWithService(
+    serviceId: Int,
+    serviceInfo: NsdServiceInfo,
+    subtype: String? = null
+): AnnouncementInfo {
    updateAddresses(TEST_ADDRESSES)
-    addService(serviceId, serviceInfo)
+    addService(serviceId, serviceInfo, subtype)
    val probingInfo = setServiceProbing(serviceId)
    assertNotNull(probingInfo)
    return onProbingSucceeded(probingInfo)