Do not process null network packets on all clients
Packets received from the "null" networks are packets received from
tethering downstream interfaces. They should not be processed by all
MdnsServiceTypeClients; instead they should be processed by
the MdnsServiceTypeClient for the null network.
Bug: 283708537
Test: atest
Change-Id: Ifef59eca7a24bdfe3650067445d2565869dfa852
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java b/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java
index 0154827..06ca914 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java
@@ -17,7 +17,6 @@
package com.android.server.connectivity.mdns;
import static com.android.server.connectivity.mdns.util.MdnsUtils.ensureRunningOnHandlerThread;
-import static com.android.server.connectivity.mdns.util.MdnsUtils.isNetworkMatched;
import static com.android.server.connectivity.mdns.util.MdnsUtils.isRunningOnHandlerThread;
import android.Manifest.permission;
@@ -38,6 +37,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
+import java.util.Objects;
/**
* This class keeps tracking the set of registered {@link MdnsServiceBrowserListener} instances, and
@@ -86,15 +86,12 @@
return list;
}
- public List<MdnsServiceTypeClient> getByMatchingNetwork(@Nullable Network network) {
+ public List<MdnsServiceTypeClient> getByNetwork(@Nullable Network network) {
final List<MdnsServiceTypeClient> list = new ArrayList<>();
for (int i = 0; i < clients.size(); i++) {
final Pair<String, Network> perNetworkServiceType = clients.keyAt(i);
final Network serviceTypeNetwork = perNetworkServiceType.second;
- // The serviceTypeNetwork would be null if the MdnsSocketClient is being used. This
- // is also the case if the socket is for a tethering interface. In either of these
- // cases, the client is expected to process any responses.
- if (serviceTypeNetwork == null || isNetworkMatched(network, serviceTypeNetwork)) {
+ if (Objects.equals(network, serviceTypeNetwork)) {
list.add(clients.valueAt(i));
}
}
@@ -239,7 +236,7 @@
private void handleOnResponseReceived(@NonNull MdnsPacket packet, int interfaceIndex,
@Nullable Network network) {
for (MdnsServiceTypeClient serviceTypeClient
- : perNetworkServiceTypeClients.getByMatchingNetwork(network)) {
+ : perNetworkServiceTypeClients.getByNetwork(network)) {
serviceTypeClient.processResponse(packet, interfaceIndex, network);
}
}
@@ -254,7 +251,7 @@
private void handleOnFailedToParseMdnsResponse(int receivedPacketNumber, int errorCode,
@Nullable Network network) {
for (MdnsServiceTypeClient serviceTypeClient
- : perNetworkServiceTypeClients.getByMatchingNetwork(network)) {
+ : perNetworkServiceTypeClients.getByNetwork(network)) {
serviceTypeClient.onFailedToParseMdnsResponse(receivedPacketNumber, errorCode);
}
}
diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java
index 89776e2..52f2712 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java
@@ -18,6 +18,8 @@
import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
@@ -62,21 +64,24 @@
private static final String SERVICE_TYPE_2 = "_test._tcp.local";
private static final Network NETWORK_1 = Mockito.mock(Network.class);
private static final Network NETWORK_2 = Mockito.mock(Network.class);
- private static final Pair<String, Network> PER_NETWORK_SERVICE_TYPE_1 =
+ private static final Pair<String, Network> PER_NETWORK_SERVICE_TYPE_1_NULL_NETWORK =
Pair.create(SERVICE_TYPE_1, null);
- private static final Pair<String, Network> PER_NETWORK_SERVICE_TYPE_1_1 =
+ private static final Pair<String, Network> PER_NETWORK_SERVICE_TYPE_1_NETWORK_1 =
Pair.create(SERVICE_TYPE_1, NETWORK_1);
- private static final Pair<String, Network> PER_NETWORK_SERVICE_TYPE_2 =
+ private static final Pair<String, Network> PER_NETWORK_SERVICE_TYPE_2_NULL_NETWORK =
Pair.create(SERVICE_TYPE_2, null);
- private static final Pair<String, Network> PER_NETWORK_SERVICE_TYPE_2_2 =
+ private static final Pair<String, Network> PER_NETWORK_SERVICE_TYPE_2_NETWORK_1 =
+ Pair.create(SERVICE_TYPE_2, NETWORK_1);
+ private static final Pair<String, Network> PER_NETWORK_SERVICE_TYPE_2_NETWORK_2 =
Pair.create(SERVICE_TYPE_2, NETWORK_2);
@Mock private ExecutorProvider executorProvider;
@Mock private MdnsSocketClientBase socketClient;
- @Mock private MdnsServiceTypeClient mockServiceTypeClientOne;
- @Mock private MdnsServiceTypeClient mockServiceTypeClientOne1;
- @Mock private MdnsServiceTypeClient mockServiceTypeClientTwo;
- @Mock private MdnsServiceTypeClient mockServiceTypeClientTwo2;
+ @Mock private MdnsServiceTypeClient mockServiceTypeClientType1NullNetwork;
+ @Mock private MdnsServiceTypeClient mockServiceTypeClientType1Network1;
+ @Mock private MdnsServiceTypeClient mockServiceTypeClientType2NullNetwork;
+ @Mock private MdnsServiceTypeClient mockServiceTypeClientType2Network1;
+ @Mock private MdnsServiceTypeClient mockServiceTypeClientType2Network2;
@Mock MdnsServiceBrowserListener mockListenerOne;
@Mock MdnsServiceBrowserListener mockListenerTwo;
@@ -99,14 +104,20 @@
@Nullable Network network) {
final Pair<String, Network> perNetworkServiceType =
Pair.create(serviceType, network);
- if (perNetworkServiceType.equals(PER_NETWORK_SERVICE_TYPE_1)) {
- return mockServiceTypeClientOne;
- } else if (perNetworkServiceType.equals(PER_NETWORK_SERVICE_TYPE_1_1)) {
- return mockServiceTypeClientOne1;
- } else if (perNetworkServiceType.equals(PER_NETWORK_SERVICE_TYPE_2)) {
- return mockServiceTypeClientTwo;
- } else if (perNetworkServiceType.equals(PER_NETWORK_SERVICE_TYPE_2_2)) {
- return mockServiceTypeClientTwo2;
+ if (perNetworkServiceType.equals(PER_NETWORK_SERVICE_TYPE_1_NULL_NETWORK)) {
+ return mockServiceTypeClientType1NullNetwork;
+ } else if (perNetworkServiceType.equals(
+ PER_NETWORK_SERVICE_TYPE_1_NETWORK_1)) {
+ return mockServiceTypeClientType1Network1;
+ } else if (perNetworkServiceType.equals(
+ PER_NETWORK_SERVICE_TYPE_2_NULL_NETWORK)) {
+ return mockServiceTypeClientType2NullNetwork;
+ } else if (perNetworkServiceType.equals(
+ PER_NETWORK_SERVICE_TYPE_2_NETWORK_1)) {
+ return mockServiceTypeClientType2Network1;
+ } else if (perNetworkServiceType.equals(
+ PER_NETWORK_SERVICE_TYPE_2_NETWORK_2)) {
+ return mockServiceTypeClientType2Network2;
}
return null;
}
@@ -143,11 +154,12 @@
final SocketCreationCallback callback = expectSocketCreationCallback(
SERVICE_TYPE_1, mockListenerOne, options);
runOnHandler(() -> callback.onSocketCreated(null /* network */));
- verify(mockServiceTypeClientOne).startSendAndReceive(mockListenerOne, options);
+ verify(mockServiceTypeClientType1NullNetwork).startSendAndReceive(mockListenerOne, options);
- when(mockServiceTypeClientOne.stopSendAndReceive(mockListenerOne)).thenReturn(true);
+ when(mockServiceTypeClientType1NullNetwork.stopSendAndReceive(mockListenerOne))
+ .thenReturn(true);
runOnHandler(() -> discoveryManager.unregisterListener(SERVICE_TYPE_1, mockListenerOne));
- verify(mockServiceTypeClientOne).stopSendAndReceive(mockListenerOne);
+ verify(mockServiceTypeClientType1NullNetwork).stopSendAndReceive(mockListenerOne);
verify(socketClient).stopDiscovery();
}
@@ -158,16 +170,16 @@
final SocketCreationCallback callback = expectSocketCreationCallback(
SERVICE_TYPE_1, mockListenerOne, options);
runOnHandler(() -> callback.onSocketCreated(null /* network */));
- verify(mockServiceTypeClientOne).startSendAndReceive(mockListenerOne, options);
+ verify(mockServiceTypeClientType1NullNetwork).startSendAndReceive(mockListenerOne, options);
runOnHandler(() -> callback.onSocketCreated(NETWORK_1));
- verify(mockServiceTypeClientOne1).startSendAndReceive(mockListenerOne, options);
+ verify(mockServiceTypeClientType1Network1).startSendAndReceive(mockListenerOne, options);
final SocketCreationCallback callback2 = expectSocketCreationCallback(
SERVICE_TYPE_2, mockListenerTwo, options);
runOnHandler(() -> callback2.onSocketCreated(null /* network */));
- verify(mockServiceTypeClientTwo).startSendAndReceive(mockListenerTwo, options);
+ verify(mockServiceTypeClientType2NullNetwork).startSendAndReceive(mockListenerTwo, options);
runOnHandler(() -> callback2.onSocketCreated(NETWORK_2));
- verify(mockServiceTypeClientTwo2).startSendAndReceive(mockListenerTwo, options);
+ verify(mockServiceTypeClientType2Network2).startSendAndReceive(mockListenerTwo, options);
}
@Test
@@ -177,103 +189,106 @@
final SocketCreationCallback callback = expectSocketCreationCallback(
SERVICE_TYPE_1, mockListenerOne, options1);
runOnHandler(() -> callback.onSocketCreated(null /* network */));
- verify(mockServiceTypeClientOne).startSendAndReceive(mockListenerOne, options1);
+ verify(mockServiceTypeClientType1NullNetwork).startSendAndReceive(
+ mockListenerOne, options1);
runOnHandler(() -> callback.onSocketCreated(NETWORK_1));
- verify(mockServiceTypeClientOne1).startSendAndReceive(mockListenerOne, options1);
+ verify(mockServiceTypeClientType1Network1).startSendAndReceive(mockListenerOne, options1);
final MdnsSearchOptions options2 =
MdnsSearchOptions.newBuilder().setNetwork(NETWORK_2).build();
final SocketCreationCallback callback2 = expectSocketCreationCallback(
SERVICE_TYPE_2, mockListenerTwo, options2);
runOnHandler(() -> callback2.onSocketCreated(NETWORK_2));
- verify(mockServiceTypeClientTwo2).startSendAndReceive(mockListenerTwo, options2);
+ verify(mockServiceTypeClientType2Network2).startSendAndReceive(mockListenerTwo, options2);
final MdnsPacket responseForServiceTypeOne = createMdnsPacket(SERVICE_TYPE_1);
final int ifIndex = 1;
runOnHandler(() -> discoveryManager.onResponseReceived(
responseForServiceTypeOne, ifIndex, null /* network */));
- verify(mockServiceTypeClientOne).processResponse(responseForServiceTypeOne, ifIndex,
- null /* network */);
- verify(mockServiceTypeClientOne1).processResponse(responseForServiceTypeOne, ifIndex,
- null /* network */);
- verify(mockServiceTypeClientTwo2).processResponse(responseForServiceTypeOne, ifIndex,
- null /* network */);
+ // Packets for network null are only processed by the ServiceTypeClient for network null
+ verify(mockServiceTypeClientType1NullNetwork).processResponse(responseForServiceTypeOne,
+ ifIndex, null /* network */);
+ verify(mockServiceTypeClientType1Network1, never()).processResponse(any(), anyInt(), any());
+ verify(mockServiceTypeClientType2Network2, never()).processResponse(any(), anyInt(), any());
final MdnsPacket responseForServiceTypeTwo = createMdnsPacket(SERVICE_TYPE_2);
runOnHandler(() -> discoveryManager.onResponseReceived(
responseForServiceTypeTwo, ifIndex, NETWORK_1));
- verify(mockServiceTypeClientOne).processResponse(responseForServiceTypeTwo, ifIndex,
- NETWORK_1);
- verify(mockServiceTypeClientOne1).processResponse(responseForServiceTypeTwo, ifIndex,
- NETWORK_1);
- verify(mockServiceTypeClientTwo2, never()).processResponse(responseForServiceTypeTwo,
+ verify(mockServiceTypeClientType1NullNetwork, never()).processResponse(any(), anyInt(),
+ eq(NETWORK_1));
+ verify(mockServiceTypeClientType1Network1).processResponse(responseForServiceTypeTwo,
ifIndex, NETWORK_1);
+ verify(mockServiceTypeClientType2Network2, never()).processResponse(any(), anyInt(),
+ eq(NETWORK_1));
final MdnsPacket responseForSubtype =
createMdnsPacket("subtype._sub._googlecast._tcp.local");
runOnHandler(() -> discoveryManager.onResponseReceived(
responseForSubtype, ifIndex, NETWORK_2));
- verify(mockServiceTypeClientOne).processResponse(responseForSubtype, ifIndex, NETWORK_2);
- verify(mockServiceTypeClientOne1, never()).processResponse(
+ verify(mockServiceTypeClientType1NullNetwork, never()).processResponse(
+ any(), anyInt(), eq(NETWORK_2));
+ verify(mockServiceTypeClientType1Network1, never()).processResponse(
+ any(), anyInt(), eq(NETWORK_2));
+ verify(mockServiceTypeClientType2Network2).processResponse(
responseForSubtype, ifIndex, NETWORK_2);
- verify(mockServiceTypeClientTwo2).processResponse(responseForSubtype, ifIndex, NETWORK_2);
}
@Test
public void testSocketCreatedAndDestroyed() throws IOException {
// Create a ServiceTypeClient for SERVICE_TYPE_1 and NETWORK_1
- final MdnsSearchOptions options1 =
+ final MdnsSearchOptions network1Options =
MdnsSearchOptions.newBuilder().setNetwork(NETWORK_1).build();
final SocketCreationCallback callback = expectSocketCreationCallback(
- SERVICE_TYPE_1, mockListenerOne, options1);
+ SERVICE_TYPE_1, mockListenerOne, network1Options);
runOnHandler(() -> callback.onSocketCreated(NETWORK_1));
- verify(mockServiceTypeClientOne1).startSendAndReceive(mockListenerOne, options1);
+ verify(mockServiceTypeClientType1Network1).startSendAndReceive(
+ mockListenerOne, network1Options);
- // Create a ServiceTypeClient for SERVICE_TYPE_2 and NETWORK_2
- final MdnsSearchOptions options2 =
- MdnsSearchOptions.newBuilder().setNetwork(NETWORK_2).build();
+ // Create a ServiceTypeClient for SERVICE_TYPE_2 and NETWORK_1
final SocketCreationCallback callback2 = expectSocketCreationCallback(
- SERVICE_TYPE_2, mockListenerTwo, options2);
- runOnHandler(() -> callback2.onSocketCreated(NETWORK_2));
- verify(mockServiceTypeClientTwo2).startSendAndReceive(mockListenerTwo, options2);
+ SERVICE_TYPE_2, mockListenerTwo, network1Options);
+ runOnHandler(() -> callback2.onSocketCreated(NETWORK_1));
+ verify(mockServiceTypeClientType2Network1).startSendAndReceive(
+ mockListenerTwo, network1Options);
// Receive a response, it should be processed on both clients.
final MdnsPacket response = createMdnsPacket(SERVICE_TYPE_1);
final int ifIndex = 1;
runOnHandler(() -> discoveryManager.onResponseReceived(
- response, ifIndex, null /* network */));
- verify(mockServiceTypeClientOne1).processResponse(response, ifIndex, null /* network */);
- verify(mockServiceTypeClientTwo2).processResponse(response, ifIndex, null /* network */);
+ response, ifIndex, NETWORK_1));
+ verify(mockServiceTypeClientType1Network1).processResponse(response, ifIndex, NETWORK_1);
+ verify(mockServiceTypeClientType2Network1).processResponse(response, ifIndex, NETWORK_1);
- // The client for NETWORK_1 receives the callback that the NETWORK_1 has been destroyed,
+ // The first callback receives a notification that the network has been destroyed,
// mockServiceTypeClientOne1 should send service removed notifications and remove from the
// list of clients.
runOnHandler(() -> callback.onAllSocketsDestroyed(NETWORK_1));
- verify(mockServiceTypeClientOne1).notifySocketDestroyed();
+ verify(mockServiceTypeClientType1Network1).notifySocketDestroyed();
- // Receive a response again, it should be processed only on mockServiceTypeClientTwo2.
- // Because the mockServiceTypeClientOne1 is removed from the list of clients, it is no
- // longer able to process responses.
+ // Receive a response again, it should be processed only on
+ // mockServiceTypeClientType2Network1. Because the mockServiceTypeClientType1Network1 is
+ // removed from the list of clients, it is no longer able to process responses.
runOnHandler(() -> discoveryManager.onResponseReceived(
- response, ifIndex, null /* network */));
- verify(mockServiceTypeClientOne1, times(1))
- .processResponse(response, ifIndex, null /* network */);
- verify(mockServiceTypeClientTwo2, times(2))
- .processResponse(response, ifIndex, null /* network */);
+ response, ifIndex, NETWORK_1));
+ // Still times(1) as a response was received once previously
+ verify(mockServiceTypeClientType1Network1, times(1))
+ .processResponse(response, ifIndex, NETWORK_1);
+ verify(mockServiceTypeClientType2Network1, times(2))
+ .processResponse(response, ifIndex, NETWORK_1);
- // The client for NETWORK_2 receives the callback that the NETWORK_1 has been destroyed,
+ // The client for NETWORK_1 receives the callback that the NETWORK_2 has been destroyed,
// mockServiceTypeClientTwo2 shouldn't send any notifications.
- runOnHandler(() -> callback2.onAllSocketsDestroyed(NETWORK_1));
- verify(mockServiceTypeClientTwo2, never()).notifySocketDestroyed();
+ runOnHandler(() -> callback2.onAllSocketsDestroyed(NETWORK_2));
+ verify(mockServiceTypeClientType2Network1, never()).notifySocketDestroyed();
- // Receive a response again, mockServiceTypeClientTwo2 is still in the list of clients, it's
- // still able to process responses.
+ // Receive a response again, mockServiceTypeClientType2Network1 is still in the list of
+ // clients, it's still able to process responses.
runOnHandler(() -> discoveryManager.onResponseReceived(
- response, ifIndex, null /* network */));
- verify(mockServiceTypeClientOne1, times(1))
- .processResponse(response, ifIndex, null /* network */);
- verify(mockServiceTypeClientTwo2, times(3))
- .processResponse(response, ifIndex, null /* network */);
+ response, ifIndex, NETWORK_1));
+ verify(mockServiceTypeClientType1Network1, times(1))
+ .processResponse(response, ifIndex, NETWORK_1);
+ verify(mockServiceTypeClientType2Network1, times(3))
+ .processResponse(response, ifIndex, NETWORK_1);
}
private MdnsPacket createMdnsPacket(String serviceType) {