diff options
3 files changed, 70 insertions, 12 deletions
diff --git a/tools/lint/common/src/main/java/com/google/android/lint/aidl/Constants.kt b/tools/lint/common/src/main/java/com/google/android/lint/aidl/Constants.kt index e03d92ab44a0..f1727b78f135 100644 --- a/tools/lint/common/src/main/java/com/google/android/lint/aidl/Constants.kt +++ b/tools/lint/common/src/main/java/com/google/android/lint/aidl/Constants.kt @@ -30,6 +30,7 @@ const val BINDER_CLASS = "android.os.Binder" const val IINTERFACE_INTERFACE = "android.os.IInterface" const val AIDL_PERMISSION_HELPER_SUFFIX = "_enforcePermission" +const val PERMISSION_PREFIX_LITERAL = "android.permission." /** * If a non java (e.g. c++) backend is enabled, the @EnforcePermission diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt index a74400d3c0b3..dcd94f1bcba4 100644 --- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt +++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt @@ -97,7 +97,7 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner { val v1 = ConstantEvaluator.evaluate(context, value1) val v2 = ConstantEvaluator.evaluate(context, value2) if (v1 != null && v2 != null) { - if (v1 != v2) { + if (v1 != v2 && !isOneShortPermissionOfOther(v1, v2)) { return false } } else { @@ -109,7 +109,7 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner { for (j in children1.indices) { val c1 = ConstantEvaluator.evaluate(context, children1[j]) val c2 = ConstantEvaluator.evaluate(context, children2[j]) - if (c1 != c2) { + if (c1 != c2 && !isOneShortPermissionOfOther(c1, c2)) { return false } } @@ -118,6 +118,12 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner { return true } + private fun isOneShortPermissionOfOther( + permission1: Any?, + permission2: Any? + ): Boolean = permission1 == (permission2 as? String)?.removePrefix(PERMISSION_PREFIX_LITERAL) || + permission2 == (permission1 as? String)?.removePrefix(PERMISSION_PREFIX_LITERAL) + private fun compareMethods( context: JavaContext, element: UElement, diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt index 75b00737a168..b3dacbdf57a6 100644 --- a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt +++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt @@ -316,20 +316,55 @@ class EnforcePermissionDetectorTest : LintDetectorTest() { overrides the method Stub.testMethod which is annotated with @EnforcePermission. The same annotation must be used on Default.testMethod [MissingEnforcePermissionAnnotation] public void testMethod() {} ~~~~~~~~~~ - 1 errors, 0 warnings + 1 errors, 0 warnings """.addLineContinuation() ) } - fun testDoesDetectIssuesShortStringsNotAllowed() { + fun testDoesNotDetectIssuesShortStringsAllowedInChildAndParent() { lint().files(java( """ package test.pkg; import android.annotation.EnforcePermission; public class TestClass121 extends IFooMethod.Stub { @Override + @EnforcePermission("READ_PHONE_STATE") + public void testMethod() {} + @Override + @EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) + public void testMethodParentShortPermission() {} + @Override @EnforcePermission(anyOf={"INTERNET", "READ_PHONE_STATE"}) public void testMethodAnyLiteral() {} + @Override + @EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE}) + public void testMethodAnyLiteralParentsShortPermission() {} + } + """).indented(), + *stubs + ) + .run() + .expectClean() + } + + fun testDoesDetectIssuesWrongShortStringsInChildAndParent() { + lint().files(java( + """ + package test.pkg; + import android.annotation.EnforcePermission; + public class TestClass121 extends IFooMethod.Stub { + @Override + @EnforcePermission("READ_WRONG_PHONE_STATE") + public void testMethod() {} + @Override + @EnforcePermission(android.Manifest.permission.READ_WRONG_PHONE_STATE) + public void testMethodParentShortPermission() {} + @Override + @EnforcePermission(anyOf={"WRONG_INTERNET", "READ_PHONE_STATE"}) + public void testMethodAnyLiteral() {} + @Override + @EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_WRONG_PHONE_STATE}) + public void testMethodAnyLiteralParentsShortPermission() {} } """).indented(), *stubs @@ -337,14 +372,19 @@ class EnforcePermissionDetectorTest : LintDetectorTest() { .run() .expect( """ - src/test/pkg/TestClass121.java:6: Error: The method \ - TestClass121.testMethodAnyLiteral is annotated with @EnforcePermission(anyOf={"INTERNET", "READ_PHONE_STATE"}) \ - which differs from the overridden method Stub.testMethodAnyLiteral: \ - @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}). \ - The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation] - public void testMethodAnyLiteral() {} - ~~~~~~~~~~~~~~~~~~~~ - 1 errors, 0 warnings + src/test/pkg/TestClass121.java:6: Error: The method TestClass121.testMethod is annotated with @EnforcePermission("READ_WRONG_PHONE_STATE") which differs from the overridden method Stub.testMethod: @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE). The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation] + public void testMethod() {} + ~~~~~~~~~~ + src/test/pkg/TestClass121.java:9: Error: The method TestClass121.testMethodParentShortPermission is annotated with @EnforcePermission(android.Manifest.permission.READ_WRONG_PHONE_STATE) which differs from the overridden method Stub.testMethodParentShortPermission: @android.annotation.EnforcePermission("READ_PHONE_STATE"). The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation] + public void testMethodParentShortPermission() {} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + src/test/pkg/TestClass121.java:12: Error: The method TestClass121.testMethodAnyLiteral is annotated with @EnforcePermission(anyOf={"WRONG_INTERNET", "READ_PHONE_STATE"}) which differs from the overridden method Stub.testMethodAnyLiteral: @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}). The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation] + public void testMethodAnyLiteral() {} + ~~~~~~~~~~~~~~~~~~~~ + src/test/pkg/TestClass121.java:15: Error: The method TestClass121.testMethodAnyLiteralParentsShortPermission is annotated with @EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_WRONG_PHONE_STATE}) which differs from the overridden method Stub.testMethodAnyLiteralParentsShortPermission: @android.annotation.EnforcePermission(anyOf={"INTERNET", "READ_PHONE_STATE"}). The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation] + public void testMethodAnyLiteralParentsShortPermission() {} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 4 errors, 0 warnings """.addLineContinuation() ) } @@ -360,12 +400,18 @@ class EnforcePermissionDetectorTest : LintDetectorTest() { @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) public void testMethod() {} @Override + @android.annotation.EnforcePermission("READ_PHONE_STATE") + public void testMethodParentShortPermission() {} + @Override @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE}) public void testMethodAny() {} @Override @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}) public void testMethodAnyLiteral() {} @Override + @android.annotation.EnforcePermission(anyOf={"INTERNET", "READ_PHONE_STATE"}) + public void testMethodAnyLiteralParentsShortPermission() {} + @Override @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE}) public void testMethodAll() {} @Override @@ -374,10 +420,14 @@ class EnforcePermissionDetectorTest : LintDetectorTest() { } @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) public void testMethod(); + @android.annotation.EnforcePermission("READ_PHONE_STATE") + public void testMethodParentShortPermission(); @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE}) public void testMethodAny() {} @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}) public void testMethodAnyLiteral() {} + @android.annotation.EnforcePermission(anyOf={"INTERNET", "READ_PHONE_STATE"}) + public void testMethodAnyLiteralParentsShortPermission() {} @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE}) public void testMethodAll() {} @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}) @@ -404,6 +454,7 @@ class EnforcePermissionDetectorTest : LintDetectorTest() { package android.Manifest; class permission { public static final String READ_PHONE_STATE = "android.permission.READ_PHONE_STATE"; + public static final String READ_WRONG_PHONE_STATE = "android.permission.READ_WRONG_PHONE_STATE"; public static final String NFC = "android.permission.NFC"; public static final String INTERNET = "android.permission.INTERNET"; } |