From 7a3abce74b0e167a555fcd27811ecd23764eb8fb 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 | 97 +++++++++++-------- 2 files changed, 56 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..774116c6349 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 && isRelatedPackage((String) pkg, className)) { + BundleWiring provider = dependency.getProviderWiring(); + if (null != provider && visited.add(provider.getRevision())) { + try { + return provider.getBundle().loadClass(className); + } catch (Exception ignore) { + // continue search... + } } } } @@ -106,4 +105,18 @@ private static T searchDirectWires( } return null; } + + private static boolean isRelatedPackage(String providedPackage, String className) { + int segmentsMatched = 0; + for (int i = 0; i < providedPackage.length(); i++) { + char c = providedPackage.charAt(i); + if (i >= className.length() || c != className.charAt(i)) { + return false; // no common package prefix + } + if (c == '.' && ++segmentsMatched >= 3) { + break; // three package segments matched, assume related + } + } + return true; + } }