Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0dda935b authored by Thiébaud Weksteen's avatar Thiébaud Weksteen
Browse files

Compare nested values for EnforcePermissionDetector

Ensure that anyOf or allOf nested values are compared.

Test: java -cp AndroidGlobalLintCheckerTest.jar \
      org.junit.runner.JUnitCore \
      com.google.android.lint.aidl.EnforcePermissionDetectorTest
Bug: 261871823
Change-Id: I3415c5c4bf461805ca25b6d57b0bd6e215bef927
parent 9a512e9c
Loading
Loading
Loading
Loading
+28 −10
Original line number Diff line number Diff line
@@ -31,7 +31,9 @@ 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
@@ -65,6 +67,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 +90,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
    }
+207 −0
Original line number Diff line number Diff line
@@ -65,6 +65,74 @@ class EnforcePermissionDetectorTest : LintDetectorTest() {
        .expectClean()
    }

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

    fun testDoesNotDetectIssuesCorrectAnnotationAllLiteralOnMethod() {
        lint().files(java(
            """
            package test.pkg;
            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()
        .expectClean()
    }

    fun testDetectIssuesMismatchingAnnotationOnClass() {
        lint().files(java(
            """
@@ -109,6 +177,124 @@ annotation must be used for both methods. [MismatchingEnforcePermissionAnnotatio
1 errors, 0 warnings""".addLineContinuation())
    }

    fun testDetectIssuesEmptyAnnotationOnMethod() {
        lint().files(java(
            """
            package test.pkg;
            public class TestClass41 extends IFooMethod.Stub {
                @android.annotation.EnforcePermission
                public void testMethod() {}
            }
            """).indented(),
                *stubs
        )
        .run()
        .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 testDetectIssuesMismatchingAnyAnnotationOnMethod() {
        lint().files(java(
            """
            package test.pkg;
            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/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 testDetectIssuesMismatchingAnyLiteralAnnotationOnMethod() {
        lint().files(java(
            """
            package test.pkg;
            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 testDetectIssuesMissingAnnotationOnClass() {
        lint().files(java(
            """
@@ -236,9 +422,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 +467,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";
        }
        """