diff --git a/service/mdns/com/android/server/connectivity/mdns/MdnsResponseDecoder.java b/service/mdns/com/android/server/connectivity/mdns/MdnsResponseDecoder.java index 861acbb8b0..6c2bc19545 100644 --- a/service/mdns/com/android/server/connectivity/mdns/MdnsResponseDecoder.java +++ b/service/mdns/com/android/server/connectivity/mdns/MdnsResponseDecoder.java @@ -256,6 +256,9 @@ public class MdnsResponseDecoder { response = new MdnsResponse(now); responses.add(response); } + // Set interface index earlier because some responses have PTR record only. + // Need to know every response is getting from which interface. + response.setInterfaceIndex(interfaceIndex); response.addPointerRecord((MdnsPointerRecord) record); } } @@ -286,13 +289,13 @@ public class MdnsResponseDecoder { List matchingResponses = findResponsesWithHostName(responses, inetRecord.getName()); for (MdnsResponse response : matchingResponses) { - assignInetRecord(response, inetRecord, interfaceIndex); + assignInetRecord(response, inetRecord); } } else { MdnsResponse response = findResponseWithHostName(responses, inetRecord.getName()); if (response != null) { - assignInetRecord(response, inetRecord, interfaceIndex); + assignInetRecord(response, inetRecord); } } } @@ -301,14 +304,11 @@ public class MdnsResponseDecoder { return SUCCESS; } - private static void assignInetRecord( - MdnsResponse response, MdnsInetAddressRecord inetRecord, int interfaceIndex) { + private static void assignInetRecord(MdnsResponse response, MdnsInetAddressRecord inetRecord) { if (inetRecord.getInet4Address() != null) { response.setInet4AddressRecord(inetRecord); - response.setInterfaceIndex(interfaceIndex); } else if (inetRecord.getInet6Address() != null) { response.setInet6AddressRecord(inetRecord); - response.setInterfaceIndex(interfaceIndex); } } diff --git a/service/mdns/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java b/service/mdns/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java index 53e58d1247..7a8fcc0cc7 100644 --- a/service/mdns/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java +++ b/service/mdns/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java @@ -44,9 +44,9 @@ public interface MdnsServiceBrowserListener { /** * Called when an mDNS service instance is no longer valid and removed. * - * @param serviceInstanceName The service instance name of the removed mDNS service. + * @param serviceInfo The service instance of the removed mDNS service. */ - void onServiceRemoved(@NonNull String serviceInstanceName); + void onServiceRemoved(@NonNull MdnsServiceInfo serviceInfo); /** * Called when searching for mDNS service has stopped because of an error. diff --git a/service/mdns/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java b/service/mdns/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java index 8ca71b9a07..0fd60250ed 100644 --- a/service/mdns/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java +++ b/service/mdns/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java @@ -91,8 +91,12 @@ public class MdnsServiceTypeClient { private static MdnsServiceInfo buildMdnsServiceInfoFromResponse( @NonNull MdnsResponse response, @NonNull String[] serviceTypeLabels) { - String[] hostName = response.getServiceRecord().getServiceHost(); - int port = response.getServiceRecord().getServicePort(); + String[] hostName = null; + int port = 0; + if (response.hasServiceRecord()) { + hostName = response.getServiceRecord().getServiceHost(); + port = response.getServiceRecord().getServicePort(); + } String ipv4Address = null; String ipv6Address = null; @@ -104,15 +108,17 @@ public class MdnsServiceTypeClient { Inet6Address inet6Address = response.getInet6AddressRecord().getInet6Address(); ipv6Address = (inet6Address == null) ? null : inet6Address.getHostAddress(); } - if (ipv4Address == null && ipv6Address == null) { - throw new IllegalArgumentException( - "Either ipv4Address or ipv6Address must be non-null"); - } String serviceInstanceName = response.getServiceInstanceName(); if (serviceInstanceName == null) { throw new IllegalStateException( "mDNS response must have non-null service instance name"); } + List textStrings = null; + List textEntries = null; + if (response.hasTextRecord()) { + textStrings = response.getTextRecord().getStrings(); + textEntries = response.getTextRecord().getEntries(); + } // TODO: Throw an error message if response doesn't have Inet6 or Inet4 address. return new MdnsServiceInfo( serviceInstanceName, @@ -122,8 +128,8 @@ public class MdnsServiceTypeClient { port, ipv4Address, ipv6Address, - response.getTextRecord().getStrings(), - response.getTextRecord().getEntries(), + textStrings, + textEntries, response.getInterfaceIndex()); } @@ -246,12 +252,14 @@ public class MdnsServiceTypeClient { } private void onGoodbyeReceived(@Nullable String serviceInstanceName) { - if (serviceInstanceName == null) { + final MdnsResponse response = instanceNameToResponse.remove(serviceInstanceName); + if (response == null) { return; } - instanceNameToResponse.remove(serviceInstanceName); for (MdnsServiceBrowserListener listener : listeners) { - listener.onServiceRemoved(serviceInstanceName); + final MdnsServiceInfo serviceInfo = + buildMdnsServiceInfoFromResponse(response, serviceTypeLabels); + listener.onServiceRemoved(serviceInfo); } } @@ -425,7 +433,10 @@ public class MdnsServiceTypeClient { String serviceInstanceName = existingResponse.getServiceInstanceName(); if (serviceInstanceName != null) { - listener.onServiceRemoved(serviceInstanceName); + final MdnsServiceInfo serviceInfo = + buildMdnsServiceInfoFromResponse( + existingResponse, serviceTypeLabels); + listener.onServiceRemoved(serviceInfo); } } } diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java index 6b10c7176c..6f8b85a423 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java @@ -24,6 +24,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -35,6 +36,7 @@ import static org.mockito.Mockito.when; import static java.nio.charset.StandardCharsets.UTF_8; import android.annotation.NonNull; +import android.text.TextUtils; import com.android.server.connectivity.mdns.MdnsServiceInfo.TextEntry; import com.android.server.connectivity.mdns.MdnsServiceTypeClient.QueryTaskConfig; @@ -57,6 +59,7 @@ import java.net.Inet4Address; import java.net.Inet6Address; import java.net.SocketAddress; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; @@ -71,6 +74,7 @@ import java.util.concurrent.TimeUnit; public class MdnsServiceTypeClientTests { private static final String SERVICE_TYPE = "_googlecast._tcp.local"; + private static final String[] SERVICE_TYPE_LABELS = TextUtils.split(SERVICE_TYPE, "\\."); @Mock private MdnsServiceBrowserListener mockListenerOne; @@ -491,17 +495,43 @@ public class MdnsServiceTypeClientTests { } @Test - public void processResponse_goodBye() { + public void processResponse_goodBye() throws Exception { client.startSendAndReceive(mockListenerOne, MdnsSearchOptions.getDefaultOptions()); client.startSendAndReceive(mockListenerTwo, MdnsSearchOptions.getDefaultOptions()); + final String serviceName = "service-instance-1"; + final String ipV6Address = "2000:3333::da6c:63ff:fe7c:7483"; + final int interfaceIndex = 999; + // Process the initial response. + final MdnsResponse initialResponse = + createResponse( + serviceName, + ipV6Address, + 5353 /* port */, + Collections.singletonList("ABCDE"), + Collections.emptyMap(), + interfaceIndex); + client.processResponse(initialResponse); MdnsResponse response = mock(MdnsResponse.class); - when(response.getServiceInstanceName()).thenReturn("goodbye-service-instance-name"); - when(response.isGoodbye()).thenReturn(true); + doReturn("goodbye-service").when(response).getServiceInstanceName(); + doReturn(interfaceIndex).when(response).getInterfaceIndex(); + doReturn(true).when(response).isGoodbye(); client.processResponse(response); + // Verify onServiceRemoved won't be called if the service is not existed. + verify(mockListenerOne, never()).onServiceRemoved(any()); + verify(mockListenerTwo, never()).onServiceRemoved(any()); - verify(mockListenerOne).onServiceRemoved("goodbye-service-instance-name"); - verify(mockListenerTwo).onServiceRemoved("goodbye-service-instance-name"); + // Verify onServiceRemoved would be called. + doReturn(serviceName).when(response).getServiceInstanceName(); + client.processResponse(response); + verify(mockListenerOne).onServiceRemoved(argThat( + info -> serviceName.equals(info.getServiceInstanceName()) + && Arrays.equals(SERVICE_TYPE_LABELS, info.getServiceType()) + && info.getInterfaceIndex() == interfaceIndex)); + verify(mockListenerTwo).onServiceRemoved(argThat( + info -> serviceName.equals(info.getServiceInstanceName()) + && Arrays.equals(SERVICE_TYPE_LABELS, info.getServiceType()) + && info.getInterfaceIndex() == interfaceIndex)); } @Test @@ -563,7 +593,7 @@ public class MdnsServiceTypeClientTests { firstMdnsTask.run(); // Verify onServiceRemoved was not called. - verify(mockListenerOne, never()).onServiceRemoved(serviceInstanceName); + verify(mockListenerOne, never()).onServiceRemoved(any()); } @Test @@ -586,7 +616,7 @@ public class MdnsServiceTypeClientTests { MdnsResponse initialResponse = createResponse( serviceInstanceName, "192.168.1.1", 5353, List.of("ABCDE"), - Map.of()); + Map.of(), 999 /* interfaceIndex */); client.processResponse(initialResponse); // Clear the scheduled runnable. @@ -597,14 +627,17 @@ public class MdnsServiceTypeClientTests { firstMdnsTask.run(); // Verify onServiceRemoved was not called. - verify(mockListenerOne, never()).onServiceRemoved(serviceInstanceName); + verify(mockListenerOne, never()).onServiceRemoved(any()); // Simulate the case where the response is after TTL. when(initialResponse.getServiceRecord().getRemainingTTL(anyLong())).thenReturn((long) 0); firstMdnsTask.run(); // Verify onServiceRemoved was called. - verify(mockListenerOne, times(1)).onServiceRemoved(serviceInstanceName); + verify(mockListenerOne, times(1)).onServiceRemoved(argThat( + info -> serviceInstanceName.equals(info.getServiceInstanceName()) + && Arrays.equals(SERVICE_TYPE_LABELS, info.getServiceType()) + && info.getInterfaceIndex() == 999)); } @Test @@ -636,7 +669,7 @@ public class MdnsServiceTypeClientTests { firstMdnsTask.run(); // Verify onServiceRemoved was not called. - verify(mockListenerOne, never()).onServiceRemoved(serviceInstanceName); + verify(mockListenerOne, never()).onServiceRemoved(any()); } @Test @@ -659,7 +692,7 @@ public class MdnsServiceTypeClientTests { MdnsResponse initialResponse = createResponse( serviceInstanceName, "192.168.1.1", 5353, List.of("ABCDE"), - Map.of()); + Map.of(), 999 /* interfaceIndex */); client.processResponse(initialResponse); // Clear the scheduled runnable. @@ -669,8 +702,11 @@ public class MdnsServiceTypeClientTests { when(initialResponse.getServiceRecord().getRemainingTTL(anyLong())).thenReturn((long) 0); firstMdnsTask.run(); - // Verify onServiceRemoved was not called. - verify(mockListenerOne, times(1)).onServiceRemoved(serviceInstanceName); + // Verify onServiceRemoved was called. + verify(mockListenerOne, times(1)).onServiceRemoved(argThat( + info -> serviceInstanceName.equals(info.getServiceInstanceName()) + && Arrays.equals(SERVICE_TYPE_LABELS, info.getServiceType()) + && info.getInterfaceIndex() == 999)); } // verifies that the right query was enqueued with the right delay, and send query by executing