diff --git a/CHANGELOG.md b/CHANGELOG.md index 55ef1a8f2c0..2a29c72a3d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ ### Fixes +- Close manifest input streams after version detection to avoid retaining JAR resources ([#6125](https://github.com/getsentry/sentry-java/pull/6125)) - Keep resolving the server name after `Sentry.close()` or a re-init. Closing the SDK shut down the shared hostname cache for the life of the process, so `server_name` silently froze at the value it had last resolved ([#6119](https://github.com/getsentry/sentry-java/pull/6119)) ### Internal diff --git a/sentry/src/main/java/io/sentry/internal/ManifestVersionReader.java b/sentry/src/main/java/io/sentry/internal/ManifestVersionReader.java index 3a0161b1879..753637ccb3e 100644 --- a/sentry/src/main/java/io/sentry/internal/ManifestVersionReader.java +++ b/sentry/src/main/java/io/sentry/internal/ManifestVersionReader.java @@ -4,13 +4,16 @@ import io.sentry.SentryIntegrationPackageStorage; import io.sentry.util.AutoClosableReentrantLock; import java.io.IOException; +import java.io.InputStream; import java.net.URL; +import java.net.URLConnection; import java.util.Enumeration; import java.util.jar.Attributes; import java.util.jar.Manifest; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; @ApiStatus.Internal public final class ManifestVersionReader { @@ -18,6 +21,7 @@ public final class ManifestVersionReader { private static final @NotNull AutoClosableReentrantLock staticLock = new AutoClosableReentrantLock(); private volatile boolean hasManifestBeenRead = false; + private final @NotNull ClassLoader classLoader; private final @NotNull VersionInfoHolder versionInfo = new VersionInfoHolder(); private @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); @@ -33,7 +37,14 @@ public final class ManifestVersionReader { return INSTANCE; } - private ManifestVersionReader() {} + private ManifestVersionReader() { + this(ClassLoader.getSystemClassLoader()); + } + + @TestOnly + ManifestVersionReader(final @NotNull ClassLoader classLoader) { + this.classLoader = classLoader; + } public @Nullable VersionInfoHolder readOpenTelemetryVersion() { readManifestFiles(); @@ -52,52 +63,59 @@ public void readManifestFiles() { if (hasManifestBeenRead) { return; } - final @NotNull Enumeration resources = - ClassLoader.getSystemClassLoader().getResources("META-INF/MANIFEST.MF"); + final @NotNull Enumeration resources = classLoader.getResources("META-INF/MANIFEST.MF"); while (resources.hasMoreElements()) { try { - final @NotNull Manifest manifest = new Manifest(resources.nextElement().openStream()); - final @Nullable Attributes mainAttributes = manifest.getMainAttributes(); - if (mainAttributes != null) { - final @Nullable String name = mainAttributes.getValue("Sentry-Opentelemetry-SDK-Name"); - final @Nullable String version = mainAttributes.getValue("Implementation-Version"); - final @Nullable String sdkName = mainAttributes.getValue("Sentry-SDK-Name"); - final @Nullable String packageName = mainAttributes.getValue("Sentry-SDK-Package-Name"); + final @NotNull URLConnection connection = resources.nextElement().openConnection(); + // Avoid retaining JarFile and inflater resources in the default cache for jar: URLs. + connection.setUseCaches(false); + try (final @NotNull InputStream inputStream = connection.getInputStream()) { + final @NotNull Manifest manifest = new Manifest(inputStream); + final @Nullable Attributes mainAttributes = manifest.getMainAttributes(); + if (mainAttributes != null) { + final @Nullable String name = + mainAttributes.getValue("Sentry-Opentelemetry-SDK-Name"); + final @Nullable String version = mainAttributes.getValue("Implementation-Version"); + final @Nullable String sdkName = mainAttributes.getValue("Sentry-SDK-Name"); + final @Nullable String packageName = + mainAttributes.getValue("Sentry-SDK-Package-Name"); - if (name != null && version != null) { - versionInfo.sdkName = name; - versionInfo.sdkVersion = version; - final @Nullable String otelVersion = - mainAttributes.getValue("Sentry-Opentelemetry-Version-Name"); - if (otelVersion != null) { - SentryIntegrationPackageStorage.getInstance() - .addPackage("maven:io.opentelemetry:opentelemetry-sdk", otelVersion); - SentryIntegrationPackageStorage.getInstance().addIntegration("OpenTelemetry"); - } - final @Nullable String otelJavaagentVersion = - mainAttributes.getValue("Sentry-Opentelemetry-Javaagent-Version-Name"); - if (otelJavaagentVersion != null) { - SentryIntegrationPackageStorage.getInstance() - .addPackage( - "maven:io.opentelemetry.javaagent:opentelemetry-javaagent", - otelJavaagentVersion); - SentryIntegrationPackageStorage.getInstance().addIntegration("OpenTelemetry-Agent"); + if (name != null && version != null) { + versionInfo.sdkName = name; + versionInfo.sdkVersion = version; + final @Nullable String otelVersion = + mainAttributes.getValue("Sentry-Opentelemetry-Version-Name"); + if (otelVersion != null) { + SentryIntegrationPackageStorage.getInstance() + .addPackage("maven:io.opentelemetry:opentelemetry-sdk", otelVersion); + SentryIntegrationPackageStorage.getInstance().addIntegration("OpenTelemetry"); + } + final @Nullable String otelJavaagentVersion = + mainAttributes.getValue("Sentry-Opentelemetry-Javaagent-Version-Name"); + if (otelJavaagentVersion != null) { + SentryIntegrationPackageStorage.getInstance() + .addPackage( + "maven:io.opentelemetry.javaagent:opentelemetry-javaagent", + otelJavaagentVersion); + SentryIntegrationPackageStorage.getInstance() + .addIntegration("OpenTelemetry-Agent"); + } + if (name.equals("sentry.java.opentelemetry.agentless")) { + SentryIntegrationPackageStorage.getInstance() + .addIntegration("OpenTelemetry-Agentless"); + } + if (name.equals("sentry.java.opentelemetry.agentless-spring")) { + SentryIntegrationPackageStorage.getInstance() + .addIntegration("OpenTelemetry-Agentless-Spring"); + } } - if (name.equals("sentry.java.opentelemetry.agentless")) { - SentryIntegrationPackageStorage.getInstance() - .addIntegration("OpenTelemetry-Agentless"); - } - if (name.equals("sentry.java.opentelemetry.agentless-spring")) { - SentryIntegrationPackageStorage.getInstance() - .addIntegration("OpenTelemetry-Agentless-Spring"); - } - } - if (sdkName != null - && version != null - && packageName != null - && sdkName.startsWith("sentry.java")) { - SentryIntegrationPackageStorage.getInstance().addPackage(packageName, version); + if (sdkName != null + && version != null + && packageName != null + && sdkName.startsWith("sentry.java")) { + SentryIntegrationPackageStorage.getInstance().addPackage(packageName, version); + } } } } catch (Exception e) { diff --git a/sentry/src/test/java/io/sentry/internal/ManifestVersionReaderTest.kt b/sentry/src/test/java/io/sentry/internal/ManifestVersionReaderTest.kt new file mode 100644 index 00000000000..bd456efe3b2 --- /dev/null +++ b/sentry/src/test/java/io/sentry/internal/ManifestVersionReaderTest.kt @@ -0,0 +1,78 @@ +package io.sentry.internal + +import com.google.common.truth.Truth.assertThat +import java.io.ByteArrayInputStream +import java.net.URL +import java.net.URLConnection +import java.nio.charset.StandardCharsets +import java.util.Collections +import kotlin.test.Test +import org.mockito.kotlin.inOrder +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever + +class ManifestVersionReaderTest { + private class CloseTrackingInputStream(content: String) : + ByteArrayInputStream(content.toByteArray(StandardCharsets.UTF_8)) { + var isClosed = false + + override fun close() { + isClosed = true + super.close() + } + } + + private class Fixture(contents: List) { + val classLoader = mock() + val inputStreams = contents.map(::CloseTrackingInputStream) + val connections = contents.map { mock() } + val urls = contents.map { mock() } + + init { + whenever(classLoader.getResources("META-INF/MANIFEST.MF")) + .thenReturn(Collections.enumeration(urls)) + urls.indices.forEach { index -> + whenever(urls[index].openConnection()).thenReturn(connections[index]) + whenever(connections[index].inputStream).thenReturn(inputStreams[index]) + } + } + + val sut = ManifestVersionReader(classLoader) + } + + @Test + fun `closes manifest stream and disables connection caching before opening it`() { + val fixture = Fixture(listOf(validManifest())) + + fixture.sut.readManifestFiles() + + assertThat(fixture.inputStreams.single().isClosed).isTrue() + inOrder(fixture.connections.single()) { + verify(fixture.connections.single()).useCaches = false + verify(fixture.connections.single()).inputStream + } + } + + @Test + fun `closes malformed manifest stream and continues reading manifests`() { + val fixture = Fixture(listOf("not a manifest\n", validManifest())) + + val versionInfo = fixture.sut.readOpenTelemetryVersion() + + assertThat(fixture.inputStreams.map { it.isClosed }).containsExactly(true, true).inOrder() + assertThat(versionInfo).isNotNull() + assertThat(versionInfo!!.sdkName).isEqualTo("sentry.java.opentelemetry.test") + assertThat(versionInfo.sdkVersion).isEqualTo("1.2.3") + } + + companion object { + private fun validManifest() = + """ + Manifest-Version: 1.0 + Sentry-Opentelemetry-SDK-Name: sentry.java.opentelemetry.test + Implementation-Version: 1.2.3 + + """ + .trimIndent() + } +}