Merge changes Ieae3558e,Ibc30de5e,I3415c5c4 (e57a25ea) · Commits · e / os / android_frameworks_base

tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt

+30 −60

Original line number	Diff line number	Diff line
		@@ -31,10 +31,11 @@ import com.android.tools.lint.detector.api.Scope
		import com.android.tools.lint.detector.api.Severity
		import com.android.tools.lint.detector.api.SourceCodeScanner
		import com.intellij.psi.PsiAnnotation
		import com.intellij.psi.PsiArrayInitializerMemberValue
		import com.intellij.psi.PsiClass
		import com.intellij.psi.PsiElement
		import com.intellij.psi.PsiMethod
		import org.jetbrains.uast.UAnnotation
		import org.jetbrains.uast.UClass
		import org.jetbrains.uast.UElement
		import org.jetbrains.uast.UMethod

		@@ -65,6 +66,12 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
		return listOf(UAnnotation::class.java)
		}

		private fun annotationValueGetChildren(elem: PsiElement): Array<PsiElement> {
		if (elem is PsiArrayInitializerMemberValue)
		return elem.getInitializers().map { it as PsiElement }.toTypedArray()
		return elem.getChildren()
		}

		private fun areAnnotationsEquivalent(
		context: JavaContext,
		anno1: PsiAnnotation,
		@@ -82,19 +89,29 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
		if (attr1[i].name != attr2[i].name) {
		return false
		}
		val value1 = attr1[i].value
		val value2 = attr2[i].value
		if (value1 == null && value2 == null) {
		continue
		}
		if (value1 == null \|\| value2 == null) {
		return false
		}
		val value1 = attr1[i].value ?: return false
		val value2 = attr2[i].value ?: return false
		// Try to compare values directly with each other.
		val v1 = ConstantEvaluator.evaluate(context, value1)
		val v2 = ConstantEvaluator.evaluate(context, value2)
		if (v1 != null && v2 != null) {
		if (v1 != v2) {
		return false
		}
		} else {
		val children1 = annotationValueGetChildren(value1)
		val children2 = annotationValueGetChildren(value2)
		if (children1.size != children2.size) {
		return false
		}
		for (j in children1.indices) {
		val c1 = ConstantEvaluator.evaluate(context, children1[j])
		val c2 = ConstantEvaluator.evaluate(context, children2[j])
		if (c1 != c2) {
		return false
		}
		}
		}
		}
		return true
		}
		@@ -140,50 +157,13 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
		}
		}

		private fun compareClasses(
		context: JavaContext,
		element: UElement,
		newClass: PsiClass,
		extendedClass: PsiClass,
		checkEquivalence: Boolean = true
		) {
		val newAnnotation = newClass.getAnnotation(ANNOTATION_ENFORCE_PERMISSION)
		val extendedAnnotation = extendedClass.getAnnotation(ANNOTATION_ENFORCE_PERMISSION)

		val location = context.getLocation(element)
		val newClassName = newClass.qualifiedName
		val extendedClassName = extendedClass.qualifiedName
		if (newAnnotation == null) {
		val msg = "The class $newClassName extends the class $extendedClassName which " +
		"is annotated with @EnforcePermission. The same annotation must be used " +
		"on $newClassName."
		context.report(ISSUE_MISSING_ENFORCE_PERMISSION, element, location, msg)
		} else if (extendedAnnotation == null) {
		val msg = "The class $newClassName extends the class $extendedClassName which " +
		"is not annotated with @EnforcePermission. The same annotation must be used " +
		"on $extendedClassName. Did you forget to annotate the AIDL definition?"
		context.report(ISSUE_MISSING_ENFORCE_PERMISSION, element, location, msg)
		} else if (checkEquivalence && !areAnnotationsEquivalent(
		context, newAnnotation, extendedAnnotation)) {
		val msg = "The class $newClassName is annotated with ${newAnnotation.text} " +
		"which differs from the parent class $extendedClassName: " +
		"${extendedAnnotation.text}. The same annotation must be used for " +
		"both classes."
		context.report(ISSUE_MISMATCHING_ENFORCE_PERMISSION, element, location, msg)
		}
		}

		override fun visitAnnotationUsage(
		context: JavaContext,
		element: UElement,
		annotationInfo: AnnotationInfo,
		usageInfo: AnnotationUsageInfo
		) {
		if (usageInfo.type == AnnotationUsageType.EXTENDS) {
		val newClass = element.sourcePsi?.parent?.parent as PsiClass
		val extendedClass: PsiClass = usageInfo.referenced as PsiClass
		compareClasses(context, element, newClass, extendedClass)
		} else if (usageInfo.type == AnnotationUsageType.METHOD_OVERRIDE &&
		if (usageInfo.type == AnnotationUsageType.METHOD_OVERRIDE &&
		annotationInfo.origin == AnnotationOrigin.METHOD) {
		val overridingMethod = element.sourcePsi as PsiMethod
		val overriddenMethod = usageInfo.referenced as PsiMethod
		@@ -198,17 +178,7 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
		return
		}
		val method = node.uastParent as? UMethod
		val klass = node.uastParent as? UClass
		if (klass != null) {
		val newClass = klass as PsiClass
		val extendedClass = newClass.getSuperClass()
		if (extendedClass != null && extendedClass.qualifiedName != JAVA_OBJECT) {
		// The equivalence check can be skipped, if both classes are
		// annotated, it will be verified by visitAnnotationUsage.
		compareClasses(context, klass, newClass,
		extendedClass, checkEquivalence = false)
		}
		} else if (method != null) {
		if (method != null) {
		val overridingMethod = method as PsiMethod
		val parents = overridingMethod.findSuperMethods()
		for (overriddenMethod in parents) {

tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt

+196 −80

Original line number	Diff line number	Diff line
		@@ -33,12 +33,14 @@ class EnforcePermissionDetectorTest : LintDetectorTest() {

		override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)

		fun testDoesNotDetectIssuesCorrectAnnotationOnClass() {
		fun testDoesNotDetectIssuesCorrectAnnotationOnMethod() {
		lint().files(java(
		"""
		package test.pkg;
		@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
		public class TestClass1 extends IFoo.Stub {
		import android.annotation.EnforcePermission;
		public class TestClass2 extends IFooMethod.Stub {
		@Override
		@EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
		public void testMethod() {}
		}
		""").indented(),
		@@ -48,15 +50,15 @@ class EnforcePermissionDetectorTest : LintDetectorTest() {
		.expectClean()
		}

		fun testDoesNotDetectIssuesCorrectAnnotationOnMethod() {
		fun testDoesNotDetectIssuesCorrectAnnotationAllOnMethod() {
		lint().files(java(
		"""
		package test.pkg;
		import android.annotation.EnforcePermission;
		public class TestClass2 extends IFooMethod.Stub {
		public class TestClass11 extends IFooMethod.Stub {
		@Override
		@EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
		public void testMethod() {}
		@EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
		public void testMethodAll() {}
		}
		""").indented(),
		*stubs
		@@ -65,26 +67,55 @@ class EnforcePermissionDetectorTest : LintDetectorTest() {
		.expectClean()
		}

		fun testDetectIssuesMismatchingAnnotationOnClass() {
		fun testDoesNotDetectIssuesCorrectAnnotationAllLiteralOnMethod() {
		lint().files(java(
		"""
		package test.pkg;
		@android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
		public class TestClass3 extends IFoo.Stub {
		public void testMethod() {}
		import android.annotation.EnforcePermission;
		public class TestClass111 extends IFooMethod.Stub {
		@Override
		@EnforcePermission(allOf={"android.permission.INTERNET", android.Manifest.permission.READ_PHONE_STATE})
		public void testMethodAllLiteral() {}
		}
		""").indented(),
		*stubs
		)
		.run()
		.expectClean()
		}

		fun testDoesNotDetectIssuesCorrectAnnotationAnyOnMethod() {
		lint().files(java(
		"""
		package test.pkg;
		import android.annotation.EnforcePermission;
		public class TestClass12 extends IFooMethod.Stub {
		@Override
		@EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
		public void testMethodAny() {}
		}
		""").indented(),
		*stubs
		)
		.run()
		.expectClean()
		}

		fun testDoesNotDetectIssuesCorrectAnnotationAnyLiteralOnMethod() {
		lint().files(java(
		"""
		package test.pkg;
		import android.annotation.EnforcePermission;
		public class TestClass121 extends IFooMethod.Stub {
		@Override
		@EnforcePermission(anyOf={"android.permission.INTERNET", android.Manifest.permission.READ_PHONE_STATE})
		public void testMethodAnyLiteral() {}
		}
		""").indented(),
		*stubs
		)
		.run()
		.expect("""src/test/pkg/TestClass3.java:3: Error: The class test.pkg.TestClass3 is \
		annotated with @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET) \
		which differs from the parent class IFoo.Stub: \
		@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE). The \
		same annotation must be used for both classes. [MismatchingEnforcePermissionAnnotation]
		public class TestClass3 extends IFoo.Stub {
		~~~~~~~~~
		1 errors, 0 warnings""".addLineContinuation())
		.expectClean()
		}

		fun testDetectIssuesMismatchingAnnotationOnMethod() {
		@@ -99,94 +130,173 @@ public class TestClass3 extends IFoo.Stub {
		*stubs
		)
		.run()
		.expect("""src/test/pkg/TestClass4.java:4: Error: The method TestClass4.testMethod is \
		annotated with @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET) \
		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]
		.expect("""
		src/test/pkg/TestClass4.java:4: Error: The method TestClass4.testMethod is annotated with @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET) \
		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() {}
		~~~~~~~~~~
		1 errors, 0 warnings""".addLineContinuation())
		1 errors, 0 warnings
		""".addLineContinuation())
		}

		fun testDetectIssuesMissingAnnotationOnClass() {
		fun testDetectIssuesEmptyAnnotationOnMethod() {
		lint().files(java(
		"""
		package test.pkg;
		public class TestClass5 extends IFoo.Stub {
		public class TestClass41 extends IFooMethod.Stub {
		@android.annotation.EnforcePermission
		public void testMethod() {}
		}
		""").indented(),
		*stubs
		)
		.run()
		.expect("""src/test/pkg/TestClass5.java:2: Error: The class test.pkg.TestClass5 extends \
		the class IFoo.Stub which is annotated with @EnforcePermission. The same annotation must be \
		used on test.pkg.TestClass5. [MissingEnforcePermissionAnnotation]
		public class TestClass5 extends IFoo.Stub {
		~~~~~~~~~
		1 errors, 0 warnings""".addLineContinuation())
		.expect("""
		src/test/pkg/TestClass41.java:4: Error: The method TestClass41.testMethod is annotated with @android.annotation.EnforcePermission \
		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() {}
		~~~~~~~~~~
		1 errors, 0 warnings
		""".addLineContinuation())
		}

		fun testDetectIssuesMissingAnnotationOnMethod() {
		fun testDetectIssuesMismatchingAnyAnnotationOnMethod() {
		lint().files(java(
		"""
		package test.pkg;
		public class TestClass6 extends IFooMethod.Stub {
		public void testMethod() {}
		public class TestClass9 extends IFooMethod.Stub {
		@android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.NFC})
		public void testMethodAny() {}
		}
		""").indented(),
		*stubs
		)
		.run()
		.expect("""src/test/pkg/TestClass6.java:3: Error: The method TestClass6.testMethod \
		overrides the method Stub.testMethod which is annotated with @EnforcePermission. The same \
		annotation must be used on TestClass6.testMethod [MissingEnforcePermissionAnnotation]
		public void testMethod() {}
		~~~~~~~~~~
		1 errors, 0 warnings""".addLineContinuation())
		.expect("""
		src/test/pkg/TestClass9.java:4: Error: The method TestClass9.testMethodAny is annotated with \
		@android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.NFC}) \
		which differs from the overridden method Stub.testMethodAny: \
		@android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE}). \
		The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
		public void testMethodAny() {}
		~~~~~~~~~~~~~
		1 errors, 0 warnings
		""".addLineContinuation())
		}

		fun testDetectIssuesExtraAnnotationMethod() {
		fun testDetectIssuesMismatchingAnyLiteralAnnotationOnMethod() {
		lint().files(java(
		"""
		package test.pkg;
		public class TestClass7 extends IBar.Stub {
		@android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
		public class TestClass91 extends IFooMethod.Stub {
		@android.annotation.EnforcePermission(anyOf={"android.permission.INTERNET", "android.permissionoopsthisisatypo.READ_PHONE_STATE"})
		public void testMethodAnyLiteral() {}
		}
		""").indented(),
		*stubs
		)
		.run()
		.expect("""
		src/test/pkg/TestClass91.java:4: Error: The method TestClass91.testMethodAnyLiteral is annotated with \
		@android.annotation.EnforcePermission(anyOf={"android.permission.INTERNET", "android.permissionoopsthisisatypo.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
		""".addLineContinuation())
		}

		fun testDetectIssuesMismatchingAllAnnotationOnMethod() {
		lint().files(java(
		"""
		package test.pkg;
		public class TestClass10 extends IFooMethod.Stub {
		@android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.NFC})
		public void testMethodAll() {}
		}
		""").indented(),
		*stubs
		)
		.run()
		.expect("""
		src/test/pkg/TestClass10.java:4: Error: The method TestClass10.testMethodAll is annotated with \
		@android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.NFC}) \
		which differs from the overridden method Stub.testMethodAll: \
		@android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE}). \
		The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
		public void testMethodAll() {}
		~~~~~~~~~~~~~
		1 errors, 0 warnings
		""".addLineContinuation())
		}

		fun testDetectIssuesMismatchingAllLiteralAnnotationOnMethod() {
		lint().files(java(
		"""
		package test.pkg;
		public class TestClass101 extends IFooMethod.Stub {
		@android.annotation.EnforcePermission(allOf={"android.permission.INTERNET", "android.permissionoopsthisisatypo.READ_PHONE_STATE"})
		public void testMethodAllLiteral() {}
		}
		""").indented(),
		*stubs
		)
		.run()
		.expect("""
		src/test/pkg/TestClass101.java:4: Error: The method TestClass101.testMethodAllLiteral is annotated with \
		@android.annotation.EnforcePermission(allOf={"android.permission.INTERNET", "android.permissionoopsthisisatypo.READ_PHONE_STATE"}) \
		which differs from the overridden method Stub.testMethodAllLiteral: \
		@android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}). \
		The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
		public void testMethodAllLiteral() {}
		~~~~~~~~~~~~~~~~~~~~
		1 errors, 0 warnings
		""".addLineContinuation())
		}

		fun testDetectIssuesMissingAnnotationOnMethod() {
		lint().files(java(
		"""
		package test.pkg;
		public class TestClass6 extends IFooMethod.Stub {
		public void testMethod() {}
		}
		""").indented(),
		*stubs
		)
		.run()
		.expect("""src/test/pkg/TestClass7.java:4: Error: The method TestClass7.testMethod \
		overrides the method Stub.testMethod which is not annotated with @EnforcePermission. The same \
		annotation must be used on Stub.testMethod. Did you forget to annotate the AIDL definition? \
		[MissingEnforcePermissionAnnotation]
		.expect("""
		src/test/pkg/TestClass6.java:3: Error: The method TestClass6.testMethod overrides the method Stub.testMethod which is annotated with @EnforcePermission. \
		The same annotation must be used on TestClass6.testMethod [MissingEnforcePermissionAnnotation]
		public void testMethod() {}
		~~~~~~~~~~
		1 errors, 0 warnings""".addLineContinuation())
		1 errors, 0 warnings
		""".addLineContinuation())
		}

		fun testDetectIssuesExtraAnnotationInterface() {
		fun testDetectIssuesExtraAnnotationMethod() {
		lint().files(java(
		"""
		package test.pkg;
		public class TestClass7 extends IBar.Stub {
		@android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
		public class TestClass8 extends IBar.Stub {
		public void testMethod() {}
		}
		""").indented(),
		*stubs
		)
		.run()
		.expect("""src/test/pkg/TestClass8.java:2: Error: The class test.pkg.TestClass8 \
		extends the class IBar.Stub which is not annotated with @EnforcePermission. The same annotation \
		must be used on IBar.Stub. Did you forget to annotate the AIDL definition? \
		[MissingEnforcePermissionAnnotation]
		@android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
		^
		1 errors, 0 warnings""".addLineContinuation())
		.expect("""
		src/test/pkg/TestClass7.java:4: Error: The method TestClass7.testMethod overrides the method Stub.testMethod which is not annotated with @EnforcePermission. \
		The same annotation must be used on Stub.testMethod. Did you forget to annotate the AIDL definition? [MissingEnforcePermissionAnnotation]
		public void testMethod() {}
		~~~~~~~~~~
		1 errors, 0 warnings
		""".addLineContinuation())
		}

		fun testDetectIssuesMissingAnnotationOnMethodWhenClassIsCalledDefault() {
		@@ -213,21 +323,6 @@ must be used on IBar.Stub. Did you forget to annotate the AIDL definition? \

		/* Stubs */

		// A service with permission annotation on the class.
		private val interfaceIFooStub: TestFile = java(
		"""
		@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
		public interface IFoo {
		@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
		public static abstract class Stub extends android.os.Binder implements IFoo {
		@Override
		public void testMethod() {}
		}
		public void testMethod();
		}
		"""
		).indented()

		// A service with permission annotation on the method.
		private val interfaceIFooMethodStub: TestFile = java(
		"""
		@@ -236,9 +331,29 @@ must be used on IBar.Stub. Did you forget to annotate the AIDL definition? \
		@Override
		@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
		public void testMethod() {}
		@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(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
		public void testMethodAll() {}
		@Override
		@android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"})
		public void testMethodAllLiteral() {}
		}
		@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
		public void testMethod();
		@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(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"})
		public void testMethodAllLiteral() {}
		}
		"""
		).indented()
		@@ -261,6 +376,7 @@ must be used on IBar.Stub. Did you forget to annotate the AIDL definition? \
		package android.Manifest;
		class permission {
		public static final String READ_PHONE_STATE = "android.permission.READ_PHONE_STATE";
		public static final String NFC = "android.permission.NFC";
		public static final String INTERNET = "android.permission.INTERNET";
		}
		"""
		@@ -273,7 +389,7 @@ must be used on IBar.Stub. Did you forget to annotate the AIDL definition? \
		"""
		).indented()

		private val stubs = arrayOf(interfaceIFooStub, interfaceIFooMethodStub, interfaceIBarStub,
		private val stubs = arrayOf(interfaceIFooMethodStub, interfaceIBarStub,
		manifestPermissionStub, enforcePermissionAnnotationStub)

		// Substitutes "backslash + new line" with an empty string to imitate line continuation