From 41217e9dfe175573c43e18e8828cd66ffe6111f7 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Mon, 2 Oct 2023 23:27:25 +0100 Subject: [PATCH] Limit OSGi visibility fix to those direct dependencies that export a package with a common prefix to the class being loaded. --- .../BundleReferenceInstrumentation.java | 6 +- .../osgi43/BundleWiringHelper.java | 83 +++++++++---------- 2 files changed, 42 insertions(+), 47 deletions(-) diff --git a/dd-java-agent/instrumentation/osgi-4.3/src/main/java/datadog/trace/instrumentation/osgi43/BundleReferenceInstrumentation.java b/dd-java-agent/instrumentation/osgi-4.3/src/main/java/datadog/trace/instrumentation/osgi43/BundleReferenceInstrumentation.java index 1d3c3668cee..0e6fea7abc7 100644 --- a/dd-java-agent/instrumentation/osgi-4.3/src/main/java/datadog/trace/instrumentation/osgi43/BundleReferenceInstrumentation.java +++ b/dd-java-agent/instrumentation/osgi-4.3/src/main/java/datadog/trace/instrumentation/osgi43/BundleReferenceInstrumentation.java @@ -46,11 +46,7 @@ public ElementMatcher hierarchyMatcher() { @Override public String[] helperClassNames() { - return new String[] { - packageName + ".BundleWiringHelper", - packageName + ".BundleWiringHelper$1", - packageName + ".BundleWiringHelper$2" - }; + return new String[] {packageName + ".BundleWiringHelper"}; } @Override diff --git a/dd-java-agent/instrumentation/osgi-4.3/src/main/java/datadog/trace/instrumentation/osgi43/BundleWiringHelper.java b/dd-java-agent/instrumentation/osgi-4.3/src/main/java/datadog/trace/instrumentation/osgi43/BundleWiringHelper.java index 46e5170a90c..f1dab161daa 100644 --- a/dd-java-agent/instrumentation/osgi-4.3/src/main/java/datadog/trace/instrumentation/osgi43/BundleWiringHelper.java +++ b/dd-java-agent/instrumentation/osgi-4.3/src/main/java/datadog/trace/instrumentation/osgi43/BundleWiringHelper.java @@ -6,7 +6,6 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.function.Function; import org.osgi.framework.Bundle; import org.osgi.framework.wiring.BundleRevision; import org.osgi.framework.wiring.BundleWire; @@ -51,54 +50,54 @@ public static Object probeResource(final Bundle origin, final String resourceNam return SKIP_REQUEST; } - /** Delegates the resource request to any bundles wired as dependencies. */ + /** Delegates resource request to any direct dependencies (Import-Package, Require-Bundle etc.) */ public static URL getResource(final Bundle origin, final String resourceName) { - return searchDirectWires( - (BundleWiring) origin.adapt(BundleWiring.class), - // Uses inner class for predictable name for Instrumenter.Default.helperClassNames() - new Function() { - @Override - public URL apply(final BundleWiring wiring) { - return wiring.getBundle().getResource(resourceName); - } - }, - new HashSet()); - } - - /** Delegates the class-load request to any bundles wired as dependencies. */ - public static Class loadClass(final Bundle origin, final String className) { - return searchDirectWires( - (BundleWiring) origin.adapt(BundleWiring.class), - // Uses inner class for predictable name for Instrumenter.Default.helperClassNames() - new Function>() { - @Override - public Class apply(final BundleWiring wiring) { + BundleWiring wiring = (BundleWiring) origin.adapt(BundleWiring.class); + if (null != wiring) { + // track which bundles we've visited to avoid dependency cycles + Set visited = new HashSet<>(); + visited.add(wiring.getRevision()); + // check all Import-Package and Require-Bundle dependencies for resources + List dependencies = wiring.getRequiredWires(null); + if (null != dependencies) { + for (BundleWire dependency : dependencies) { + BundleWiring provider = dependency.getProviderWiring(); + if (null != provider && visited.add(provider.getRevision())) { try { - return wiring.getBundle().loadClass(className); - } catch (ClassNotFoundException e) { - return null; + URL result = provider.getBundle().getResource(resourceName); + if (null != result) { + return result; + } + } catch (Exception ignore) { + // continue search... } } - }, - new HashSet()); + } + } + } + return null; } - /** Searches a bundle's direct dependencies (Import-Package, Require-Bundle etc.) */ - private static T searchDirectWires( - final BundleWiring origin, - final Function filter, - final Set visited) { - if (null != origin) { + /** Delegates class-load request to those direct dependencies that provide a similar package. */ + public static Class loadClass(final Bundle origin, final String className) { + BundleWiring wiring = (BundleWiring) origin.adapt(BundleWiring.class); + if (null != wiring) { // track which bundles we've visited to avoid dependency cycles - visited.add(origin.getRevision()); - List wires = origin.getRequiredWires(null); - if (null != wires) { - for (BundleWire wire : wires) { - BundleWiring wiring = wire.getProviderWiring(); - if (null != wiring && visited.add(wiring.getRevision())) { - T result = filter.apply(wiring); - if (null != result) { - return result; + Set visited = new HashSet<>(); + visited.add(wiring.getRevision()); + // check Import-Package dependencies that share a package prefix with the class + List dependencies = wiring.getRequiredWires(PACKAGE_NAMESPACE); + if (null != dependencies) { + for (BundleWire dependency : dependencies) { + Object pkg = dependency.getCapability().getAttributes().get(PACKAGE_NAMESPACE); + if (pkg instanceof String && className.startsWith((String) pkg)) { + BundleWiring provider = dependency.getProviderWiring(); + if (null != provider && visited.add(provider.getRevision())) { + try { + return provider.getBundle().loadClass(className); + } catch (Exception ignore) { + // continue search... + } } } }