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

efficiency tweaks WIP #1618

Closed
wants to merge 1 commit into from
Closed
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
4 changes: 4 additions & 0 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ Copyright (c) 2012 Jeremy Long. All Rights Reserved.
</plugins>
</build>
<dependencies>
<dependency>
Copy link
Owner

Choose a reason for hiding this comment

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

In a quick review of the code I don't see jackson-databind being used anywhere?

<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
</dependency>
<dependency>
<groupId>com.vdurmont</groupId>
<artifactId>semver4j</artifactId>
Expand Down
109 changes: 38 additions & 71 deletions core/src/main/java/org/owasp/dependencycheck/Engine.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,24 @@
import org.owasp.dependencycheck.data.update.exception.UpdateException;
import org.owasp.dependencycheck.dependency.Dependency;
import org.owasp.dependencycheck.exception.ExceptionCollection;
import org.owasp.dependencycheck.exception.H2DBLockException;
import org.owasp.dependencycheck.exception.InitializationException;
import org.owasp.dependencycheck.exception.NoDataException;
import org.owasp.dependencycheck.exception.ReportException;
import org.owasp.dependencycheck.reporting.ReportGenerator;
import org.owasp.dependencycheck.utils.H2DBLock;
import org.owasp.dependencycheck.utils.InvalidSettingException;
import org.owasp.dependencycheck.utils.Settings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.concurrent.NotThreadSafe;
import java.io.File;
import java.io.FileFilter;
import java.io.IOException;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumMap;
Expand All @@ -52,18 +56,19 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import javax.annotation.concurrent.NotThreadSafe;
import org.owasp.dependencycheck.exception.H2DBLockException;
import org.owasp.dependencycheck.utils.H2DBLock;
import java.util.stream.Collectors;

//CSOFF: AvoidStarImport
import static org.owasp.dependencycheck.analyzer.AnalysisPhase.*;

//CSOFF: AvoidStarImport
//CSON: AvoidStarImport

/**
Expand Down Expand Up @@ -96,7 +101,7 @@ public enum Mode {
* In evidence processing mode the {@link Engine} processes the evidence
* collected using the {@link #EVIDENCE_COLLECTION} mode. Dependencies
* should be injected into the {@link Engine} using
* {@link Engine#setDependencies(List)}.
* {@link Engine#setDependencies(Collection)}.
*/
EVIDENCE_PROCESSING(
true,
Expand Down Expand Up @@ -156,11 +161,13 @@ public AnalysisPhase[] getPhases() {
/**
* The list of dependencies.
*/
private final List<Dependency> dependencies = Collections.synchronizedList(new ArrayList<Dependency>());
private final TreeSet<Dependency> dependencies = new TreeSet<>();

/**
* The external view of the dependency list.
* Dependency map for ease of computation.
*/
private Dependency[] dependenciesExternalView = null;
private final Map<String, Dependency> dependencyMap = new ConcurrentHashMap<>();

/**
* A Map of analyzers grouped by Analysis phase.
*/
Expand Down Expand Up @@ -300,8 +307,7 @@ public List<Analyzer> getAnalyzers(AnalysisPhase phase) {
* @param dependency the dependency to add
*/
public synchronized void addDependency(Dependency dependency) {
dependencies.add(dependency);
dependenciesExternalView = null;
dependencyMap.put(dependency.getSha1sum(), dependency);
}

/**
Expand All @@ -319,31 +325,26 @@ public synchronized void sortDependencies() {
* @param dependency the dependency to remove.
*/
public synchronized void removeDependency(Dependency dependency) {
dependencies.remove(dependency);
dependenciesExternalView = null;
dependencyMap.remove(dependency.getSha1sum());
}

/**
* Returns a copy of the dependencies as an array.
*
* @return the dependencies identified
*/
public synchronized Dependency[] getDependencies() {
if (dependenciesExternalView == null) {
dependenciesExternalView = dependencies.toArray(new Dependency[dependencies.size()]);
}
return dependenciesExternalView;
public synchronized List<Dependency> getDependencies() {
return new ArrayList<>(dependencyMap.values());
}

/**
* Sets the dependencies.
*
* @param dependencies the dependencies
*/
public synchronized void setDependencies(List<Dependency> dependencies) {
this.dependencies.clear();
this.dependencies.addAll(dependencies);
dependenciesExternalView = null;
public synchronized void setDependencies(Collection<Dependency> dependencies) {
this.dependencyMap.clear();
dependencies.forEach((dep) -> this.dependencyMap.put(dep.getSha1sum(), dep));
}

/**
Expand Down Expand Up @@ -371,14 +372,7 @@ public List<Dependency> scan(String[] paths) {
* @since v1.4.4
*/
public List<Dependency> scan(String[] paths, String projectReference) {
final List<Dependency> deps = new ArrayList<>();
for (String path : paths) {
final List<Dependency> d = scan(path, projectReference);
if (d != null) {
deps.addAll(d);
}
}
return deps;
return scan(Arrays.stream(paths).map(File::new).collect(Collectors.toList()), projectReference);
}

/**
Expand Down Expand Up @@ -434,14 +428,7 @@ public List<Dependency> scan(File[] files) {
* @since v1.4.4
*/
public List<Dependency> scan(File[] files, String projectReference) {
final List<Dependency> deps = new ArrayList<>();
for (File file : files) {
final List<Dependency> d = scan(file, projectReference);
if (d != null) {
deps.addAll(d);
}
}
return deps;
return scan(Arrays.asList(files), projectReference);
}

/**
Expand Down Expand Up @@ -469,14 +456,7 @@ public List<Dependency> scan(Collection<File> files) {
* @since v1.4.4
*/
public List<Dependency> scan(Collection<File> files, String projectReference) {
final List<Dependency> deps = new ArrayList<>();
for (File file : files) {
final List<Dependency> d = scan(file, projectReference);
if (d != null) {
deps.addAll(d);
}
}
return deps;
return files.stream().map(file -> scan(file, projectReference).stream()).flatMap(dep -> dep).collect(Collectors.toList());
}

/**
Expand Down Expand Up @@ -591,28 +571,20 @@ protected synchronized Dependency scanFile(File file, String projectReference) {
dependency.addProjectReference(projectReference);
}
final String sha1 = dependency.getSha1sum();
boolean found = false;

if (sha1 != null) {
for (Dependency existing : dependencies) {
if (sha1.equals(existing.getSha1sum())) {
found = true;
if (projectReference != null) {
existing.addProjectReference(projectReference);
}
if (existing.getActualFilePath() != null && dependency.getActualFilePath() != null
&& !existing.getActualFilePath().equals(dependency.getActualFilePath())) {
existing.addRelatedDependency(dependency);
} else {
dependency = existing;
}
break;
final Dependency hashDependency = dependency;
dependencyMap.computeIfPresent(sha1, (sha, existing) -> {
if (projectReference != null) {
existing.addProjectReference(projectReference);
}
}
}
if (!found) {
dependencies.add(dependency);
dependenciesExternalView = null;
if (existing.getActualFilePath() != null && hashDependency.getActualFilePath() != null
&& !existing.getActualFilePath().equals(hashDependency.getActualFilePath())) {
existing.addRelatedDependency(hashDependency);
}
return existing;
});
dependency = dependencyMap.computeIfAbsent(sha1, (sha) -> hashDependency);
}
}
} else {
Expand Down Expand Up @@ -787,12 +759,7 @@ protected void executeAnalysisTasks(Analyzer analyzer, List<Throwable> exception
* @return a collection of analysis tasks
*/
protected synchronized List<AnalysisTask> getAnalysisTasks(Analyzer analyzer, List<Throwable> exceptions) {
final List<AnalysisTask> result = new ArrayList<>();
for (final Dependency dependency : dependencies) {
final AnalysisTask task = new AnalysisTask(analyzer, dependency, this, exceptions);
result.add(task);
}
return result;
return getDependencies().stream().map(dependency -> new AnalysisTask(analyzer, dependency, this, exceptions)).collect(Collectors.toList());
}

/**
Expand Down Expand Up @@ -1113,7 +1080,7 @@ public synchronized void writeReports(String applicationName, String groupId, St
throw new UnsupportedOperationException("Cannot generate report in evidence collection mode.");
}
final DatabaseProperties prop = database.getDatabaseProperties();
final ReportGenerator r = new ReportGenerator(applicationName, groupId, artifactId, version, dependencies, getAnalyzers(), prop, settings);
final ReportGenerator r = new ReportGenerator(applicationName, groupId, artifactId, version, getDependencies(), getAnalyzers(), prop, settings);
try {
r.write(outputDir.getAbsolutePath(), format);
} catch (ReportException ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@
*/
package org.owasp.dependencycheck.agent;

import java.io.File;
import java.io.IOException;
import java.util.List;
import javax.annotation.concurrent.NotThreadSafe;
import org.owasp.dependencycheck.Engine;
import org.owasp.dependencycheck.data.nvdcve.DatabaseException;
import org.owasp.dependencycheck.data.update.exception.UpdateException;
Expand All @@ -35,6 +31,12 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.concurrent.NotThreadSafe;
import java.io.File;
import java.io.IOException;
import java.util.Collection;
import java.util.List;

/**
* This class provides a way to easily conduct a scan solely based on existing
* evidence metadata rather than collecting evidence from the files themselves.
Expand Down Expand Up @@ -1032,7 +1034,7 @@ public Engine execute() throws ScanAgentException {
* @throws org.owasp.dependencycheck.exception.ScanAgentException thrown if
* there is an exception executing the scan.
*/
private void checkForFailure(Dependency[] dependencies) throws ScanAgentException {
private void checkForFailure(Collection<Dependency> dependencies) throws ScanAgentException {
final StringBuilder ids = new StringBuilder();
for (Dependency d : dependencies) {
boolean addName = true;
Expand Down Expand Up @@ -1069,7 +1071,7 @@ private void checkForFailure(Dependency[] dependencies) throws ScanAgentExceptio
*
* @param dependencies a list of dependency objects
*/
private void showSummary(Dependency[] dependencies) {
private void showSummary(Collection<Dependency> dependencies) {
final StringBuilder summary = new StringBuilder();
for (Dependency d : dependencies) {
boolean firstEntry = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@
import org.owasp.dependencycheck.exception.InitializationException;
import org.owasp.dependencycheck.utils.InvalidSettingException;
import org.owasp.dependencycheck.utils.Settings;
import javax.annotation.concurrent.ThreadSafe;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.concurrent.ThreadSafe;

/**
* Base class for analyzers to avoid code duplication of prepare and close as
* most analyzers do not need these methods.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,17 @@
*/
package org.owasp.dependencycheck.analyzer;

import java.util.HashSet;
import java.util.Set;
import javax.annotation.concurrent.ThreadSafe;
import org.owasp.dependencycheck.Engine;
import org.owasp.dependencycheck.analyzer.exception.AnalysisException;
import org.owasp.dependencycheck.dependency.Dependency;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.concurrent.ThreadSafe;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

/**
* <p>
* This analyzer ensures dependencies that should be grouped together, to remove
Expand Down Expand Up @@ -91,15 +93,15 @@ protected synchronized void analyzeDependency(Dependency ignore, Engine engine)
analyzed = true;
final Set<Dependency> dependenciesToRemove = new HashSet<>();

final Dependency[] dependencies = engine.getDependencies();
if (dependencies.length < 2) {
final List<Dependency> dependencies = engine.getDependencies();
if (dependencies.size() < 2) {
return;
}
for (int x = 0; x < dependencies.length - 1; x++) {
final Dependency dependency = dependencies[x];
for (int x = 0; x < dependencies.size() - 1; x++) {
final Dependency dependency = dependencies.get(x);
if (!dependenciesToRemove.contains(dependency)) {
for (int y = x + 1; y < dependencies.length; y++) {
final Dependency nextDependency = dependencies[y];
for (int y = x + 1; y < dependencies.size(); y++) {
final Dependency nextDependency = dependencies.get(y);
if (evaluateDependencies(dependency, nextDependency, dependenciesToRemove)) {
break;
}
Expand Down
Loading