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

Fix dependency resolution for nested JARs (with fix for Spring Boot) #6720

Merged
merged 4 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 0 additions & 3 deletions telemetry/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ dependencies {
testImplementation project(':utils:test-utils')
testImplementation group: 'org.hamcrest', name: 'hamcrest', version: '2.2'
testImplementation group: 'org.jboss', name: 'jboss-vfs', version: '3.2.16.Final'
testImplementation group: 'org.springframework.boot', name: 'spring-boot-loader', version: '1.5.22.RELEASE'

jmh group: 'org.springframework.boot', name: 'spring-boot-loader', version: '1.5.22.RELEASE'
}

jmh {
Expand Down
100 changes: 43 additions & 57 deletions telemetry/src/main/java/datadog/telemetry/dependency/Dependency.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package datadog.telemetry.dependency;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.math.BigInteger;
Expand All @@ -9,14 +8,11 @@
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Properties;
import java.util.jar.Attributes;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import java.util.jar.Manifest;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -75,59 +71,50 @@ public String toString() {
+ '}';
}

public static List<Dependency> fromMavenPom(JarFile jar) {
if (jar == null) {
public static List<Dependency> fromMavenPom(
final String jar, Map<String, Properties> pomProperties) {
if (pomProperties == null) {
return Collections.emptyList();
}
List<Dependency> dependencies = new ArrayList<>(1);
Enumeration<JarEntry> entries = jar.entries();
while (entries.hasMoreElements()) {
JarEntry jarEntry = entries.nextElement();
String filename = jarEntry.getName();
if (filename.endsWith("pom.properties")) {
try (InputStream is = jar.getInputStream(jarEntry)) {
if (is == null) {
return Collections.emptyList();
}
Properties properties = new Properties();
properties.load(is);
String groupId = properties.getProperty("groupId");
String artifactId = properties.getProperty("artifactId");
String version = properties.getProperty("version");
String name = groupId + ":" + artifactId;

if (groupId == null || artifactId == null || version == null) {
log.debug(
"'pom.properties' does not have all the required properties: "
+ "jar={}, entry={}, groupId={}, artifactId={}, version={}",
jar.getName(),
jarEntry.getName(),
groupId,
artifactId,
version);
} else {
log.debug(
"dependency found in pom.properties: "
+ "jar={}, entry={}, groupId={}, artifactId={}, version={}",
jar.getName(),
jarEntry.getName(),
groupId,
artifactId,
version);
dependencies.add(
new Dependency(name, version, new File(jar.getName()).getName(), null));
}
} catch (IOException e) {
log.debug("unable to read 'pom.properties' file from {}", jar.getName(), e);
return Collections.emptyList();
}
List<Dependency> dependencies = new ArrayList<>(pomProperties.size());
for (final Map.Entry<String, Properties> entry : pomProperties.entrySet()) {
final Properties properties = entry.getValue();
final String groupId = properties.getProperty("groupId");
final String artifactId = properties.getProperty("artifactId");
final String version = properties.getProperty("version");
final String name = groupId + ":" + artifactId;

if (groupId == null || artifactId == null || version == null) {
log.debug(
"pom.properties does not have all the required properties: "
+ "jar={}, entry={}, groupId={}, artifactId={}, version={}",
jar,
entry.getKey(),
groupId,
artifactId,
version);
} else {
log.debug(
"dependency found in pom.properties: "
+ "jar={}, entry={}, groupId={}, artifactId={}, version={}",
jar,
entry.getKey(),
groupId,
artifactId,
version);
dependencies.add(new Dependency(name, version, jar, null));
}
}
return dependencies;
}

public static synchronized Dependency guessFallbackNoPom(
Manifest manifest, String source, InputStream is) throws IOException {
Attributes manifest, String source, InputStream is) throws IOException {
final int slashIndex = source.lastIndexOf('/');
if (slashIndex >= 0) {
source = source.substring(slashIndex + 1);
}

String artifactId;
String groupId = null;
String version;
Expand All @@ -140,12 +127,11 @@ public static synchronized Dependency guessFallbackNoPom(
String bundleVersion = null;
String implementationVersion = null;
if (manifest != null) {
Attributes mainAttributes = manifest.getMainAttributes();
bundleSymbolicName = mainAttributes.getValue("bundle-symbolicname");
bundleName = mainAttributes.getValue("bundle-name");
bundleVersion = mainAttributes.getValue("bundle-version");
implementationTitle = mainAttributes.getValue("implementation-title");
implementationVersion = mainAttributes.getValue("implementation-version");
bundleSymbolicName = manifest.getValue("bundle-symbolicname");
bundleName = manifest.getValue("bundle-name");
bundleVersion = manifest.getValue("bundle-version");
implementationTitle = manifest.getValue("implementation-title");
implementationVersion = manifest.getValue("implementation-version");
}

// Guess from file name
Expand Down Expand Up @@ -204,7 +190,7 @@ public static synchronized Dependency guessFallbackNoPom(
}

if (md != null) {
// Compute hash for all dependencies that has no pom
// Compute hash for all dependencies that have no pom
// No reliable version calculate hash and use any version
md.reset();
is = new DigestInputStream(is, md);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
package datadog.telemetry.dependency;

import java.io.File;
import java.io.IOException;
import java.io.FileInputStream;
import java.io.InputStream;
import java.net.JarURLConnection;
import java.net.URI;
import java.net.URL;
import java.nio.file.Files;
import java.util.Collections;
import java.util.List;
import java.util.jar.JarFile;
import java.util.jar.Manifest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -20,110 +15,38 @@ public class DependencyResolver {
private static final String JAR_SUFFIX = ".jar";

public static List<Dependency> resolve(URI uri) {
return extractDependenciesFromURI(uri);
}

/**
* Identify library from a URI
*
* @param uri URI to a dependency
* @return dependency, or null if unable to qualify jar
*/
// package private for testing
static List<Dependency> extractDependenciesFromURI(URI uri) {
String scheme = uri.getScheme();
List<Dependency> dependencies = Collections.emptyList();
final String scheme = uri.getScheme();
try {
JarReader.Extracted metadata = null;
String path = null;
if ("file".equals(scheme)) {
File f;
if (uri.isOpaque()) {
f = new File(uri.getSchemeSpecificPart());

Choose a reason for hiding this comment

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

What is the reason of doing it like this instead of:

File f = Paths.get(uri).toFile();

I think this part is not covered by any test 😢

Copy link
Member Author

Choose a reason for hiding this comment

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

The background story here is tricky, so I'm not touching this logic until I can test it properly. We'll probably require Windows tests for it. See #4820

} else {
f = new File(uri);
}
dependencies = extractDependenciesFromJar(f);
} else if ("jar".equals(scheme)) {
Dependency dependency = getNestedDependency(uri);
if (dependency != null) {
dependencies = Collections.singletonList(dependency);
}
path = f.getAbsolutePath();
metadata = JarReader.readJarFile(path);
} else if ("jar".equals(scheme) && uri.getSchemeSpecificPart().startsWith("file:")) {
path = uri.getSchemeSpecificPart().substring("file:".length());
metadata = JarReader.readNestedJarFile(path);
} else {
log.debug("unsupported dependency type: {}", uri);
return Collections.emptyList();
}
} catch (RuntimeException rte) {
log.debug("Failed to determine dependency for uri {}", uri, rte);
}
// TODO : moving jboss vfs here is probably a idea
// it might however require to do somme checks to make sure it's only applied to jboss
// and not any application server that also uses vfs:// locations

return dependencies;
}

/**
* Identify a library from a .jar file
*
* @param jar jar dependency
* @return detected dependency, {@code null} if unable to get dependency from jar
*/
static List<Dependency> extractDependenciesFromJar(File jar) {
if (!jar.exists()) {
log.debug("unable to find dependency {} (path does not exist)", jar);
return Collections.emptyList();
} else if (!jar.getName().endsWith(JAR_SUFFIX)) {
log.debug("unsupported file dependency type : {}", jar);
return Collections.emptyList();
}

List<Dependency> dependencies = Collections.emptyList();
try (JarFile file = new JarFile(jar, false /* no verify */)) {

// Try to get from maven properties
dependencies = Dependency.fromMavenPom(file);

// Try to guess from manifest or file name
if (dependencies.isEmpty()) {
try (InputStream is = Files.newInputStream(jar.toPath())) {
Manifest manifest = file.getManifest();
dependencies =
Collections.singletonList(Dependency.guessFallbackNoPom(manifest, jar.getName(), is));
}
final List<Dependency> dependencies =
Dependency.fromMavenPom(metadata.jarName, metadata.pomProperties);
if (!dependencies.isEmpty()) {
return dependencies;
}
} catch (IOException e) {
log.debug("unable to read jar file {}", jar, e);
}

return dependencies;
}

/* for jar urls as handled by spring boot */
static Dependency getNestedDependency(URI uri) {
String lastPart = null;
String fileName = null;
try {
URL url = uri.toURL();
JarURLConnection jarConnection = (JarURLConnection) url.openConnection();
Manifest manifest = jarConnection.getManifest();

JarFile jarFile = jarConnection.getJarFile();

// the !/ separator is hardcoded into JarURLConnection class
String jarFileName = jarFile.getName();
int posSep = jarFileName.indexOf("!/");
if (posSep == -1) {
log.debug("Unable to guess nested dependency for uri '{}': '!/' not found", uri);
return null;
try (final InputStream is = new FileInputStream(path)) {
return Collections.singletonList(
Dependency.guessFallbackNoPom(metadata.manifest, metadata.jarName, is));
}
lastPart = jarFileName.substring(posSep + 1);
fileName = lastPart.substring(lastPart.lastIndexOf("/") + 1);

return Dependency.guessFallbackNoPom(manifest, fileName, jarConnection.getInputStream());
} catch (Exception e) {
log.debug("unable to open nested jar manifest for {}", uri, e);
} catch (Throwable t) {
log.debug("Failed to determine dependency for uri {}", uri, t);
}
log.debug(
"Unable to guess nested dependency for uri '{}', lastPart: '{}', fileName: '{}'",
uri,
lastPart,
fileName);
return null;
return Collections.emptyList();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package datadog.telemetry.dependency;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.NoSuchFileException;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
import java.util.jar.Attributes;
import java.util.jar.JarFile;
import java.util.jar.JarInputStream;
import java.util.jar.Manifest;
import java.util.zip.ZipEntry;

class JarReader {
static class Extracted {
final String jarName;
final Map<String, Properties> pomProperties;
final Attributes manifest;

public Extracted(
final String jarName,
final Map<String, Properties> pomProperties,
final Attributes manifest) {
this.jarName = jarName;
this.pomProperties = pomProperties;
this.manifest = manifest;
}
}

public static Extracted readJarFile(String jarPath) throws IOException {
try (final JarFile jar = new JarFile(jarPath, false /* no verify */)) {
final Map<String, Properties> pomProperties = new HashMap<>();
final Enumeration<? extends ZipEntry> entries = jar.entries();
while (entries.hasMoreElements()) {
final ZipEntry entry = entries.nextElement();
if (entry.getName().endsWith("pom.properties")) {
try (final InputStream is = jar.getInputStream(entry)) {
final Properties properties = new Properties();
properties.load(is);
pomProperties.put(entry.getName(), properties);
}
}
}
final Manifest manifest = jar.getManifest();
final Attributes attributes =
(manifest == null) ? new Attributes() : manifest.getMainAttributes();
return new Extracted(new File(jar.getName()).getName(), pomProperties, attributes);
}
}

public static Extracted readNestedJarFile(final String jarPath) throws IOException {
final int sepIdx = jarPath.indexOf("!/");
if (sepIdx == -1) {
throw new IllegalArgumentException("Invalid nested jar path: " + jarPath);
}
final String outerJarPath = jarPath.substring(0, sepIdx);
String innerJarPath = jarPath.substring(sepIdx + 2);
if (innerJarPath.endsWith("!/")) {
innerJarPath = innerJarPath.substring(0, innerJarPath.length() - 2);
}
try (final JarFile outerJar = new JarFile(outerJarPath, false /* no verify */)) {
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, but is it a thing to have more levels of nested jars?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have seen at least 1 user report at Spring Boot asking about a third level of nesting, but Spring Boot does not support that. It would definitely be possible to do it with a custom URLConnection handler. But until we find some real scenario, I would rather not implement it.

Also note that, while it should be relatively easy to add support for an arbitrary nesting depth, we would always set a limit to nesting for performance reasons since each additional level of nesting needs to decompress a stream.

final ZipEntry entry = outerJar.getEntry(innerJarPath);
if (entry == null) {
throw new NoSuchFileException("Nested jar not found: " + jarPath);
}
try (final InputStream is = outerJar.getInputStream(entry);
final JarInputStream innerJar = new JarInputStream(is, false /* no verify */)) {
final Map<String, Properties> pomProperties = new HashMap<>();
ZipEntry innerEntry;
while ((innerEntry = innerJar.getNextEntry()) != null) {
if (innerEntry.getName().endsWith("pom.properties")) {
final Properties properties = new Properties();
properties.load(innerJar);
pomProperties.put(innerEntry.getName(), properties);
}
}
final Manifest manifest = innerJar.getManifest();
final Attributes attributes =
(manifest == null) ? new Attributes() : manifest.getMainAttributes();
return new Extracted(new File(innerJarPath).getName(), pomProperties, attributes);
}
}
}
}
Loading
Loading