TrustAllX509TrustManager lint check incorrectly flags interfaces that extends X509TrustManager [270065082]

Fixed

Bug

Status Update

No update yet.

Description

ry...@intune.corp-partner.google.com

created issue #1

Feb 21, 2023 04:16PM

DESCRIBE THE ISSUE IN DETAIL:

STEPS TO REPRODUCE: 1.Create an interface that extends X509TrustManager

public interface ExtendedX509TrustManager extends X509TrustManager {
    List<X509Certificate> checkServerTrusted(X509Certificate[] chain, String authType, String host)
            throws CertificateException;
}

Extend the new interface correctly implementing checkServerTrusted and checkClientTrusted.
run gradlew lintDebug

EXPECTED: lint should pass ACTUAL: lint flags ExtendedX509TrustManager interface for not implementing checkServerTrusted or checkClientTrusted even though it is just and interface. The classes that fully implement the interface are not flagged (as expected).

ATTACH SCREENSHOTS/RECORDINGS OF THE ISSUE

ATTACH LOG FILES (Select Help > Show Log in Files, or Show Log in Finder on a Mac)

IMPORTANT: Please read https://developer.android.com/studio/report-bugs.html carefully and supply all required information.

Studio Build: Version of Gradle Plugin: Version of Gradle: Version of Java: OS:

Comments

ry...@intune.corp-partner.google.com <ry...@intune.corp-partner.google.com> #2Feb 21, 2023 04:20PM

This looks like a bug in the lint detector. The code checks if the method has no operations in the method body. It seems like this check results in a false positive for interface methods.

https://cs.android.com/android-studio/platform/tools/base/+/mirror-goog-studio-main:lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/X509TrustManagerDetector.java;l=180?q=X509TrustManagerDetector&ss=android-studio%2Fplatform%2Ftools%2Fbase

This bug is difficult to ignore because SuppressLint is ignored by consuming apps. The only workaround appears to be to suppress using a regex match in lint.xml

ha...@google.com <ha...@google.com> Feb 21, 2023 06:22PM

Assigned to an...@google.com.

tn...@google.com <tn...@google.com> Feb 23, 2023 08:26PM

Reassigned to tn...@google.com.

tn...@google.com <tn...@google.com> #3Feb 24, 2023 10:59PM

Marked as fixed.

Fixed by https://cs.android.com/android-studio/platform/tools/base/+/448a2a18826f6e4ed5d1e955f04155230a2ec3c4 for an upcoming 8.1/Giraffe canary.

(Note that there are two X509 checks; one which complains if it looks like these methods aren't overridden, and that's the bug fixed here. There's a separate check which discourages you from implementing this interface at all since it's risky/errorprone, and that check is unaffected.)

da...@robinhood.com <da...@robinhood.com> #4Aug 9, 2023 04:21PM

We ran into this recently with BouncyCastle - the fix applied here doesn't seem to apply to abstract classes, so the lint check still picks up abstract methods. It seems the best fix here would probably be to ensure that the method isn't abstract?

Issue 270065082