diff options
author | James Mattis <jmattis@google.com> | 2022-02-28 14:08:12 -0800 |
---|---|---|
committer | James Mattis <jmattis@google.com> | 2022-03-02 14:02:53 -0800 |
commit | b7c9537a271dca8917a0faf888afeaa5678770a1 (patch) | |
tree | 3f17fb5d4e64301cf2ed699008e9cfcabee8c431 | |
parent | fcc56fc1588a9f5f884cd8b3eaa63bcea610f8b5 (diff) | |
download | ethernet-b7c9537a271dca8917a0faf888afeaa5678770a1.tar.gz |
Ethernet validate if iface is tracked on handler
Removing synchronous validation of whether a particular interface
is tracked or not and instead relying on asynchronous validation and
callbacks. An interface can be in the midst of being provisioned and
checking if it is tracked sychronously before provisioning is complete
will erroneously throw an error for a call that would have been
successful when executed on the ethernet handler thread.
Bug: 210487893
Bug: 210485380
Test: atest EthernetServiceTests
Change-Id: Ib70312a240cab412a54ca7f598893aa9b1e108fd
4 files changed, 31 insertions, 46 deletions
diff --git a/java/com/android/server/ethernet/EthernetNetworkFactory.java b/java/com/android/server/ethernet/EthernetNetworkFactory.java index f266386..8ce27a6 100644 --- a/java/com/android/server/ethernet/EthernetNetworkFactory.java +++ b/java/com/android/server/ethernet/EthernetNetworkFactory.java @@ -220,20 +220,17 @@ public class EthernetNetworkFactory extends NetworkFactory { @NonNull final IpConfiguration ipConfig, @Nullable final NetworkCapabilities capabilities, @Nullable final IEthernetNetworkManagementListener listener) { - enforceInterfaceIsTracked(ifaceName); + if (!hasInterface(ifaceName)) { + maybeSendNetworkManagementCallbackForUntracked(ifaceName, listener); + return; + } + final NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName); iface.updateInterface(ipConfig, capabilities, listener); mTrackingInterfaces.put(ifaceName, iface); updateCapabilityFilter(); } - private void enforceInterfaceIsTracked(@NonNull final String ifaceName) { - if (!hasInterface(ifaceName)) { - throw new UnsupportedOperationException( - "Interface with name " + ifaceName + " is not being tracked."); - } - } - private static NetworkCapabilities mixInCapabilities(NetworkCapabilities nc, NetworkCapabilities addedNc) { final NetworkCapabilities.Builder builder = new NetworkCapabilities.Builder(nc); @@ -272,10 +269,8 @@ public class EthernetNetworkFactory extends NetworkFactory { @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) protected boolean updateInterfaceLinkState(@NonNull final String ifaceName, final boolean up, @Nullable final IEthernetNetworkManagementListener listener) { - if (!mTrackingInterfaces.containsKey(ifaceName)) { - maybeSendNetworkManagementCallback(listener, null, - new EthernetNetworkManagementException( - ifaceName + " can't be updated as it is not available.")); + if (!hasInterface(ifaceName)) { + maybeSendNetworkManagementCallbackForUntracked(ifaceName, listener); return false; } @@ -287,6 +282,13 @@ public class EthernetNetworkFactory extends NetworkFactory { return iface.updateLinkState(up, listener); } + private void maybeSendNetworkManagementCallbackForUntracked( + String ifaceName, IEthernetNetworkManagementListener listener) { + maybeSendNetworkManagementCallback(listener, null, + new EthernetNetworkManagementException( + ifaceName + " can't be updated as it is not available.")); + } + @VisibleForTesting protected boolean hasInterface(String ifaceName) { return mTrackingInterfaces.containsKey(ifaceName); diff --git a/java/com/android/server/ethernet/EthernetServiceImpl.java b/java/com/android/server/ethernet/EthernetServiceImpl.java index fed500f..f80f6a0 100644 --- a/java/com/android/server/ethernet/EthernetServiceImpl.java +++ b/java/com/android/server/ethernet/EthernetServiceImpl.java @@ -66,12 +66,6 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { methodName + " is only available on automotive devices."); } - private void enforceInterfaceIsTracked(final @NonNull String iface) { - if(!mTracker.isTrackingInterface(iface)) { - throw new UnsupportedOperationException("The given iface is not currently tracked."); - } - } - private boolean checkUseRestrictedNetworksPermission() { return PermissionUtils.checkAnyPermissionOf(mContext, android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS); @@ -220,13 +214,12 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { */ private void validateNetworkManagementState(@NonNull final String iface, final @NonNull String methodName) { + Objects.requireNonNull(iface, "Pass a non-null iface."); + Objects.requireNonNull(methodName, "Pass a non-null methodName."); + enforceAutomotiveDevice(methodName); enforceNetworkManagementPermission(); logIfEthernetNotStarted(); - - Objects.requireNonNull(iface, "Pass a non-null iface."); - Objects.requireNonNull(methodName, "Pass a non-null methodName."); - enforceInterfaceIsTracked(iface); } @Override diff --git a/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java b/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java index 33831a5..61425bf 100644 --- a/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java +++ b/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java @@ -763,4 +763,18 @@ public class EthernetNetworkFactoryTest { eq(capabilities), any(), any(), any(), any()); verifyRestart(ipConfiguration); } + + @Test + public void testUpdateInterfaceForNonExistingInterface() throws Exception { + initEthernetNetworkFactory(); + // No interface exists due to not calling createAndVerifyProvisionedInterface(...). + final NetworkCapabilities capabilities = createDefaultFilterCaps(); + final IpConfiguration ipConfiguration = createStaticIpConfig(); + final TestNetworkManagementListener listener = new TestNetworkManagementListener(); + + mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener); + + verifyNoStopOrStart(); + assertFailedListener(listener, "can't be updated as it is not available"); + } } diff --git a/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java index 2829753..e74a5a3 100644 --- a/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java +++ b/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java @@ -156,30 +156,6 @@ public class EthernetServiceImplTest { }); } - @Test - public void testUpdateConfigurationRejectsWithUntrackedIface() { - shouldTrackIface(TEST_IFACE, false); - assertThrows(UnsupportedOperationException.class, () -> { - mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST, NULL_LISTENER); - }); - } - - @Test - public void testConnectNetworkRejectsWithUntrackedIface() { - shouldTrackIface(TEST_IFACE, false); - assertThrows(UnsupportedOperationException.class, () -> { - mEthernetServiceImpl.connectNetwork(TEST_IFACE, NULL_LISTENER); - }); - } - - @Test - public void testDisconnectNetworkRejectsWithUntrackedIface() { - shouldTrackIface(TEST_IFACE, false); - assertThrows(UnsupportedOperationException.class, () -> { - mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, NULL_LISTENER); - }); - } - private void denyManageEthPermission() { doThrow(new SecurityException("")).when(mContext) .enforceCallingOrSelfPermission( |