Merge "Do not process null network packets on all clients" am: f4cf2707e7

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2602192

Change-Id: I9ba8e9f5bc3f4e876f3ffe34d4947da3936357b8
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Remi NGUYEN VAN
2023-05-29 04:35:53 +00:00
committed by Automerger Merge Worker
2 changed files with 92 additions and 80 deletions

View File

@@ -17,7 +17,6 @@
package com.android.server.connectivity.mdns; 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.ensureRunningOnHandlerThread;
import static com.android.server.connectivity.mdns.util.MdnsUtils.isNetworkMatched;
import static com.android.server.connectivity.mdns.util.MdnsUtils.isRunningOnHandlerThread; import static com.android.server.connectivity.mdns.util.MdnsUtils.isRunningOnHandlerThread;
import android.Manifest.permission; import android.Manifest.permission;
@@ -38,6 +37,7 @@ import com.android.server.connectivity.mdns.util.MdnsUtils;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.Objects;
/** /**
* This class keeps tracking the set of registered {@link MdnsServiceBrowserListener} instances, and * This class keeps tracking the set of registered {@link MdnsServiceBrowserListener} instances, and
@@ -86,15 +86,12 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback {
return list; return list;
} }
public List<MdnsServiceTypeClient> getByMatchingNetwork(@Nullable Network network) { public List<MdnsServiceTypeClient> getByNetwork(@Nullable Network network) {
final List<MdnsServiceTypeClient> list = new ArrayList<>(); final List<MdnsServiceTypeClient> list = new ArrayList<>();
for (int i = 0; i < clients.size(); i++) { for (int i = 0; i < clients.size(); i++) {
final Pair<String, Network> perNetworkServiceType = clients.keyAt(i); final Pair<String, Network> perNetworkServiceType = clients.keyAt(i);
final Network serviceTypeNetwork = perNetworkServiceType.second; final Network serviceTypeNetwork = perNetworkServiceType.second;
// The serviceTypeNetwork would be null if the MdnsSocketClient is being used. This if (Objects.equals(network, serviceTypeNetwork)) {
// 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)) {
list.add(clients.valueAt(i)); list.add(clients.valueAt(i));
} }
} }
@@ -239,7 +236,7 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback {
private void handleOnResponseReceived(@NonNull MdnsPacket packet, int interfaceIndex, private void handleOnResponseReceived(@NonNull MdnsPacket packet, int interfaceIndex,
@Nullable Network network) { @Nullable Network network) {
for (MdnsServiceTypeClient serviceTypeClient for (MdnsServiceTypeClient serviceTypeClient
: perNetworkServiceTypeClients.getByMatchingNetwork(network)) { : perNetworkServiceTypeClients.getByNetwork(network)) {
serviceTypeClient.processResponse(packet, interfaceIndex, network); serviceTypeClient.processResponse(packet, interfaceIndex, network);
} }
} }
@@ -254,7 +251,7 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback {
private void handleOnFailedToParseMdnsResponse(int receivedPacketNumber, int errorCode, private void handleOnFailedToParseMdnsResponse(int receivedPacketNumber, int errorCode,
@Nullable Network network) { @Nullable Network network) {
for (MdnsServiceTypeClient serviceTypeClient for (MdnsServiceTypeClient serviceTypeClient
: perNetworkServiceTypeClients.getByMatchingNetwork(network)) { : perNetworkServiceTypeClients.getByNetwork(network)) {
serviceTypeClient.onFailedToParseMdnsResponse(receivedPacketNumber, errorCode); serviceTypeClient.onFailedToParseMdnsResponse(receivedPacketNumber, errorCode);
} }
} }

View File

@@ -18,6 +18,8 @@ package com.android.server.connectivity.mdns;
import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2; 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.eq;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times; import static org.mockito.Mockito.times;
@@ -62,21 +64,24 @@ public class MdnsDiscoveryManagerTests {
private static final String SERVICE_TYPE_2 = "_test._tcp.local"; 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_1 = Mockito.mock(Network.class);
private static final Network NETWORK_2 = 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); 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); 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); 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); Pair.create(SERVICE_TYPE_2, NETWORK_2);
@Mock private ExecutorProvider executorProvider; @Mock private ExecutorProvider executorProvider;
@Mock private MdnsSocketClientBase socketClient; @Mock private MdnsSocketClientBase socketClient;
@Mock private MdnsServiceTypeClient mockServiceTypeClientOne; @Mock private MdnsServiceTypeClient mockServiceTypeClientType1NullNetwork;
@Mock private MdnsServiceTypeClient mockServiceTypeClientOne1; @Mock private MdnsServiceTypeClient mockServiceTypeClientType1Network1;
@Mock private MdnsServiceTypeClient mockServiceTypeClientTwo; @Mock private MdnsServiceTypeClient mockServiceTypeClientType2NullNetwork;
@Mock private MdnsServiceTypeClient mockServiceTypeClientTwo2; @Mock private MdnsServiceTypeClient mockServiceTypeClientType2Network1;
@Mock private MdnsServiceTypeClient mockServiceTypeClientType2Network2;
@Mock MdnsServiceBrowserListener mockListenerOne; @Mock MdnsServiceBrowserListener mockListenerOne;
@Mock MdnsServiceBrowserListener mockListenerTwo; @Mock MdnsServiceBrowserListener mockListenerTwo;
@@ -99,14 +104,20 @@ public class MdnsDiscoveryManagerTests {
@Nullable Network network) { @Nullable Network network) {
final Pair<String, Network> perNetworkServiceType = final Pair<String, Network> perNetworkServiceType =
Pair.create(serviceType, network); Pair.create(serviceType, network);
if (perNetworkServiceType.equals(PER_NETWORK_SERVICE_TYPE_1)) { if (perNetworkServiceType.equals(PER_NETWORK_SERVICE_TYPE_1_NULL_NETWORK)) {
return mockServiceTypeClientOne; return mockServiceTypeClientType1NullNetwork;
} else if (perNetworkServiceType.equals(PER_NETWORK_SERVICE_TYPE_1_1)) { } else if (perNetworkServiceType.equals(
return mockServiceTypeClientOne1; PER_NETWORK_SERVICE_TYPE_1_NETWORK_1)) {
} else if (perNetworkServiceType.equals(PER_NETWORK_SERVICE_TYPE_2)) { return mockServiceTypeClientType1Network1;
return mockServiceTypeClientTwo; } else if (perNetworkServiceType.equals(
} else if (perNetworkServiceType.equals(PER_NETWORK_SERVICE_TYPE_2_2)) { PER_NETWORK_SERVICE_TYPE_2_NULL_NETWORK)) {
return mockServiceTypeClientTwo2; 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; return null;
} }
@@ -143,11 +154,12 @@ public class MdnsDiscoveryManagerTests {
final SocketCreationCallback callback = expectSocketCreationCallback( final SocketCreationCallback callback = expectSocketCreationCallback(
SERVICE_TYPE_1, mockListenerOne, options); SERVICE_TYPE_1, mockListenerOne, options);
runOnHandler(() -> callback.onSocketCreated(null /* network */)); 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)); runOnHandler(() -> discoveryManager.unregisterListener(SERVICE_TYPE_1, mockListenerOne));
verify(mockServiceTypeClientOne).stopSendAndReceive(mockListenerOne); verify(mockServiceTypeClientType1NullNetwork).stopSendAndReceive(mockListenerOne);
verify(socketClient).stopDiscovery(); verify(socketClient).stopDiscovery();
} }
@@ -158,16 +170,16 @@ public class MdnsDiscoveryManagerTests {
final SocketCreationCallback callback = expectSocketCreationCallback( final SocketCreationCallback callback = expectSocketCreationCallback(
SERVICE_TYPE_1, mockListenerOne, options); SERVICE_TYPE_1, mockListenerOne, options);
runOnHandler(() -> callback.onSocketCreated(null /* network */)); runOnHandler(() -> callback.onSocketCreated(null /* network */));
verify(mockServiceTypeClientOne).startSendAndReceive(mockListenerOne, options); verify(mockServiceTypeClientType1NullNetwork).startSendAndReceive(mockListenerOne, options);
runOnHandler(() -> callback.onSocketCreated(NETWORK_1)); runOnHandler(() -> callback.onSocketCreated(NETWORK_1));
verify(mockServiceTypeClientOne1).startSendAndReceive(mockListenerOne, options); verify(mockServiceTypeClientType1Network1).startSendAndReceive(mockListenerOne, options);
final SocketCreationCallback callback2 = expectSocketCreationCallback( final SocketCreationCallback callback2 = expectSocketCreationCallback(
SERVICE_TYPE_2, mockListenerTwo, options); SERVICE_TYPE_2, mockListenerTwo, options);
runOnHandler(() -> callback2.onSocketCreated(null /* network */)); runOnHandler(() -> callback2.onSocketCreated(null /* network */));
verify(mockServiceTypeClientTwo).startSendAndReceive(mockListenerTwo, options); verify(mockServiceTypeClientType2NullNetwork).startSendAndReceive(mockListenerTwo, options);
runOnHandler(() -> callback2.onSocketCreated(NETWORK_2)); runOnHandler(() -> callback2.onSocketCreated(NETWORK_2));
verify(mockServiceTypeClientTwo2).startSendAndReceive(mockListenerTwo, options); verify(mockServiceTypeClientType2Network2).startSendAndReceive(mockListenerTwo, options);
} }
@Test @Test
@@ -177,103 +189,106 @@ public class MdnsDiscoveryManagerTests {
final SocketCreationCallback callback = expectSocketCreationCallback( final SocketCreationCallback callback = expectSocketCreationCallback(
SERVICE_TYPE_1, mockListenerOne, options1); SERVICE_TYPE_1, mockListenerOne, options1);
runOnHandler(() -> callback.onSocketCreated(null /* network */)); runOnHandler(() -> callback.onSocketCreated(null /* network */));
verify(mockServiceTypeClientOne).startSendAndReceive(mockListenerOne, options1); verify(mockServiceTypeClientType1NullNetwork).startSendAndReceive(
mockListenerOne, options1);
runOnHandler(() -> callback.onSocketCreated(NETWORK_1)); runOnHandler(() -> callback.onSocketCreated(NETWORK_1));
verify(mockServiceTypeClientOne1).startSendAndReceive(mockListenerOne, options1); verify(mockServiceTypeClientType1Network1).startSendAndReceive(mockListenerOne, options1);
final MdnsSearchOptions options2 = final MdnsSearchOptions options2 =
MdnsSearchOptions.newBuilder().setNetwork(NETWORK_2).build(); MdnsSearchOptions.newBuilder().setNetwork(NETWORK_2).build();
final SocketCreationCallback callback2 = expectSocketCreationCallback( final SocketCreationCallback callback2 = expectSocketCreationCallback(
SERVICE_TYPE_2, mockListenerTwo, options2); SERVICE_TYPE_2, mockListenerTwo, options2);
runOnHandler(() -> callback2.onSocketCreated(NETWORK_2)); runOnHandler(() -> callback2.onSocketCreated(NETWORK_2));
verify(mockServiceTypeClientTwo2).startSendAndReceive(mockListenerTwo, options2); verify(mockServiceTypeClientType2Network2).startSendAndReceive(mockListenerTwo, options2);
final MdnsPacket responseForServiceTypeOne = createMdnsPacket(SERVICE_TYPE_1); final MdnsPacket responseForServiceTypeOne = createMdnsPacket(SERVICE_TYPE_1);
final int ifIndex = 1; final int ifIndex = 1;
runOnHandler(() -> discoveryManager.onResponseReceived( runOnHandler(() -> discoveryManager.onResponseReceived(
responseForServiceTypeOne, ifIndex, null /* network */)); responseForServiceTypeOne, ifIndex, null /* network */));
verify(mockServiceTypeClientOne).processResponse(responseForServiceTypeOne, ifIndex, // Packets for network null are only processed by the ServiceTypeClient for network null
null /* network */); verify(mockServiceTypeClientType1NullNetwork).processResponse(responseForServiceTypeOne,
verify(mockServiceTypeClientOne1).processResponse(responseForServiceTypeOne, ifIndex, ifIndex, null /* network */);
null /* network */); verify(mockServiceTypeClientType1Network1, never()).processResponse(any(), anyInt(), any());
verify(mockServiceTypeClientTwo2).processResponse(responseForServiceTypeOne, ifIndex, verify(mockServiceTypeClientType2Network2, never()).processResponse(any(), anyInt(), any());
null /* network */);
final MdnsPacket responseForServiceTypeTwo = createMdnsPacket(SERVICE_TYPE_2); final MdnsPacket responseForServiceTypeTwo = createMdnsPacket(SERVICE_TYPE_2);
runOnHandler(() -> discoveryManager.onResponseReceived( runOnHandler(() -> discoveryManager.onResponseReceived(
responseForServiceTypeTwo, ifIndex, NETWORK_1)); responseForServiceTypeTwo, ifIndex, NETWORK_1));
verify(mockServiceTypeClientOne).processResponse(responseForServiceTypeTwo, ifIndex, verify(mockServiceTypeClientType1NullNetwork, never()).processResponse(any(), anyInt(),
NETWORK_1); eq(NETWORK_1));
verify(mockServiceTypeClientOne1).processResponse(responseForServiceTypeTwo, ifIndex, verify(mockServiceTypeClientType1Network1).processResponse(responseForServiceTypeTwo,
NETWORK_1);
verify(mockServiceTypeClientTwo2, never()).processResponse(responseForServiceTypeTwo,
ifIndex, NETWORK_1); ifIndex, NETWORK_1);
verify(mockServiceTypeClientType2Network2, never()).processResponse(any(), anyInt(),
eq(NETWORK_1));
final MdnsPacket responseForSubtype = final MdnsPacket responseForSubtype =
createMdnsPacket("subtype._sub._googlecast._tcp.local"); createMdnsPacket("subtype._sub._googlecast._tcp.local");
runOnHandler(() -> discoveryManager.onResponseReceived( runOnHandler(() -> discoveryManager.onResponseReceived(
responseForSubtype, ifIndex, NETWORK_2)); responseForSubtype, ifIndex, NETWORK_2));
verify(mockServiceTypeClientOne).processResponse(responseForSubtype, ifIndex, NETWORK_2); verify(mockServiceTypeClientType1NullNetwork, never()).processResponse(
verify(mockServiceTypeClientOne1, never()).processResponse( any(), anyInt(), eq(NETWORK_2));
verify(mockServiceTypeClientType1Network1, never()).processResponse(
any(), anyInt(), eq(NETWORK_2));
verify(mockServiceTypeClientType2Network2).processResponse(
responseForSubtype, ifIndex, NETWORK_2); responseForSubtype, ifIndex, NETWORK_2);
verify(mockServiceTypeClientTwo2).processResponse(responseForSubtype, ifIndex, NETWORK_2);
} }
@Test @Test
public void testSocketCreatedAndDestroyed() throws IOException { public void testSocketCreatedAndDestroyed() throws IOException {
// Create a ServiceTypeClient for SERVICE_TYPE_1 and NETWORK_1 // Create a ServiceTypeClient for SERVICE_TYPE_1 and NETWORK_1
final MdnsSearchOptions options1 = final MdnsSearchOptions network1Options =
MdnsSearchOptions.newBuilder().setNetwork(NETWORK_1).build(); MdnsSearchOptions.newBuilder().setNetwork(NETWORK_1).build();
final SocketCreationCallback callback = expectSocketCreationCallback( final SocketCreationCallback callback = expectSocketCreationCallback(
SERVICE_TYPE_1, mockListenerOne, options1); SERVICE_TYPE_1, mockListenerOne, network1Options);
runOnHandler(() -> callback.onSocketCreated(NETWORK_1)); 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 // Create a ServiceTypeClient for SERVICE_TYPE_2 and NETWORK_1
final MdnsSearchOptions options2 =
MdnsSearchOptions.newBuilder().setNetwork(NETWORK_2).build();
final SocketCreationCallback callback2 = expectSocketCreationCallback( final SocketCreationCallback callback2 = expectSocketCreationCallback(
SERVICE_TYPE_2, mockListenerTwo, options2); SERVICE_TYPE_2, mockListenerTwo, network1Options);
runOnHandler(() -> callback2.onSocketCreated(NETWORK_2)); runOnHandler(() -> callback2.onSocketCreated(NETWORK_1));
verify(mockServiceTypeClientTwo2).startSendAndReceive(mockListenerTwo, options2); verify(mockServiceTypeClientType2Network1).startSendAndReceive(
mockListenerTwo, network1Options);
// Receive a response, it should be processed on both clients. // Receive a response, it should be processed on both clients.
final MdnsPacket response = createMdnsPacket(SERVICE_TYPE_1); final MdnsPacket response = createMdnsPacket(SERVICE_TYPE_1);
final int ifIndex = 1; final int ifIndex = 1;
runOnHandler(() -> discoveryManager.onResponseReceived( runOnHandler(() -> discoveryManager.onResponseReceived(
response, ifIndex, null /* network */)); response, ifIndex, NETWORK_1));
verify(mockServiceTypeClientOne1).processResponse(response, ifIndex, null /* network */); verify(mockServiceTypeClientType1Network1).processResponse(response, ifIndex, NETWORK_1);
verify(mockServiceTypeClientTwo2).processResponse(response, ifIndex, null /* network */); 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 // mockServiceTypeClientOne1 should send service removed notifications and remove from the
// list of clients. // list of clients.
runOnHandler(() -> callback.onAllSocketsDestroyed(NETWORK_1)); runOnHandler(() -> callback.onAllSocketsDestroyed(NETWORK_1));
verify(mockServiceTypeClientOne1).notifySocketDestroyed(); verify(mockServiceTypeClientType1Network1).notifySocketDestroyed();
// Receive a response again, it should be processed only on mockServiceTypeClientTwo2. // Receive a response again, it should be processed only on
// Because the mockServiceTypeClientOne1 is removed from the list of clients, it is no // mockServiceTypeClientType2Network1. Because the mockServiceTypeClientType1Network1 is
// longer able to process responses. // removed from the list of clients, it is no longer able to process responses.
runOnHandler(() -> discoveryManager.onResponseReceived( runOnHandler(() -> discoveryManager.onResponseReceived(
response, ifIndex, null /* network */)); response, ifIndex, NETWORK_1));
verify(mockServiceTypeClientOne1, times(1)) // Still times(1) as a response was received once previously
.processResponse(response, ifIndex, null /* network */); verify(mockServiceTypeClientType1Network1, times(1))
verify(mockServiceTypeClientTwo2, times(2)) .processResponse(response, ifIndex, NETWORK_1);
.processResponse(response, ifIndex, null /* network */); 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. // mockServiceTypeClientTwo2 shouldn't send any notifications.
runOnHandler(() -> callback2.onAllSocketsDestroyed(NETWORK_1)); runOnHandler(() -> callback2.onAllSocketsDestroyed(NETWORK_2));
verify(mockServiceTypeClientTwo2, never()).notifySocketDestroyed(); verify(mockServiceTypeClientType2Network1, never()).notifySocketDestroyed();
// Receive a response again, mockServiceTypeClientTwo2 is still in the list of clients, it's // Receive a response again, mockServiceTypeClientType2Network1 is still in the list of
// still able to process responses. // clients, it's still able to process responses.
runOnHandler(() -> discoveryManager.onResponseReceived( runOnHandler(() -> discoveryManager.onResponseReceived(
response, ifIndex, null /* network */)); response, ifIndex, NETWORK_1));
verify(mockServiceTypeClientOne1, times(1)) verify(mockServiceTypeClientType1Network1, times(1))
.processResponse(response, ifIndex, null /* network */); .processResponse(response, ifIndex, NETWORK_1);
verify(mockServiceTypeClientTwo2, times(3)) verify(mockServiceTypeClientType2Network1, times(3))
.processResponse(response, ifIndex, null /* network */); .processResponse(response, ifIndex, NETWORK_1);
} }
private MdnsPacket createMdnsPacket(String serviceType) { private MdnsPacket createMdnsPacket(String serviceType) {