From d55bdd26e2227104c583f84bc6f8304024777e2a Mon Sep 17 00:00:00 2001 From: Motomu Utsumi Date: Fri, 23 Aug 2024 16:23:24 +0900 Subject: [PATCH] Update checkLocationPermissionInternal not to throw Currently, checkLocationPermissionInternal throws if the package does not exist or package name and uid doest not match. This caused system server crash in ConnectivityService. ConnectivityService is an only caller of checkLocationPermissionInternal and ConnectivityService should consider uid does not have location permission in this case. So this CL updates checkLocationPermissionInternal return ERROR_LOCATION_PERMISSION_MISSING instead of throwing exception. Bug: 337473824 Test: LocationPermissionCheckerTest Change-Id: Ic42d6203bafa95aea7e5428c490b0a298e3bd260 --- .../module/util/LocationPermissionChecker.java | 6 +++++- .../util/LocationPermissionCheckerTest.java | 18 +++--------------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/staticlibs/framework/com/android/net/module/util/LocationPermissionChecker.java b/staticlibs/framework/com/android/net/module/util/LocationPermissionChecker.java index 28c33f3257..e4d25cdf07 100644 --- a/staticlibs/framework/com/android/net/module/util/LocationPermissionChecker.java +++ b/staticlibs/framework/com/android/net/module/util/LocationPermissionChecker.java @@ -117,7 +117,11 @@ public class LocationPermissionChecker { @VisibleForTesting(visibility = PRIVATE) public @LocationPermissionCheckStatus int checkLocationPermissionInternal( String pkgName, @Nullable String featureId, int uid, @Nullable String message) { - checkPackage(uid, pkgName); + try { + checkPackage(uid, pkgName); + } catch (SecurityException e) { + return ERROR_LOCATION_PERMISSION_MISSING; + } // Apps with NETWORK_SETTINGS, NETWORK_SETUP_WIZARD, NETWORK_STACK & MAINLINE_NETWORK_STACK // are granted a bypass. diff --git a/staticlibs/tests/unit/src/com/android/net/module/util/LocationPermissionCheckerTest.java b/staticlibs/tests/unit/src/com/android/net/module/util/LocationPermissionCheckerTest.java index c8f8656cd5..d773374602 100644 --- a/staticlibs/tests/unit/src/com/android/net/module/util/LocationPermissionCheckerTest.java +++ b/staticlibs/tests/unit/src/com/android/net/module/util/LocationPermissionCheckerTest.java @@ -18,7 +18,6 @@ package com.android.net.module.util; import static android.Manifest.permission.NETWORK_SETTINGS; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; @@ -47,7 +46,6 @@ import androidx.annotation.RequiresApi; import com.android.testutils.DevSdkIgnoreRule; -import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -242,9 +240,9 @@ public class LocationPermissionCheckerTest { mWifiScanAllowApps = AppOpsManager.MODE_ALLOWED; setupTestCase(); - assertThrows(SecurityException.class, - () -> mChecker.checkLocationPermissionInternal( - TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null)); + final int result = mChecker.checkLocationPermissionInternal( + TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null); + assertEquals(LocationPermissionChecker.ERROR_LOCATION_PERMISSION_MISSING, result); } @Test @@ -305,14 +303,4 @@ public class LocationPermissionCheckerTest { TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null); assertEquals(LocationPermissionChecker.SUCCEEDED, result); } - - - private static void assertThrows(Class exceptionClass, Runnable r) { - try { - r.run(); - Assert.fail("Expected " + exceptionClass + " to be thrown."); - } catch (Exception exception) { - assertTrue(exceptionClass.isInstance(exception)); - } - } }