Skip to content
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.

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

Detect dependencies by listing MANIFEST.MF files at runtime #2538

Merged
merged 8 commits into from
Feb 16, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Features

- Add `main` flag to threads and `in_foreground` flag for app contexts ([#2516](https://github.com/getsentry/sentry-java/pull/2516))
- Detect dependencies by listing MANIFEST.MF files at runtime ([#2538](https://github.com/getsentry/sentry-java/pull/2538))

### Fixes

Expand Down
13 changes: 13 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -2417,10 +2417,18 @@ public final class io/sentry/internal/gestures/UiElement$Type : java/lang/Enum {
public static fun values ()[Lio/sentry/internal/gestures/UiElement$Type;
}

public final class io/sentry/internal/modules/CompositeModulesLoader : io/sentry/internal/modules/ModulesLoader {
public fun <init> (Ljava/util/List;Lio/sentry/ILogger;)V
}

public abstract interface class io/sentry/internal/modules/IModulesLoader {
public abstract fun getOrLoadModules ()Ljava/util/Map;
}

public final class io/sentry/internal/modules/ManifestModulesLoader : io/sentry/internal/modules/ModulesLoader {
public fun <init> (Lio/sentry/ILogger;)V
}

public abstract class io/sentry/internal/modules/ModulesLoader : io/sentry/internal/modules/IModulesLoader {
public static final field EXTERNAL_MODULES_FILENAME Ljava/lang/String;
protected final field logger Lio/sentry/ILogger;
Expand Down Expand Up @@ -3634,6 +3642,11 @@ public abstract class io/sentry/transport/TransportResult {
public static fun success ()Lio/sentry/transport/TransportResult;
}

public final class io/sentry/util/ClassLoaderUtils {
public fun <init> ()V
public static fun classLoaderOrDefault (Ljava/lang/ClassLoader;)Ljava/lang/ClassLoader;
}

public final class io/sentry/util/CollectionUtils {
public static fun filterListEntries (Ljava/util/List;Lio/sentry/util/CollectionUtils$Predicate;)Ljava/util/List;
public static fun filterMapEntries (Ljava/util/Map;Lio/sentry/util/CollectionUtils$Predicate;)Ljava/util/Map;
Expand Down
18 changes: 15 additions & 3 deletions sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,22 @@
import io.sentry.cache.EnvelopeCache;
import io.sentry.cache.IEnvelopeCache;
import io.sentry.config.PropertiesProviderFactory;
import io.sentry.internal.modules.CompositeModulesLoader;
import io.sentry.internal.modules.IModulesLoader;
import io.sentry.internal.modules.ManifestModulesLoader;
import io.sentry.internal.modules.NoOpModulesLoader;
import io.sentry.internal.modules.ResourcesModulesLoader;
import io.sentry.protocol.SentryId;
import io.sentry.protocol.User;
import io.sentry.transport.NoOpEnvelopeCache;
import io.sentry.util.FileUtils;
import io.sentry.util.Platform;
import io.sentry.util.thread.IMainThreadChecker;
import io.sentry.util.thread.MainThreadChecker;
import io.sentry.util.thread.NoOpMainThreadChecker;
import java.io.File;
import java.lang.reflect.InvocationTargetException;
import java.util.Arrays;
import java.util.List;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
Expand Down Expand Up @@ -276,10 +280,18 @@ private static boolean initConfigurations(final @NotNull SentryOptions options)
});
}

final IModulesLoader modulesLoader = options.getModulesLoader();
// only override the ModulesLoader if it's not already set by Android
final @NotNull IModulesLoader modulesLoader = options.getModulesLoader();
if (modulesLoader instanceof NoOpModulesLoader) {
options.setModulesLoader(new ResourcesModulesLoader(options.getLogger()));
if (Platform.isJvm()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: I think we don't need this check, 'cause this was the codepath for JVM platforms anyway, so I think CompositeModulesLoader covers everything already

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK Great, I presume this also won't cause problems with other SDKs using the Java SDK, correct?

options.setModulesLoader(
new CompositeModulesLoader(
Arrays.asList(
new ManifestModulesLoader(options.getLogger()),
new ResourcesModulesLoader(options.getLogger())),
options.getLogger()));
} else {
options.setModulesLoader(new ResourcesModulesLoader(options.getLogger()));
}
}

final IMainThreadChecker mainThreadChecker = options.getMainThreadChecker();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.sentry.config;

import static io.sentry.util.ClassLoaderUtils.classLoaderOrDefault;

import io.sentry.ILogger;
import io.sentry.SentryLevel;
import java.io.BufferedInputStream;
Expand All @@ -18,12 +20,7 @@ final class ClasspathPropertiesLoader implements PropertiesLoader {
public ClasspathPropertiesLoader(
@NotNull String fileName, @Nullable ClassLoader classLoader, @NotNull ILogger logger) {
this.fileName = fileName;
// bootstrap classloader is represented as null, so using system classloader instead
if (classLoader == null) {
this.classLoader = ClassLoader.getSystemClassLoader();
} else {
this.classLoader = classLoader;
}
this.classLoader = classLoaderOrDefault(classLoader);
this.logger = logger;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package io.sentry.internal.modules;

import io.sentry.ILogger;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@ApiStatus.Experimental
@ApiStatus.Internal
public final class CompositeModulesLoader extends ModulesLoader {

private final List<IModulesLoader> loaders;

public CompositeModulesLoader(
final @NotNull List<IModulesLoader> loaders, final @NotNull ILogger logger) {
super(logger);
this.loaders = loaders;
}

@Override
protected Map<String, String> loadModules() {
final @NotNull TreeMap<String, String> allModules = new TreeMap<>();

for (IModulesLoader loader : this.loaders) {
final @Nullable Map<String, String> modules = loader.getOrLoadModules();
if (modules != null) {
allModules.putAll(modules);
}
}

return allModules;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package io.sentry.internal.modules;

import static io.sentry.util.ClassLoaderUtils.classLoaderOrDefault;

import io.sentry.ILogger;
import io.sentry.SentryLevel;
import java.io.IOException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@ApiStatus.Experimental
@ApiStatus.Internal
public final class ManifestModulesLoader extends ModulesLoader {
private final Pattern URL_LIB_PATTERN = Pattern.compile(".*/(.+)!/META-INF/MANIFEST.MF");
private final Pattern NAME_AND_VERSION = Pattern.compile("(.*?)-(\\d+\\.\\d+.*).jar");
private final ClassLoader classLoader;

public ManifestModulesLoader(final @NotNull ILogger logger) {
this(ManifestModulesLoader.class.getClassLoader(), logger);
}

ManifestModulesLoader(final @Nullable ClassLoader classLoader, final @NotNull ILogger logger) {
super(logger);
this.classLoader = classLoaderOrDefault(classLoader);
}

@Override
protected Map<String, String> loadModules() {
final @NotNull Map<String, String> modules = new HashMap<>();
List<Module> detectedModules = detectModulesViaManifestFiles();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: one minor question - are the modules alphabetically sorted within the MANIFEST.MF file? If not, I guess we'd have to sort them, not sure if there's sorting of those somewhere on relay or frontend.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're not sorted when sending the JSON but show up sorted in the UI. Guess that's fine then?


for (Module module : detectedModules) {
modules.put(module.name, module.version);
}

return modules;
}

private @NotNull List<Module> detectModulesViaManifestFiles() {
final @NotNull List<Module> modules = new ArrayList<>();
try {
final @NotNull Enumeration<URL> manifestUrls =
classLoader.getResources("META-INF/MANIFEST.MF");
while (manifestUrls.hasMoreElements()) {
final @NotNull URL manifestUrl = manifestUrls.nextElement();
final @Nullable String originalName = extractDependencyNameFromUrl(manifestUrl);
final @Nullable Module module = convertOriginalNameToModule(originalName);
if (module != null) {
modules.add(module);
}
}
} catch (IOException e) {
adinauer marked this conversation as resolved.
Show resolved Hide resolved
logger.log(SentryLevel.ERROR, "Unable to detect modules via manifest files.", e);
}

return modules;
}

private @Nullable Module convertOriginalNameToModule(@Nullable String originalName) {
if (originalName == null) {
return null;
}

final @NotNull Matcher matcher = NAME_AND_VERSION.matcher(originalName);
if (matcher.matches() && matcher.groupCount() == 2) {
@NotNull String moduleName = matcher.group(1);
@NotNull String moduleVersion = matcher.group(2);
return new Module(moduleName, moduleVersion);
}

return null;
}

private @Nullable String extractDependencyNameFromUrl(final @NotNull URL url) {
final @NotNull String urlString = url.toString();
final @NotNull Matcher matcher = URL_LIB_PATTERN.matcher(urlString);
if (matcher.matches() && matcher.groupCount() == 1) {
return matcher.group(1);
}

return null;
}

private static final class Module {
private final @NotNull String name;
private final @NotNull String version;

public Module(final @NotNull String name, final @NotNull String version) {
this.name = name;
this.version = version;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.sentry.internal.modules;

import static io.sentry.util.ClassLoaderUtils.classLoaderOrDefault;

import io.sentry.ILogger;
import io.sentry.SentryLevel;
import java.io.InputStream;
Expand All @@ -20,12 +22,7 @@ public ResourcesModulesLoader(final @NotNull ILogger logger) {

ResourcesModulesLoader(final @NotNull ILogger logger, final @Nullable ClassLoader classLoader) {
super(logger);
// bootstrap classloader is represented as null, so using system classloader instead
if (classLoader == null) {
this.classLoader = ClassLoader.getSystemClassLoader();
} else {
this.classLoader = classLoader;
}
this.classLoader = classLoaderOrDefault(classLoader);
}

@Override
Expand Down
16 changes: 16 additions & 0 deletions sentry/src/main/java/io/sentry/util/ClassLoaderUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package io.sentry.util;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

public final class ClassLoaderUtils {

public static @NotNull ClassLoader classLoaderOrDefault(final @Nullable ClassLoader classLoader) {
// bootstrap classloader is represented as null, so using system classloader instead
if (classLoader == null) {
return ClassLoader.getSystemClassLoader();
} else {
return classLoader;
}
}
}
4 changes: 2 additions & 2 deletions sentry/src/test/java/io/sentry/SentryTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package io.sentry

import io.sentry.cache.EnvelopeCache
import io.sentry.cache.IEnvelopeCache
import io.sentry.internal.modules.CompositeModulesLoader
import io.sentry.internal.modules.IModulesLoader
import io.sentry.internal.modules.ResourcesModulesLoader
import io.sentry.protocol.SentryId
import io.sentry.util.thread.IMainThreadChecker
import io.sentry.util.thread.MainThreadChecker
Expand Down Expand Up @@ -309,7 +309,7 @@ class SentryTest {
sentryOptions = it
}

assertTrue { sentryOptions!!.modulesLoader is ResourcesModulesLoader }
assertTrue { sentryOptions!!.modulesLoader is CompositeModulesLoader }
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package io.sentry.internal.modules

import io.sentry.ILogger
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import org.mockito.kotlin.verifyNoMoreInteractions
import org.mockito.kotlin.whenever
import kotlin.test.Test
import kotlin.test.assertEquals

class CompositeModulesLoaderTest {

@Test
fun `reads modules from multiple loaders and caches result`() {
val logger = mock<ILogger>()
val loader1 = mock<IModulesLoader>()
val loader2 = mock<IModulesLoader>()

whenever(loader1.orLoadModules).thenReturn(mapOf("spring-core" to "6.0.0"))
whenever(loader2.orLoadModules).thenReturn(mapOf("spring-webmvc" to "6.0.2"))

val sut = CompositeModulesLoader(listOf(loader1, loader2), logger)

assertEquals(
mapOf(
"spring-core" to "6.0.0",
"spring-webmvc" to "6.0.2"
),
sut.orLoadModules
)

verify(loader1).orLoadModules
verify(loader2).orLoadModules

assertEquals(
mapOf(
"spring-core" to "6.0.0",
"spring-webmvc" to "6.0.2"
),
sut.orLoadModules
)

verifyNoMoreInteractions(loader1, loader2)
}
}
Loading