From 94a00dfc649f5ddb630e6713a81421d0ecd00415 Mon Sep 17 00:00:00 2001 From: Xiao Ma Date: Tue, 17 May 2022 13:38:42 +0900 Subject: [PATCH] Fix the ethernet setting API bug. After turning off the ethernet settings via api, the ethernet service can still be started when plugging-in the cable. Improve the api by ignoring the upcoming interface unsolicited event listener callbacks and register new interface observer once the ethernet setting is enabled again. Bug: 231933028 Test: atest testListenEthernetStateChange_unsolicitedEventListener Change-Id: I1b1cb7d157f023fca82d75592168bd360a6fa6e1 --- .../server/ethernet/EthernetTracker.java | 20 ++++-- .../server/ethernet/EthernetTrackerTest.java | 70 +++++++++++++++---- 2 files changed, 74 insertions(+), 16 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java index e9053dd654..e5f1ea534b 100644 --- a/service-t/src/com/android/server/ethernet/EthernetTracker.java +++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java @@ -586,14 +586,18 @@ public class EthernetTracker { } } - private class InterfaceObserver extends BaseNetdUnsolicitedEventListener { + @VisibleForTesting + class InterfaceObserver extends BaseNetdUnsolicitedEventListener { @Override public void onInterfaceLinkStateChanged(String iface, boolean up) { if (DBG) { Log.i(TAG, "interfaceLinkStateChanged, iface: " + iface + ", up: " + up); } - mHandler.post(() -> updateInterfaceState(iface, up)); + mHandler.post(() -> { + if (mEthernetState == ETHERNET_STATE_DISABLED) return; + updateInterfaceState(iface, up); + }); } @Override @@ -601,7 +605,10 @@ public class EthernetTracker { if (DBG) { Log.i(TAG, "onInterfaceAdded, iface: " + iface); } - mHandler.post(() -> maybeTrackInterface(iface)); + mHandler.post(() -> { + if (mEthernetState == ETHERNET_STATE_DISABLED) return; + maybeTrackInterface(iface); + }); } @Override @@ -609,7 +616,10 @@ public class EthernetTracker { if (DBG) { Log.i(TAG, "onInterfaceRemoved, iface: " + iface); } - mHandler.post(() -> stopTrackingInterface(iface)); + mHandler.post(() -> { + if (mEthernetState == ETHERNET_STATE_DISABLED) return; + stopTrackingInterface(iface); + }); } } @@ -888,6 +898,8 @@ public class EthernetTracker { void dump(FileDescriptor fd, IndentingPrintWriter pw, String[] args) { postAndWaitForRunnable(() -> { pw.println(getClass().getSimpleName()); + pw.println("Ethernet State: " + + (mEthernetState == ETHERNET_STATE_ENABLED ? "enabled" : "disabled")); pw.println("Ethernet interface name filter: " + mIfaceMatch); pw.println("Default interface: " + mDefaultInterface); pw.println("Default interface mode: " + mDefaultInterfaceMode); diff --git a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java index b1831c411b..33b36fdeee 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java +++ b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java @@ -29,22 +29,23 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.content.Context; -import android.content.res.Resources; import android.net.EthernetManager; -import android.net.InetAddresses; -import android.net.INetworkInterfaceOutcomeReceiver; import android.net.IEthernetServiceListener; import android.net.INetd; +import android.net.INetworkInterfaceOutcomeReceiver; +import android.net.InetAddresses; +import android.net.InterfaceConfigurationParcel; import android.net.IpConfiguration; import android.net.IpConfiguration.IpAssignment; import android.net.IpConfiguration.ProxySettings; -import android.net.InterfaceConfigurationParcel; import android.net.LinkAddress; import android.net.NetworkCapabilities; import android.net.StaticIpConfiguration; @@ -54,13 +55,13 @@ import android.os.RemoteException; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; -import com.android.connectivity.resources.R; import com.android.testutils.HandlerUtils; import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -410,18 +411,24 @@ public class EthernetTrackerTest { IpConfiguration configuration) { } } + private InterfaceConfigurationParcel createMockedIfaceParcel(final String ifname, + final String hwAddr) { + final InterfaceConfigurationParcel ifaceParcel = new InterfaceConfigurationParcel(); + ifaceParcel.ifName = ifname; + ifaceParcel.hwAddr = hwAddr; + ifaceParcel.flags = new String[] {INetd.IF_STATE_UP}; + return ifaceParcel; + } + @Test public void testListenEthernetStateChange() throws Exception { - final String testIface = "testtap123"; - final String testHwAddr = "11:22:33:44:55:66"; - final InterfaceConfigurationParcel ifaceParcel = new InterfaceConfigurationParcel(); - ifaceParcel.ifName = testIface; - ifaceParcel.hwAddr = testHwAddr; - ifaceParcel.flags = new String[] {INetd.IF_STATE_UP}; - tracker.setIncludeTestInterfaces(true); waitForIdle(); + final String testIface = "testtap123"; + final String testHwAddr = "11:22:33:44:55:66"; + final InterfaceConfigurationParcel ifaceParcel = createMockedIfaceParcel(testIface, + testHwAddr); when(mNetd.interfaceGetList()).thenReturn(new String[] {testIface}); when(mNetd.interfaceGetCfg(eq(testIface))).thenReturn(ifaceParcel); doReturn(new String[] {testIface}).when(mFactory).getAvailableInterfaces(anyBoolean()); @@ -453,4 +460,43 @@ public class EthernetTrackerTest { verify(listener).onInterfaceStateChanged(eq(testIface), eq(EthernetManager.STATE_LINK_UP), anyInt(), any()); } + + @Test + public void testListenEthernetStateChange_unsolicitedEventListener() throws Exception { + when(mNetd.interfaceGetList()).thenReturn(new String[] {}); + doReturn(new String[] {}).when(mFactory).getAvailableInterfaces(anyBoolean()); + + tracker.setIncludeTestInterfaces(true); + tracker.start(); + + final ArgumentCaptor captor = + ArgumentCaptor.forClass(EthernetTracker.InterfaceObserver.class); + verify(mNetd, timeout(TIMEOUT_MS)).registerUnsolicitedEventListener(captor.capture()); + final EthernetTracker.InterfaceObserver observer = captor.getValue(); + + tracker.setEthernetEnabled(false); + waitForIdle(); + reset(mFactory); + reset(mNetd); + + final String testIface = "testtap1"; + observer.onInterfaceAdded(testIface); + verify(mFactory, never()).addInterface(eq(testIface), anyString(), any(), any()); + observer.onInterfaceRemoved(testIface); + verify(mFactory, never()).removeInterface(eq(testIface)); + + final String testHwAddr = "11:22:33:44:55:66"; + final InterfaceConfigurationParcel testIfaceParce = + createMockedIfaceParcel(testIface, testHwAddr); + when(mNetd.interfaceGetList()).thenReturn(new String[] {testIface}); + when(mNetd.interfaceGetCfg(eq(testIface))).thenReturn(testIfaceParce); + doReturn(new String[] {testIface}).when(mFactory).getAvailableInterfaces(anyBoolean()); + tracker.setEthernetEnabled(true); + waitForIdle(); + reset(mFactory); + + final String testIface2 = "testtap2"; + observer.onInterfaceRemoved(testIface2); + verify(mFactory, timeout(TIMEOUT_MS)).removeInterface(eq(testIface2)); + } }