From 3bee9d4099002a3bb53f6d1fe02aa10316d8b2a3 Mon Sep 17 00:00:00 2001 From: Yuyang Huang Date: Tue, 4 Apr 2023 13:00:54 +0900 Subject: [PATCH] Avoid creating a Network with INetd.LOCAL_NET_ID INetd.LOCAL_NET_ID cannot be referred to by system SDK. To make the MdnsSocketProvider built with system SDK, the reference to INetd.LOCAL_NET_ID must be removed. This network is created in MdnsSocketProvider.java and propagated through MdnsSocketProvider.java -> MdnsMultinetworkSocketClient.java -> MdnsDiscoveryManager.java -> MdnsServiceTypeClient.java -> NsdService.java. In NsdService.java, it was used in handleMdnsDiscoveryManagerEvent() -> buildNsdServiceInfoFromMdnsEvent() -> setServiceNetworkForCallback(). The setServiceNetworkForCallback() is updated to handle the NETID_UNSET the same as LOCAL_NET_ID. Test: atest CtsNetTestCases FrameworksNetTests Bug: 272392042 Change-Id: I07c573948e9dc6249325f0733807bb7a7ffc281c --- .../src/com/android/server/NsdService.java | 5 +- .../mdns/MdnsMultinetworkSocketClient.java | 4 +- .../connectivity/mdns/MdnsSocketProvider.java | 94 ++++++++++++++----- .../mdns/MdnsSocketProviderTest.java | 11 +-- 4 files changed, 80 insertions(+), 34 deletions(-) diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java index c00f1aefcb..a658791223 100644 --- a/service-t/src/com/android/server/NsdService.java +++ b/service-t/src/com/android/server/NsdService.java @@ -1106,9 +1106,12 @@ public class NsdService extends INsdManager.Stub { final String serviceName = serviceInfo.getServiceInstanceName(); final NsdServiceInfo servInfo = new NsdServiceInfo(serviceName, serviceType); final Network network = serviceInfo.getNetwork(); + // In MdnsDiscoveryManagerEvent, the Network can be null which means it is a + // network for Tethering interface. In other words, the network == null means the + // network has netId = INetd.LOCAL_NET_ID. setServiceNetworkForCallback( servInfo, - network == null ? NETID_UNSET : network.netId, + network == null ? INetd.LOCAL_NET_ID : network.netId, serviceInfo.getInterfaceIndex()); return servInfo; } diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java b/service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java index 5254342c0c..7af2231090 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java @@ -70,7 +70,7 @@ public class MdnsMultinetworkSocketClient implements MdnsSocketClientBase { } @Override - public void onSocketCreated(@NonNull Network network, + public void onSocketCreated(@Nullable Network network, @NonNull MdnsInterfaceSocket socket, @NonNull List addresses) { // The socket may be already created by other request before, try to get the stored // ReadPacketHandler. @@ -86,7 +86,7 @@ public class MdnsMultinetworkSocketClient implements MdnsSocketClientBase { } @Override - public void onInterfaceDestroyed(@NonNull Network network, + public void onInterfaceDestroyed(@Nullable Network network, @NonNull MdnsInterfaceSocket socket) { mSocketPacketHandlers.remove(socket); mActiveNetworkSockets.remove(socket); diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java b/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java index 1d9a24c3e3..0c709eae02 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java @@ -21,7 +21,6 @@ import android.annotation.Nullable; import android.content.Context; import android.net.ConnectivityManager; import android.net.ConnectivityManager.NetworkCallback; -import android.net.INetd; import android.net.LinkAddress; import android.net.LinkProperties; import android.net.Network; @@ -98,7 +97,7 @@ public class MdnsSocketProvider { @Override public void onLost(Network network) { mActiveNetworksLinkProperties.remove(network); - removeSocket(network, null /* interfaceName */); + removeNetworkSocket(network); } @Override @@ -235,7 +234,7 @@ public class MdnsSocketProvider { /*** Check whether the target network is matched current network */ public static boolean isNetworkMatched(@Nullable Network targetNetwork, - @NonNull Network currentNetwork) { + @Nullable Network currentNetwork) { return targetNetwork == null || targetNetwork.equals(currentNetwork); } @@ -258,9 +257,10 @@ public class MdnsSocketProvider { return; } + final NetworkAsKey networkKey = new NetworkAsKey(network); final SocketInfo socketInfo = mNetworkSockets.get(network); if (socketInfo == null) { - createSocket(network, lp); + createSocket(networkKey, lp); } else { // Update the addresses of this socket. final List addresses = lp.getLinkAddresses(); @@ -295,16 +295,16 @@ public class MdnsSocketProvider { final CompareResult interfaceDiff = new CompareResult<>( current, updated); for (String name : interfaceDiff.added) { - createSocket(new Network(INetd.LOCAL_NET_ID), createLPForTetheredInterface(name)); + createSocket(LOCAL_NET, createLPForTetheredInterface(name)); } for (String name : interfaceDiff.removed) { - removeSocket(new Network(INetd.LOCAL_NET_ID), name); + removeTetherInterfaceSocket(name); } current.clear(); current.addAll(updated); } - private void createSocket(Network network, LinkProperties lp) { + private void createSocket(NetworkKey networkKey, LinkProperties lp) { final String interfaceName = lp.getInterfaceName(); if (interfaceName == null) { Log.e(TAG, "Can not create socket with null interface name."); @@ -319,41 +319,52 @@ public class MdnsSocketProvider { } if (DBG) { - Log.d(TAG, "Create a socket on network:" + network + Log.d(TAG, "Create a socket on network:" + networkKey + " with interfaceName:" + interfaceName); } final MdnsInterfaceSocket socket = mDependencies.createMdnsInterfaceSocket( networkInterface.getNetworkInterface(), MdnsConstants.MDNS_PORT, mLooper, mPacketReadBuffer); final List addresses; - if (network.netId == INetd.LOCAL_NET_ID) { + if (networkKey == LOCAL_NET) { addresses = CollectionUtils.map( - networkInterface.getInterfaceAddresses(), LinkAddress::new); + networkInterface.getInterfaceAddresses(), + i -> new LinkAddress(i.getAddress(), i.getNetworkPrefixLength())); mTetherInterfaceSockets.put(interfaceName, new SocketInfo(socket, addresses)); } else { addresses = lp.getLinkAddresses(); - mNetworkSockets.put(network, new SocketInfo(socket, addresses)); + mNetworkSockets.put(((NetworkAsKey) networkKey).mNetwork, + new SocketInfo(socket, addresses)); } // Try to join IPv4/IPv6 group. socket.joinGroup(addresses); // Notify the listeners which need this socket. - notifySocketCreated(network, socket, addresses); + if (networkKey == LOCAL_NET) { + notifySocketCreated(null /* network */, socket, addresses); + } else { + notifySocketCreated(((NetworkAsKey) networkKey).mNetwork, socket, addresses); + } } catch (IOException e) { Log.e(TAG, "Create a socket failed with interface=" + interfaceName, e); } } - private void removeSocket(Network network, String interfaceName) { - final SocketInfo socketInfo = network.netId == INetd.LOCAL_NET_ID - ? mTetherInterfaceSockets.remove(interfaceName) - : mNetworkSockets.remove(network); + private void removeNetworkSocket(Network network) { + final SocketInfo socketInfo = mNetworkSockets.remove(network); if (socketInfo == null) return; socketInfo.mSocket.destroy(); notifyInterfaceDestroyed(network, socketInfo.mSocket); } + private void removeTetherInterfaceSocket(String interfaceName) { + final SocketInfo socketInfo = mTetherInterfaceSockets.remove(interfaceName); + if (socketInfo == null) return; + socketInfo.mSocket.destroy(); + notifyInterfaceDestroyed(null /* network */, socketInfo.mSocket); + } + private void notifySocketCreated(Network network, MdnsInterfaceSocket socket, List addresses) { for (int i = 0; i < mCallbacksToRequestedNetworks.size(); i++) { @@ -393,7 +404,7 @@ public class MdnsSocketProvider { if (DBG) Log.d(TAG, "There is no LinkProperties for this network:" + network); return; } - createSocket(network, lp); + createSocket(new NetworkAsKey(network), lp); } else { // Notify the socket for requested network. cb.onSocketCreated(network, socketInfo.mSocket, socketInfo.mAddresses); @@ -403,12 +414,11 @@ public class MdnsSocketProvider { private void retrieveAndNotifySocketFromInterface(String interfaceName, SocketCallback cb) { final SocketInfo socketInfo = mTetherInterfaceSockets.get(interfaceName); if (socketInfo == null) { - createSocket( - new Network(INetd.LOCAL_NET_ID), createLPForTetheredInterface(interfaceName)); + createSocket(LOCAL_NET, createLPForTetheredInterface(interfaceName)); } else { // Notify the socket for requested network. cb.onSocketCreated( - new Network(INetd.LOCAL_NET_ID), socketInfo.mSocket, socketInfo.mAddresses); + null /* network */, socketInfo.mSocket, socketInfo.mAddresses); } } @@ -467,7 +477,7 @@ public class MdnsSocketProvider { final SocketInfo info = mTetherInterfaceSockets.valueAt(i); info.mSocket.destroy(); // Still notify to unrequester for socket destroy. - cb.onInterfaceDestroyed(new Network(INetd.LOCAL_NET_ID), info.mSocket); + cb.onInterfaceDestroyed(null /* network */, info.mSocket); } mTetherInterfaceSockets.clear(); @@ -478,13 +488,49 @@ public class MdnsSocketProvider { /*** Callbacks for listening socket changes */ public interface SocketCallback { /*** Notify the socket is created */ - default void onSocketCreated(@NonNull Network network, @NonNull MdnsInterfaceSocket socket, + default void onSocketCreated(@Nullable Network network, @NonNull MdnsInterfaceSocket socket, @NonNull List addresses) {} /*** Notify the interface is destroyed */ - default void onInterfaceDestroyed(@NonNull Network network, + default void onInterfaceDestroyed(@Nullable Network network, @NonNull MdnsInterfaceSocket socket) {} /*** Notify the addresses is changed on the network */ - default void onAddressesChanged(@NonNull Network network, + default void onAddressesChanged(@Nullable Network network, @NonNull MdnsInterfaceSocket socket, @NonNull List addresses) {} } + + private interface NetworkKey { + } + + private static final NetworkKey LOCAL_NET = new NetworkKey() { + @Override + public String toString() { + return "NetworkKey:LOCAL_NET"; + } + }; + + private static class NetworkAsKey implements NetworkKey { + private final Network mNetwork; + + NetworkAsKey(Network network) { + this.mNetwork = network; + } + + @Override + public int hashCode() { + return mNetwork.hashCode(); + } + + @Override + public boolean equals(@Nullable Object other) { + if (!(other instanceof NetworkAsKey)) { + return false; + } + return mNetwork.equals(((NetworkAsKey) other).mNetwork); + } + + @Override + public String toString() { + return "NetworkAsKey{ network=" + mNetwork + " }"; + } + } } diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java index 6e1debe292..004ea7c75a 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java @@ -35,7 +35,6 @@ import static org.mockito.Mockito.verify; import android.content.Context; import android.net.ConnectivityManager; import android.net.ConnectivityManager.NetworkCallback; -import android.net.INetd; import android.net.LinkAddress; import android.net.LinkProperties; import android.net.Network; @@ -74,8 +73,6 @@ public class MdnsSocketProviderTest { private static final LinkAddress LINKADDRV6 = new LinkAddress("2001:0db8:85a3:0000:0000:8a2e:0370:7334/64"); private static final Network TEST_NETWORK = new Network(123); - private static final Network LOCAL_NETWORK = new Network(INetd.LOCAL_NET_ID); - @Mock private Context mContext; @Mock private Dependencies mDeps; @Mock private ConnectivityManager mCm; @@ -244,7 +241,7 @@ public class MdnsSocketProviderTest { verify(mLocalOnlyIfaceWrapper).getNetworkInterface(); testCallback1.expectedNoCallback(); testCallback2.expectedNoCallback(); - testCallback3.expectedSocketCreatedForNetwork(LOCAL_NETWORK, List.of()); + testCallback3.expectedSocketCreatedForNetwork(null /* network */, List.of()); mHandler.post(() -> mTetheringEventCallback.onTetheredInterfacesChanged( List.of(TETHERED_IFACE_NAME))); @@ -252,7 +249,7 @@ public class MdnsSocketProviderTest { verify(mTetheredIfaceWrapper).getNetworkInterface(); testCallback1.expectedNoCallback(); testCallback2.expectedNoCallback(); - testCallback3.expectedSocketCreatedForNetwork(LOCAL_NETWORK, List.of()); + testCallback3.expectedSocketCreatedForNetwork(null /* network */, List.of()); mHandler.post(() -> mSocketProvider.unrequestSocket(testCallback1)); HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT); @@ -270,14 +267,14 @@ public class MdnsSocketProviderTest { HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT); testCallback1.expectedNoCallback(); testCallback2.expectedNoCallback(); - testCallback3.expectedInterfaceDestroyedForNetwork(LOCAL_NETWORK); + testCallback3.expectedInterfaceDestroyedForNetwork(null /* network */); mHandler.post(() -> mSocketProvider.unrequestSocket(testCallback3)); HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT); testCallback1.expectedNoCallback(); testCallback2.expectedNoCallback(); // Expect the socket destroy for tethered interface. - testCallback3.expectedInterfaceDestroyedForNetwork(LOCAL_NETWORK); + testCallback3.expectedInterfaceDestroyedForNetwork(null /* network */); } @Test