diff --git a/CHANGELOG.md b/CHANGELOG.md index e086126a..c974c52d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +* Ignore R8 minified libs from instrumentation (#316) + ## 3.1.0-beta.1 * Fix: obfuscated libs instrumentation (adcolony) (#307) diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/util/ConstantPoolHelpers.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/util/ConstantPoolHelpers.kt new file mode 100644 index 00000000..8e8b2bd7 --- /dev/null +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/util/ConstantPoolHelpers.kt @@ -0,0 +1,86 @@ +package io.sentry.android.gradle.instrumentation.util + +import java.lang.reflect.Field +import org.objectweb.asm.ClassReader +import org.objectweb.asm.ClassVisitor +import org.objectweb.asm.ClassWriter + +/** + * Looks up for the original [ClassWriter] up the visitor chain by looking at the private `cv` field + * of the [ClassVisitor]. + */ +internal fun ClassVisitor.findClassWriter(): ClassWriter? { + var classWriter: ClassVisitor = this + while (!ClassWriter::class.java.isAssignableFrom(classWriter::class.java)) { + val cvField: Field = try { + classWriter::class.java.allFields.find { it.name == "cv" } ?: return null + } catch (e: Throwable) { + return null + } + cvField.isAccessible = true + classWriter = (cvField.get(classWriter) as? ClassVisitor) ?: return null + } + return classWriter as ClassWriter +} + +/** + * Looks up for [ClassReader] of the [ClassWriter] through intermediate SymbolTable field. + */ +internal fun ClassWriter.findClassReader(): ClassReader? { + val clazz: Class = this::class.java + val symbolTableField: Field = try { + clazz.allFields.find { it.name == "symbolTable" } ?: return null + } catch (e: Throwable) { + return null + } + symbolTableField.isAccessible = true + val symbolTable = symbolTableField.get(this) + val classReaderField: Field = try { + symbolTable::class.java.getDeclaredField("sourceClassReader") + } catch (e: Throwable) { + return null + } + classReaderField.isAccessible = true + return (classReaderField.get(symbolTable) as? ClassReader) +} + +/** + * Looks at the constant pool entries and searches for R8 markers + */ +internal fun ClassReader.isMinifiedClass(): Boolean { + val charBuffer = CharArray(maxStringLength) + // R8 marker is usually in the first 3-5 entries, so we limit it at 10 to speed it up + // (constant pool size can be huge otherwise) + val poolSize = minOf(10, itemCount) + for (i in 1 until poolSize) { + try { + val constantPoolEntry = readConst(i, charBuffer) + if (constantPoolEntry is String && "~~R8" in constantPoolEntry) { + // ~~R8 is a marker in the class' constant pool, which r8 itself is looking at when + // parsing a .class file. See here -> https://r8.googlesource.com/r8/+/refs/heads/main/src/main/java/com/android/tools/r8/dex/Marker.java#53 + return true + } + } catch (e: Throwable) { + // we ignore exceptions here, because some constant pool entries are nulls and the + // readConst method throws IllegalArgumentException when trying to read those + } + } + return false +} + +/** + * Gets all fields of the given class and its parents (if any). + * + * Adapted from https://github.com/apache/commons-lang/blob/master/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java + */ +private val Class<*>.allFields: List + get() { + val allFields = mutableListOf() + var currentClass: Class<*>? = this + while (currentClass != null) { + val declaredFields = currentClass.declaredFields + allFields += declaredFields + currentClass = currentClass.superclass + } + return allFields + } diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt index 43f3019b..61778f42 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt @@ -4,14 +4,19 @@ package io.sentry.android.gradle.instrumentation.wrap import com.android.build.api.instrumentation.ClassContext import com.android.build.api.instrumentation.ClassData +import io.sentry.android.gradle.SentryPlugin import io.sentry.android.gradle.instrumentation.ClassInstrumentable import io.sentry.android.gradle.instrumentation.CommonClassVisitor import io.sentry.android.gradle.instrumentation.MethodContext import io.sentry.android.gradle.instrumentation.MethodInstrumentable import io.sentry.android.gradle.instrumentation.SpanAddingClassVisitorFactory import io.sentry.android.gradle.instrumentation.util.AnalyzingVisitor +import io.sentry.android.gradle.instrumentation.util.findClassReader +import io.sentry.android.gradle.instrumentation.util.findClassWriter +import io.sentry.android.gradle.instrumentation.util.isMinifiedClass import io.sentry.android.gradle.instrumentation.util.isSentryClass import io.sentry.android.gradle.instrumentation.wrap.visitor.WrappingVisitor +import io.sentry.android.gradle.util.info import org.objectweb.asm.ClassVisitor import org.objectweb.asm.MethodVisitor import org.objectweb.asm.tree.MethodNode @@ -26,6 +31,18 @@ class WrappingInstrumentable : ClassInstrumentable { ): ClassVisitor { val simpleClassName = instrumentableContext.currentClassData.className.substringAfterLast('.') + + val classReader = originalVisitor.findClassWriter()?.findClassReader() + val isMinifiedClass = classReader?.isMinifiedClass() ?: false + if (isMinifiedClass) { + // We only check for minified classes for the WrappingInstrumentable, because it is the + // only one which runs over all classes + SentryPlugin.logger.info { + "$simpleClassName skipped from instrumentation because it's a minified class." + } + return originalVisitor + } + return AnalyzingVisitor( apiVersion = apiVersion, nextVisitor = { methods -> diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/BaseSentryPluginTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/BaseSentryPluginTest.kt index 0653ef38..19de60e5 100644 --- a/plugin-build/src/test/kotlin/io/sentry/android/gradle/BaseSentryPluginTest.kt +++ b/plugin-build/src/test/kotlin/io/sentry/android/gradle/BaseSentryPluginTest.kt @@ -55,12 +55,13 @@ abstract class BaseSentryPluginTest( repositories { google() mavenCentral() + maven { url 'https://appboy.github.io/appboy-android-sdk/sdk' } } } subprojects { pluginManager.withPlugin('com.android.application') { android { - compileSdkVersion 30 + compileSdkVersion 31 defaultConfig { applicationId "com.example" minSdkVersion 21 diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/SentryPluginWithMinifiedLibsTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/SentryPluginWithMinifiedLibsTest.kt index de42accb..b763a2df 100644 --- a/plugin-build/src/test/kotlin/io/sentry/android/gradle/SentryPluginWithMinifiedLibsTest.kt +++ b/plugin-build/src/test/kotlin/io/sentry/android/gradle/SentryPluginWithMinifiedLibsTest.kt @@ -7,7 +7,7 @@ class SentryPluginWithMinifiedLibsTest : BaseSentryPluginTest(androidGradlePluginVersion = "7.1.2", gradleVersion = "7.4") { @Test - fun `does not break when there is a play-core obfuscated dependency`() { + fun `does not break when there is a minified jar dependency`() { appBuildFile.writeText( // language=Groovy """ @@ -22,6 +22,7 @@ class SentryPluginWithMinifiedLibsTest : implementation 'com.google.android.gms:play-services-vision:20.1.3' implementation 'com.google.android.gms:play-services-mlkit-text-recognition:18.0.0' implementation 'com.adcolony:sdk:4.7.1' + implementation 'com.appboy:android-sdk-ui:19.0.0' } """.trimIndent() ) diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/VisitorTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/VisitorTest.kt index e477a15f..76c8c981 100644 --- a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/VisitorTest.kt +++ b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/VisitorTest.kt @@ -136,6 +136,12 @@ class VisitorTest( ChainedInstrumentable(listOf(WrappingInstrumentable(), RemappingInstrumentable())), null ), + arrayOf( + "fileIO", + "BrazeImageUtils", + ChainedInstrumentable(listOf(WrappingInstrumentable(), RemappingInstrumentable())), + null + ), arrayOf("okhttp/v3", "RealCall", OkHttp(), null), arrayOf("okhttp/v4", "RealCall", OkHttp(), null) ) diff --git a/plugin-build/src/test/resources/testFixtures/instrumentation/fileIO/BrazeImageUtils.class b/plugin-build/src/test/resources/testFixtures/instrumentation/fileIO/BrazeImageUtils.class new file mode 100644 index 00000000..f1f25b98 Binary files /dev/null and b/plugin-build/src/test/resources/testFixtures/instrumentation/fileIO/BrazeImageUtils.class differ