Skip to content

Fix the logic for CustomSchemeDetector to comply with the documentation. #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+144 −71
Open

Fix the logic for CustomSchemeDetector to comply with the documentation. #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 67 additions & 36 deletions checks/src/main/java/com/example/lint/checks/CustomSchemeDetector.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ package com.example.lint.checks

import com.android.SdkConstants.ANDROID_URI
import com.android.SdkConstants.ATTR_AUTO_VERIFY
import com.android.SdkConstants.ATTR_SCHEME
import com.android.SdkConstants.TAG_ACTION
import com.android.SdkConstants.TAG_CATEGORY
import com.android.SdkConstants.TAG_DATA
import com.android.SdkConstants.TAG_INTENT_FILTER
import com.android.tools.lint.detector.api.Category
Expand All @@ -31,68 +34,96 @@ import com.android.tools.lint.detector.api.XmlScanner
import com.android.utils.childrenIterator
import org.w3c.dom.Element

/**
* Detector flagging whether the application has issues verifying custom schemes (i.e. not http/https/file/ftp/ftps).
*/
class CustomSchemeDetector : Detector(), XmlScanner {
override fun getApplicableElements() = setOf(TAG_INTENT_FILTER)

override fun visitElement(context: XmlContext, element: Element) {
val autoVerifyAttribute = element.getAttributeNS(ANDROID_URI, ATTR_AUTO_VERIFY)
if (!hasViewAction(element) ||
!hasBrowsableCategory(element) ||
!hasDefaultCategory(element)
) {
return
}

if (autoVerifyAttribute != "true") {
val incident =
Incident(
if (element.hasAttributeNS(ANDROID_URI, ATTR_AUTO_VERIFY)) {
return
}

val dataElements = element.getElementsByTagName(TAG_DATA)
if (dataElements.length == 0) {
return
}

for (i in 0 until dataElements.length) {
val data = dataElements.item(i) as Element
if (!data.hasAttributeNS(ANDROID_URI, ATTR_SCHEME)) {
continue
}

val scheme = data.getAttributeNS(ANDROID_URI, ATTR_SCHEME)
if (scheme == "http" || scheme == "https") {
context.report(
AUTOVERIFY_ATTRIBUTE_ISSUE,
element,
context.getLocation(element),
"Custom scheme intent filters should explicitly set the `autoVerify` attribute to true",
fix().set().android().attribute(ATTR_AUTO_VERIFY).value("true").build()
context.getNameLocation(element),
"This intent filter matches App Links (VIEW, BROWSABLE, DEFAULT, http/https), " +
"but is missing the `android:autoVerify=\"true\"` attribute. " +
"See https://developer.android.com/training/app-links/verify-android-applinks",
fix().set().namespace(ANDROID_URI).attribute(ATTR_AUTO_VERIFY).value("true").build()
)
return
}
}
}

if (hasCustomSchemes(element)) {
// Only have the lint check fire if there are custom schemes present
context.report(incident)
private fun hasViewAction(element: Element): Boolean {
val actions = element.getElementsByTagName(TAG_ACTION)
for (i in 0 until actions.length) {
val action = actions.item(i) as Element
if ("android.intent.action.VIEW" == action.getAttributeNS(ANDROID_URI, "name")) {
return true
}
}
return false
}

private fun hasCustomSchemes(element: Element): Boolean {
for (child in element.childrenIterator()) {
if (child.nodeName == TAG_DATA && child.hasAttributes()) {
for (i in 0 until child.attributes.length) {
val attribute = child.attributes.item(i)
val name = attribute.localName ?: continue
val value = attribute.nodeValue
private fun hasBrowsableCategory(element: Element): Boolean {
val categories = element.getElementsByTagName(TAG_CATEGORY)
for (i in 0 until categories.length) {
val category = categories.item(i) as Element
if ("android.intent.category.BROWSABLE" == category.getAttributeNS(ANDROID_URI, "name")) {
return true
}
}
return false
}

if (value !in REGULAR_SCHEMES) {
return true
}
}
private fun hasDefaultCategory(element: Element): Boolean {
val categories = element.getElementsByTagName(TAG_CATEGORY)
for (i in 0 until categories.length) {
val category = categories.item(i) as Element
if ("android.intent.category.DEFAULT" == category.getAttributeNS(ANDROID_URI, "name")) {
return true
}
}
return false
}

companion object {
private val REGULAR_SCHEMES = listOf("http", "https", "file", "ftp", "ftps")

private const val EXPLANATION = """
Intent filters should contain the `autoVerify` attribute and explicitly set it to true, in order \
to signal to the system to automatically verify the associated hosts in your app's intent filters.
"""
Intent filters that handle `http` or `https` schemes and include \
`android.intent.action.VIEW`, `android.intent.category.BROWSABLE`, and \
`android.intent.category.DEFAULT` should also include \
`android:autoVerify="true"`. This enables Android App Links, which securely \
associates your app with your domain. \
See https://developer.android.com/training/app-links/verify-android-applinks
"""

/** Issue describing the problem and pointing to the detector implementation. */
@JvmField
val AUTOVERIFY_ATTRIBUTE_ISSUE: Issue =
Issue.create(
// ID: used in @SuppressLint warnings etc
id = "MissingAutoVerifyAttribute",
// Title -- shown in the IDE's preference dialog, as category headers in the
// Analysis results window, etc
briefDescription = "Application has custom scheme intent filters with missing `autoVerify` attributes",
// Full explanation of the issue; you can use some markdown markup such as
// `monospace`, *italic*, and **bold**.
briefDescription = "Application has http/https scheme intent filters with missing `autoVerify` attributes",
explanation = EXPLANATION,
category = Category.SECURITY,
priority = 6,
Expand Down
112 changes: 77 additions & 35 deletions checks/src/test/java/com/example/lint/checks/CustomSchemeDetectorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ class CustomSchemeDetectorTest : LintDetectorTest() {
<activity android:name='com.example.MainActivity'>
<intent-filter android:autoVerify='true'>
<action android:name='android.intent.action.VIEW' />
<category android:name='android.intent.category.BROWSABLE' />
<category android:name='android.intent.category.DEFAULT' />
<data android:scheme='telegram://' />
</intent-filter>
</activity>
Expand All @@ -65,7 +67,7 @@ class CustomSchemeDetectorTest : LintDetectorTest() {
}

@Test
fun testWhenNoAutoVerifyAttributeSpecifiedOnRegularSchemeIntentFilter_noWarning() {
fun testWhenHttpSchemeButMissingCategories_noWarning() {
lint()
.files(
manifest("""
Expand All @@ -87,7 +89,30 @@ class CustomSchemeDetectorTest : LintDetectorTest() {
}

@Test
fun testWhenFalseAutoVerifyAttributeSpecifiedOnCustomSchemeIntentFilter_showsWarning() {
fun testWhenHttpSchemeButMissingAction_noWarning() {
lint()
.files(
manifest("""
<manifest xmlns:android='http://schemas.android.com/apk/res/android' package='test.pkg'>
<application android:debuggable='false'>
<meta-data android:name='android.webkit.WebView.EnableSafeBrowsing'/>
<activity android:name='com.example.MainActivity'>
<intent-filter>
<category android:name='android.intent.category.BROWSABLE' />
<category android:name='android.intent.category.DEFAULT' />
<data android:scheme='http' />
<data android:scheme='https' />
</intent-filter>
</activity>
</application>
</manifest>
"""
).indented()
).run().expectClean()
}

@Test
fun testWhenFalseAutoVerifyAttributeSpecifiedOnCustomSchemeIntentFilter_noWarning() {
lint().files(
manifest("""
<manifest xmlns:android='http://schemas.android.com/apk/res/android' package='test.pkg'>
Expand All @@ -96,25 +121,20 @@ class CustomSchemeDetectorTest : LintDetectorTest() {
<activity android:name='com.example.MainActivity'>
<intent-filter android:autoVerify='false'>
<action android:name='android.intent.action.VIEW' />
<category android:name='android.intent.category.BROWSABLE' />
<category android:name='android.intent.category.DEFAULT' />
<data android:scheme='telegram://' />
</intent-filter>
</activity>
</application>
</manifest>
"""
).indented()
).run().expect(
"""
AndroidManifest.xml:5: Warning: Custom scheme intent filters should explicitly set the autoVerify attribute to true [MissingAutoVerifyAttribute]
<intent-filter android:autoVerify='false'>
^
0 errors, 1 warnings
"""
)
).run().expectClean()
}

@Test
fun testWhenNoAutoVerifyAttributeSpecifiedOnCustomSchemeIntentFilter_showsWarning() {
fun testWhenNoAutoVerifyAttributeSpecifiedOnCustomSchemeIntentFilter_noWarning() {
lint().files(
manifest("""
<manifest xmlns:android='http://schemas.android.com/apk/res/android' package='test.pkg'>
Expand All @@ -123,25 +143,50 @@ class CustomSchemeDetectorTest : LintDetectorTest() {
<activity android:name='com.example.MainActivity'>
<intent-filter>
<action android:name='android.intent.action.VIEW' />
<category android:name='android.intent.category.BROWSABLE' />
<category android:name='android.intent.category.DEFAULT' />
<data android:scheme='telegram://' />
</intent-filter>
</activity>
</application>
</manifest>
"""
).indented()
).run().expect(
"""
AndroidManifest.xml:5: Warning: Custom scheme intent filters should explicitly set the autoVerify attribute to true [MissingAutoVerifyAttribute]
).run().expectClean()
}

@Test
fun testWhenAppLinkMissingAutoVerify_showsWarning() {
lint().files(
manifest("""
<manifest xmlns:android='http://schemas.android.com/apk/res/android' package='test.pkg'>
<application android:debuggable='false'>
<meta-data android:name='android.webkit.WebView.EnableSafeBrowsing'/>
<activity android:name='com.example.MainActivity'>
<intent-filter>
^
0 errors, 1 warnings
<action android:name='android.intent.action.VIEW' />
<category android:name='android.intent.category.BROWSABLE' />
<category android:name='android.intent.category.DEFAULT' />
<data android:scheme='http' />
<data android:scheme='https' />
</intent-filter>
</activity>
</application>
</manifest>
"""
)
).indented()
).run().expect(
"""
AndroidManifest.xml:5: Warning: This intent filter matches App Links (VIEW, BROWSABLE, DEFAULT, http/https), but is missing the android:autoVerify="true" attribute. See https://developer.android.com/training/app-links/verify-android-applinks [MissingAutoVerifyAttribute]
<intent-filter>
~~~~~~~~~~~~~
0 errors, 1 warnings
"""
)
}

@Test
fun testWhenNoAutoVerifyAttributeSpecifiedOnCustomSchemeIntentFilter_showsQuickFix() {
fun testWhenAppLinkMissingAutoVerify_showsQuickFix() {
lint().files(
manifest("""
<manifest xmlns:android='http://schemas.android.com/apk/res/android' package='test.pkg'>
Expand All @@ -150,25 +195,27 @@ class CustomSchemeDetectorTest : LintDetectorTest() {
<activity android:name='com.example.MainActivity'>
<intent-filter>
<action android:name='android.intent.action.VIEW' />
<data android:scheme='telegram://' />
<category android:name='android.intent.category.BROWSABLE' />
<category android:name='android.intent.category.DEFAULT' />
<data android:scheme='https' />
</intent-filter>
</activity>
</application>
</manifest>
"""
).indented()
).run().expectFixDiffs(
"""
Fix for AndroidManifest.xml line 5: Set autoVerify="true":
@@ -9 +9
- <intent-filter>
+ <intent-filter android:autoVerify="true" >
"""
"""
Fix for AndroidManifest.xml line 5: Set autoVerify="true":
@@ -9 +9
- <intent-filter>
+ <intent-filter android:autoVerify="true" >
"""
)
}

@Test
fun testWhenFalseAutoVerifyAttributeSpecifiedOnCustomSchemeIntentFilter_showsQuickFix() {
fun testWhenAppLinkHasAutoVerifyFalse_noWarning() {
lint().files(
manifest("""
<manifest xmlns:android='http://schemas.android.com/apk/res/android' package='test.pkg'>
Expand All @@ -177,20 +224,15 @@ class CustomSchemeDetectorTest : LintDetectorTest() {
<activity android:name='com.example.MainActivity'>
<intent-filter android:autoVerify='false'>
<action android:name='android.intent.action.VIEW' />
<data android:scheme='telegram://' />
<category android:name='android.intent.category.BROWSABLE' />
<category android:name='android.intent.category.DEFAULT' />
<data android:scheme='http' />
</intent-filter>
</activity>
</application>
</manifest>
"""
).indented()
).run().expectFixDiffs(
"""
Fix for AndroidManifest.xml line 5: Set autoVerify="true":
@@ -9 +9
- <intent-filter android:autoVerify="false" >
+ <intent-filter android:autoVerify="true" >
"""
)
).run().expectClean()
}
}