Skip to content
Snippets Groups Projects
Commit 5b91f32b authored by Azhara Assanova's avatar Azhara Assanova Committed by Thiébaud Weksteen
Browse files

Allow short strings for manifest permissions in @EnforcePermission

Short strings in this context are permission strings without the leading
"android.permission.", e.g. a short string for
"android.permission.MODIFY_AUDIO_ROUTING" is "MODIFY_AUDIO_ROUTING".

Bug: 270686198
Test: atest --host AndroidGlobalLintCheckerTest
Change-Id: Ic01049613316b79123ce2300511efb5fd8143d4c
Merged-In: Ic01049613316b79123ce2300511efb5fd8143d4c
parent aec895aa
No related branches found
No related tags found
No related merge requests found
...@@ -30,6 +30,7 @@ const val BINDER_CLASS = "android.os.Binder" ...@@ -30,6 +30,7 @@ const val BINDER_CLASS = "android.os.Binder"
const val IINTERFACE_INTERFACE = "android.os.IInterface" const val IINTERFACE_INTERFACE = "android.os.IInterface"
const val AIDL_PERMISSION_HELPER_SUFFIX = "_enforcePermission" 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 * If a non java (e.g. c++) backend is enabled, the @EnforcePermission
......
...@@ -97,7 +97,7 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner { ...@@ -97,7 +97,7 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
val v1 = ConstantEvaluator.evaluate(context, value1) val v1 = ConstantEvaluator.evaluate(context, value1)
val v2 = ConstantEvaluator.evaluate(context, value2) val v2 = ConstantEvaluator.evaluate(context, value2)
if (v1 != null && v2 != null) { if (v1 != null && v2 != null) {
if (v1 != v2) { if (v1 != v2 && !isOneShortPermissionOfOther(v1, v2)) {
return false return false
} }
} else { } else {
...@@ -109,7 +109,7 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner { ...@@ -109,7 +109,7 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
for (j in children1.indices) { for (j in children1.indices) {
val c1 = ConstantEvaluator.evaluate(context, children1[j]) val c1 = ConstantEvaluator.evaluate(context, children1[j])
val c2 = ConstantEvaluator.evaluate(context, children2[j]) val c2 = ConstantEvaluator.evaluate(context, children2[j])
if (c1 != c2) { if (c1 != c2 && !isOneShortPermissionOfOther(c1, c2)) {
return false return false
} }
} }
...@@ -118,6 +118,12 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner { ...@@ -118,6 +118,12 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
return true 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( private fun compareMethods(
context: JavaContext, context: JavaContext,
element: UElement, element: UElement,
......
...@@ -316,20 +316,55 @@ class EnforcePermissionDetectorTest : LintDetectorTest() { ...@@ -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] overrides the method Stub.testMethod which is annotated with @EnforcePermission. The same annotation must be used on Default.testMethod [MissingEnforcePermissionAnnotation]
public void testMethod() {} public void testMethod() {}
~~~~~~~~~~ ~~~~~~~~~~
1 errors, 0 warnings 1 errors, 0 warnings
""".addLineContinuation() """.addLineContinuation()
) )
} }
fun testDoesDetectIssuesShortStringsNotAllowed() { fun testDoesNotDetectIssuesShortStringsAllowedInChildAndParent() {
lint().files(java( lint().files(java(
""" """
package test.pkg; package test.pkg;
import android.annotation.EnforcePermission; import android.annotation.EnforcePermission;
public class TestClass121 extends IFooMethod.Stub { 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 @Override
@EnforcePermission(anyOf={"INTERNET", "READ_PHONE_STATE"}) @EnforcePermission(anyOf={"INTERNET", "READ_PHONE_STATE"})
public void testMethodAnyLiteral() {} 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(), """).indented(),
*stubs *stubs
...@@ -337,14 +372,19 @@ class EnforcePermissionDetectorTest : LintDetectorTest() { ...@@ -337,14 +372,19 @@ class EnforcePermissionDetectorTest : LintDetectorTest() {
.run() .run()
.expect( .expect(
""" """
src/test/pkg/TestClass121.java:6: Error: The method \ 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]
TestClass121.testMethodAnyLiteral is annotated with @EnforcePermission(anyOf={"INTERNET", "READ_PHONE_STATE"}) \ public void testMethod() {}
which differs from the overridden method Stub.testMethodAnyLiteral: \ ~~~~~~~~~~
@android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}). \ 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]
The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation] public void testMethodParentShortPermission() {}
public void testMethodAnyLiteral() {} ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~~~~~~~~ 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]
1 errors, 0 warnings 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() """.addLineContinuation()
) )
} }
...@@ -360,12 +400,18 @@ class EnforcePermissionDetectorTest : LintDetectorTest() { ...@@ -360,12 +400,18 @@ class EnforcePermissionDetectorTest : LintDetectorTest() {
@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
public void testMethod() {} public void testMethod() {}
@Override @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}) @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
public void testMethodAny() {} public void testMethodAny() {}
@Override @Override
@android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}) @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"})
public void testMethodAnyLiteral() {} public void testMethodAnyLiteral() {}
@Override @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}) @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
public void testMethodAll() {} public void testMethodAll() {}
@Override @Override
...@@ -374,10 +420,14 @@ class EnforcePermissionDetectorTest : LintDetectorTest() { ...@@ -374,10 +420,14 @@ class EnforcePermissionDetectorTest : LintDetectorTest() {
} }
@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
public void testMethod(); 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}) @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
public void testMethodAny() {} public void testMethodAny() {}
@android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}) @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"})
public void testMethodAnyLiteral() {} 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}) @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
public void testMethodAll() {} public void testMethodAll() {}
@android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}) @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"})
...@@ -404,6 +454,7 @@ class EnforcePermissionDetectorTest : LintDetectorTest() { ...@@ -404,6 +454,7 @@ class EnforcePermissionDetectorTest : LintDetectorTest() {
package android.Manifest; package android.Manifest;
class permission { class permission {
public static final String READ_PHONE_STATE = "android.permission.READ_PHONE_STATE"; 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 NFC = "android.permission.NFC";
public static final String INTERNET = "android.permission.INTERNET"; public static final String INTERNET = "android.permission.INTERNET";
} }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment