From eb8f7171eca64922961bb00f427d62dbea3ec9fc Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 27 May 2026 15:06:29 -0400 Subject: [PATCH] Build DatadogClassLoader resource URL from agent jar URL, not OS path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agentResourcePrefix` was constructed via `"jar:file:" + JarFile.getName() + "!/"`. `JarFile.getName()` returns the OS-native path. On Windows that is `C:\Datadog\dd-java-agent.jar`, producing the malformed URL `jar:file:C:\Datadog\dd-java-agent.jar!/...` — the backslashes are not valid in a URL path and the embedded `file:C:\...` is missing the leading slash before the drive letter. `new URL(...)` is permissive enough to accept the string, but `URLConnection.openStream()` then fails, so `findResource()` returns URLs that cannot be read. This breaks helper-class injection on Spring Boot 1.3.5 fat jars (and WARs) on Windows: byte-buddy's `ClassFileLocator.ForClassLoader` surfaces the failure as `IllegalStateException: Could not locate class file for ` and `HelperInjector.getHelperMap` propagates it. Without the Servlet3 and Tomcat helpers no entry spans are created, so no traces reach the Agent. Use the well-formed `agentJarURL` (already built via `File.toURI().toURL()` upstream) instead. The migrated test exercises the regression cross-platform by placing the test jar in a directory whose name contains a space — `URL.toString()` percent-encodes it, `JarFile.getName()` does not — which is the same class of malformation Windows triggers. Migrate `DatadogClassLoaderTest.groovy` to Java JUnit 5 per project convention (uses reflection for the protected `getClassLoadingLock`). Fixes #6398 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../trace/bootstrap/DatadogClassLoader.java | 6 +- .../bootstrap/DatadogClassLoaderTest.groovy | 107 ----------- .../bootstrap/DatadogClassLoaderTest.java | 171 ++++++++++++++++++ 3 files changed, 176 insertions(+), 108 deletions(-) delete mode 100644 dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/DatadogClassLoaderTest.groovy create mode 100644 dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DatadogClassLoaderTest.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DatadogClassLoader.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DatadogClassLoader.java index 7ac89dfafb5..aaf263901ff 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DatadogClassLoader.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DatadogClassLoader.java @@ -41,7 +41,11 @@ public DatadogClassLoader(final URL agentJarURL, final ClassLoader parent) throw agentJarFile = new JarFile(new File(agentJarURL.toURI()), false); agentCodeSource = new CodeSource(agentJarURL, (Certificate[]) null); - agentResourcePrefix = "jar:file:" + agentJarFile.getName() + "!/"; + // Build the resource prefix from the agent jar URL rather than JarFile.getName(). + // JarFile.getName() returns the OS-native path, which on Windows looks like + // "C:\Datadog\dd-java-agent.jar" — producing a malformed URL ("jar:file:C:\..."), so + // findResource() returns URLs that openStream() cannot read. See APMS-19624 / #6398. + agentResourcePrefix = "jar:" + agentJarURL + "!/"; agentJarIndex = AgentJarIndex.readIndex(agentJarFile); } diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/DatadogClassLoaderTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/DatadogClassLoaderTest.groovy deleted file mode 100644 index 9b09d0f41c5..00000000000 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/DatadogClassLoaderTest.groovy +++ /dev/null @@ -1,107 +0,0 @@ -package datadog.trace.bootstrap - -import datadog.trace.test.util.DDSpecification -import spock.lang.Shared -import spock.lang.Timeout - -import java.util.concurrent.Callable -import java.util.concurrent.ExecutionException -import java.util.concurrent.ExecutorService -import java.util.concurrent.Executors -import java.util.concurrent.Future -import java.util.concurrent.Phaser -import java.util.concurrent.TimeUnit - -class DatadogClassLoaderTest extends DDSpecification { - @Shared - URL testJarLocation = new File("src/test/resources/classloader-test-jar/testjar-jdk8").toURI().toURL() - - @Shared - URL nestedTestJarLocation = new File("src/test/resources/classloader-test-jar/jar-with-nested-classes-jdk8").toURI().toURL() - - @Timeout(value = 60, unit = TimeUnit.SECONDS) - def "agent classloader does not lock classloading around instance"() { - setup: - def className1 = 'some/class/Name1' - def className2 = 'some/class/Name2' - final DatadogClassLoader ddLoader = new DatadogClassLoader() - final Phaser threadHoldLockPhase = new Phaser(2) - final Phaser acquireLockFromMainThreadPhase = new Phaser(2) - - when: - final Thread thread1 = new Thread() { - @Override - void run() { - synchronized (ddLoader.getClassLoadingLock(className1)) { - threadHoldLockPhase.arrive() - acquireLockFromMainThreadPhase.arriveAndAwaitAdvance() - } - } - } - thread1.start() - - final Thread thread2 = new Thread() { - @Override - void run() { - threadHoldLockPhase.arriveAndAwaitAdvance() - synchronized (ddLoader.getClassLoadingLock(className2)) { - acquireLockFromMainThreadPhase.arrive() - } - } - } - thread2.start() - thread1.join() - thread2.join() - boolean applicationDidNotDeadlock = true - - then: - applicationDidNotDeadlock - } - - def "agent classloader successfully loads classes concurrently"() { - given: - DatadogClassLoader ddLoader = new DatadogClassLoader(testJarLocation, null) - - when: - ExecutorService executorService = Executors.newCachedThreadPool() - List> futures = new ArrayList<>() - - for (int i = 0; i < 100; i++) { - futures.add(executorService.submit(new Callable() { - Void call() { - ddLoader.loadClass("a.A") - return null - } - })) - } - - for (Future callable : futures) { - try { - callable.get() - } catch (ExecutionException ex) { - throw ex.getCause() - } - } - - then: - noExceptionThrown() - } - - def "test load nested classes and call getEnclosingClass"() { - given: - DatadogClassLoader ddLoader = new DatadogClassLoader(nestedTestJarLocation, null) - - when: - Class klass = ddLoader.loadClass("p.EnclosingClass\$StaticInnerClass") - String simpleName = klass.getSimpleName() - - then: - simpleName == "StaticInnerClass" - - when: - Class enclosing = klass.getEnclosingClass() - - then: - enclosing.getSimpleName() == "EnclosingClass" - } -} diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DatadogClassLoaderTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DatadogClassLoaderTest.java new file mode 100644 index 00000000000..376b035a043 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DatadogClassLoaderTest.java @@ -0,0 +1,171 @@ +package datadog.trace.bootstrap; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.File; +import java.lang.reflect.Method; +import java.net.URL; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.Phaser; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +class DatadogClassLoaderTest { + private static final URL testJarLocation = + toUrl(new File("src/test/resources/classloader-test-jar/testjar-jdk8")); + private static final URL nestedTestJarLocation = + toUrl(new File("src/test/resources/classloader-test-jar/jar-with-nested-classes-jdk8")); + + private static URL toUrl(File file) { + try { + return file.toURI().toURL(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + // ClassLoader.getClassLoadingLock is protected; reach it via reflection, matching what the + // original Groovy test did implicitly. + private static Object classLoadingLock(ClassLoader cl, String name) throws Exception { + Method m = ClassLoader.class.getDeclaredMethod("getClassLoadingLock", String.class); + m.setAccessible(true); + return m.invoke(cl, name); + } + + @Test + @Timeout(60) + void agentClassloaderDoesNotLockClassloadingAroundInstance() throws Exception { + // setup + String className1 = "some/class/Name1"; + String className2 = "some/class/Name2"; + DatadogClassLoader ddLoader = new DatadogClassLoader(); + Object lock1 = classLoadingLock(ddLoader, className1); + Object lock2 = classLoadingLock(ddLoader, className2); + Phaser threadHoldLockPhase = new Phaser(2); + Phaser acquireLockFromMainThreadPhase = new Phaser(2); + + // when + Thread thread1 = + new Thread() { + @Override + public void run() { + synchronized (lock1) { + threadHoldLockPhase.arrive(); + acquireLockFromMainThreadPhase.arriveAndAwaitAdvance(); + } + } + }; + thread1.start(); + + Thread thread2 = + new Thread() { + @Override + public void run() { + threadHoldLockPhase.arriveAndAwaitAdvance(); + synchronized (lock2) { + acquireLockFromMainThreadPhase.arrive(); + } + } + }; + thread2.start(); + thread1.join(); + thread2.join(); + + // then — reaching this point means no deadlock occurred + } + + @Test + void agentClassloaderSuccessfullyLoadsClassesConcurrently() throws Exception { + // given + DatadogClassLoader ddLoader = new DatadogClassLoader(testJarLocation, null); + + // when + ExecutorService executorService = Executors.newCachedThreadPool(); + List> futures = new ArrayList<>(); + for (int i = 0; i < 100; i++) { + futures.add( + executorService.submit( + () -> { + ddLoader.loadClass("a.A"); + return null; + })); + } + for (Future future : futures) { + try { + future.get(); + } catch (Exception ex) { + if (ex.getCause() instanceof Throwable) { + throw (Exception) ex.getCause(); + } + throw ex; + } + } + + // then — no exception thrown + } + + @Test + void loadNestedClassesAndCallGetEnclosingClass() throws Exception { + // given + DatadogClassLoader ddLoader = new DatadogClassLoader(nestedTestJarLocation, null); + + // when + Class klass = ddLoader.loadClass("p.EnclosingClass$StaticInnerClass"); + + // then + assertEquals("StaticInnerClass", klass.getSimpleName()); + + // when + Class enclosing = klass.getEnclosingClass(); + + // then + assertEquals("EnclosingClass", enclosing.getSimpleName()); + } + + /** + * Regression test for APMS-19624 / DataDog/dd-trace-java#6398. The resource URL must be built + * from the agent jar URL (a properly-formed {@code file:} URL), not from {@code + * JarFile.getName()} (an OS-native path). On Windows the OS-native form is {@code + * C:\Datadog\dd-java-agent.jar}, which produces the malformed URL {@code + * jar:file:C:\Datadog\dd-java-agent.jar!/...} and breaks helper-class injection on Spring Boot + * 1.3.5 {@code LaunchedURLClassLoader}. + * + *

We exercise the divergence cross-platform by copying the test jar into a directory whose + * name contains a space: {@code URL.toString()} percent-encodes it, {@code JarFile.getName()} + * does not. + */ + @Test + void findResourceUsesAgentJarUrlAsPrefix(@org.junit.jupiter.api.io.TempDir File tempDir) + throws Exception { + File spacedDir = new File(tempDir, "dir with spaces"); + assertTrue(spacedDir.mkdirs()); + File spacedJar = new File(spacedDir, "testjar-jdk8"); + Files.copy( + new File("src/test/resources/classloader-test-jar/testjar-jdk8").toPath(), + spacedJar.toPath()); + + URL spacedJarUrl = spacedJar.toURI().toURL(); + DatadogClassLoader ddLoader = new DatadogClassLoader(spacedJarUrl, null); + + URL resource = ddLoader.findResource("a/A.class"); + assertNotNull(resource, "findResource should locate a/A.class in the test jar"); + + String expectedPrefix = "jar:" + spacedJarUrl + "!/"; + assertTrue( + resource.toString().startsWith(expectedPrefix), + () -> + "resource URL (" + + resource + + ") should start with the agent jar URL prefix (" + + expectedPrefix + + ") — pre-fix code derives the prefix from JarFile.getName()," + + " which leaves the space unencoded and on Windows produces a malformed URL."); + } +}