diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/AnalysisTask.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/AnalysisTask.java index b855c44b5..824f7069b 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/AnalysisTask.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/AnalysisTask.java @@ -11,6 +11,9 @@ import org.slf4j.LoggerFactory; import java.util.List; import java.util.concurrent.Callable; +/** + * The task of analyzing a single {@link Dependency} by a specific {@link Analyzer}. + */ class AnalysisTask implements Callable { private static final Logger LOGGER = LoggerFactory.getLogger(AnalysisTask.class); @@ -28,7 +31,7 @@ class AnalysisTask implements Callable { } @Override - public Void call() throws Exception { + public Void call() { Settings.initialize(); if (shouldAnalyze()) { @@ -50,10 +53,10 @@ class AnalysisTask implements Callable { return null; } - private boolean shouldAnalyze() { + boolean shouldAnalyze() { if (analyzer instanceof FileTypeAnalyzer) { - final FileTypeAnalyzer fAnalyzer = (FileTypeAnalyzer) analyzer; - return fAnalyzer.accept(dependency.getActualFile()); + final FileTypeAnalyzer fileTypeAnalyzer = (FileTypeAnalyzer) analyzer; + return fileTypeAnalyzer.accept(dependency.getActualFile()); } return true; diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java index ce83539de..caf127ea1 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java @@ -47,6 +47,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -396,20 +397,20 @@ public class Engine implements FileFilter { for (AnalysisPhase phase : AnalysisPhase.values()) { final List analyzerList = analyzers.get(phase); - for (final Analyzer a : analyzerList) { + for (final Analyzer analyzer : analyzerList) { final long analyzerStart = System.currentTimeMillis(); try { - initializeAnalyzer(a); + initializeAnalyzer(analyzer); } catch (InitializationException ex) { exceptions.add(ex); continue; } - executeAnalysisTasks(exceptions, a); + executeAnalysisTasks(analyzer, exceptions); final long analyzerDurationMillis = System.currentTimeMillis() - analyzerStart; final long analyzerDurationSeconds = TimeUnit.MILLISECONDS.toSeconds(analyzerDurationMillis); - LOGGER.info("Finished {}. Took {} secs.", a.getName(), analyzerDurationSeconds); + LOGGER.info("Finished {}. Took {} secs.", analyzer.getName(), analyzerDurationSeconds); } } for (AnalysisPhase phase : AnalysisPhase.values()) { @@ -427,7 +428,7 @@ public class Engine implements FileFilter { } } - private void executeAnalysisTasks(List exceptions, Analyzer analyzer) throws ExceptionCollection { + void executeAnalysisTasks(Analyzer analyzer, List exceptions) throws ExceptionCollection { LOGGER.debug("Starting {}", analyzer.getName()); final List analysisTasks = getAnalysisTasks(analyzer, exceptions); final ExecutorService executorService = getExecutorService(analyzer); @@ -441,14 +442,18 @@ public class Engine implements FileFilter { result.get(); } catch (ExecutionException e) { throwFatalExceptionCollection("Analysis task failed with a fatal exception.", e, exceptions); + } catch (CancellationException e) { + throwFatalExceptionCollection("Analysis task timed out.", e, exceptions); } } } catch (InterruptedException e) { throwFatalExceptionCollection("Analysis has been interrupted.", e, exceptions); + } finally { + executorService.shutdown(); } } - private List getAnalysisTasks(Analyzer analyzer, List exceptions) { + List getAnalysisTasks(Analyzer analyzer, List exceptions) { final List result = new ArrayList(); synchronized (dependencies) { for (final Dependency dependency : dependencies) { @@ -459,7 +464,7 @@ public class Engine implements FileFilter { return result; } - private ExecutorService getExecutorService(Analyzer analyzer) { + ExecutorService getExecutorService(Analyzer analyzer) { if (analyzer.supportsParallelProcessing()) { // just a fair trade-off that should be reasonable for all analyzer types int maximumNumberOfThreads = 4 * Runtime.getRuntime().availableProcessors(); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java index 1c2e9f131..af2cd49c7 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java @@ -58,10 +58,11 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { * A pattern for obtaining the first part of a filename. */ private static final Pattern STARTING_TEXT_PATTERN = Pattern.compile("^[a-zA-Z0-9]*"); + /** * a flag indicating if this analyzer has run. This analyzer only runs once. */ - private boolean analyzed = false; + boolean analyzed = false; // // /** @@ -95,7 +96,7 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { // /** - * Does not support parallel processing as it both modifies and iterates over the engine's list of dependencies. + * Does not support parallel processing as it only runs once and then operates on all dependencies. * * @see #analyze(Dependency, Engine) */ diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/AnalysisTaskTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/AnalysisTaskTest.java new file mode 100644 index 000000000..03a1520be --- /dev/null +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/AnalysisTaskTest.java @@ -0,0 +1,100 @@ +package org.owasp.dependencycheck; + +import mockit.Expectations; +import mockit.Mocked; +import mockit.Verifications; +import org.junit.Test; +import org.owasp.dependencycheck.analyzer.FileTypeAnalyzer; +import org.owasp.dependencycheck.analyzer.HintAnalyzer; +import org.owasp.dependencycheck.dependency.Dependency; + +import java.io.File; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class AnalysisTaskTest { + + @Mocked + FileTypeAnalyzer fileTypeAnalyzer; + + @Mocked + Dependency dependency; + + @Mocked + Engine engine; + + + @Test + public void shouldAnalyzeReturnsTrueForNonFileTypeAnalyzers() { + AnalysisTask instance = new AnalysisTask(new HintAnalyzer(), null, null, null); + boolean shouldAnalyze = instance.shouldAnalyze(); + assertTrue(shouldAnalyze); + } + + @Test + public void shouldAnalyzeReturnsTrueIfTheFileTypeAnalyzersAcceptsTheDependency() { + final File dependencyFile = new File(""); + new Expectations() {{ + dependency.getActualFile(); + result = dependencyFile; + + fileTypeAnalyzer.accept(dependencyFile); + result = true; + }}; + + AnalysisTask analysisTask = new AnalysisTask(fileTypeAnalyzer, dependency, null, null); + + boolean shouldAnalyze = analysisTask.shouldAnalyze(); + assertTrue(shouldAnalyze); + } + + @Test + public void shouldAnalyzeReturnsFalseIfTheFileTypeAnalyzerDoesNotAcceptTheDependency() { + final File dependencyFile = new File(""); + new Expectations() {{ + dependency.getActualFile(); + result = dependencyFile; + + fileTypeAnalyzer.accept(dependencyFile); + result = false; + }}; + + AnalysisTask analysisTask = new AnalysisTask(fileTypeAnalyzer, dependency, null, null); + + boolean shouldAnalyze = analysisTask.shouldAnalyze(); + assertFalse(shouldAnalyze); + } + + @Test + public void taskAnalyzes() throws Exception { + final AnalysisTask analysisTask = new AnalysisTask(fileTypeAnalyzer, dependency, engine, null); + new Expectations(analysisTask) {{ + analysisTask.shouldAnalyze(); + result = true; + }}; + + analysisTask.call(); + + new Verifications() {{ + fileTypeAnalyzer.analyze(dependency, engine); + times = 1; + }}; + } + + @Test + public void taskDoesNothingIfItShouldNotAnalyze() throws Exception { + final AnalysisTask analysisTask = new AnalysisTask(fileTypeAnalyzer, dependency, engine, null); + new Expectations(analysisTask) {{ + analysisTask.shouldAnalyze(); + result = false; + }}; + + analysisTask.call(); + + new Verifications() {{ + fileTypeAnalyzer.analyze(dependency, engine); + times = 0; + }}; + } +} diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/EngineTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/EngineTest.java index 3f2e9cfec..87088e764 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/EngineTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/EngineTest.java @@ -17,19 +17,36 @@ */ package org.owasp.dependencycheck; -import java.io.File; +import mockit.Expectations; +import mockit.Mocked; import org.junit.Test; -import static org.junit.Assert.*; +import org.owasp.dependencycheck.analyzer.Analyzer; import org.owasp.dependencycheck.analyzer.JarAnalyzer; import org.owasp.dependencycheck.data.nvdcve.DatabaseException; import org.owasp.dependencycheck.dependency.Dependency; +import org.owasp.dependencycheck.exception.ExceptionCollection; + +import java.io.File; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** - * * @author Jeremy Long */ public class EngineTest extends BaseDBTestCase { + @Mocked + Analyzer analyzer; + + @Mocked + AnalysisTask analysisTask; + + /** * Test of scanFile method, of class Engine. */ @@ -40,14 +57,40 @@ public class EngineTest extends BaseDBTestCase { File file = BaseTest.getResourceAsFile(this, "dwr.jar"); Dependency dwr = instance.scanFile(file); file = BaseTest.getResourceAsFile(this, "org.mortbay.jmx.jar"); - Dependency jmx = instance.scanFile(file); + instance.scanFile(file); assertEquals(2, instance.getDependencies().size()); - + file = BaseTest.getResourceAsFile(this, "dwr.jar"); Dependency secondDwr = instance.scanFile(file); - + assertEquals(2, instance.getDependencies().size()); assertTrue(dwr == secondDwr); - + } + + @Test(expected = ExceptionCollection.class) + public void exceptionDuringAnalysisTaskExecutionIsFatal() throws DatabaseException, ExceptionCollection { + final ExecutorService executorService = Executors.newFixedThreadPool(3); + final Engine instance = new Engine(); + final List exceptions = new ArrayList(); + + new Expectations() {{ + analysisTask.call(); + result = new IllegalStateException("Analysis task execution threw an exception"); + }}; + + final List failingAnalysisTask = new ArrayList(); + failingAnalysisTask.add(analysisTask); + + new Expectations(instance) {{ + instance.getExecutorService(analyzer); + result = executorService; + + instance.getAnalysisTasks(analyzer, exceptions); + result = failingAnalysisTask; + }}; + + instance.executeAnalysisTasks(analyzer, exceptions); + + assertTrue(executorService.isShutdown()); } } diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CMakeAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CMakeAnalyzerTest.java index 6b3da5e65..1e6bd10cf 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CMakeAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CMakeAnalyzerTest.java @@ -17,23 +17,31 @@ */ package org.owasp.dependencycheck.analyzer; +import mockit.Mock; +import mockit.MockUp; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.owasp.dependencycheck.BaseDBTestCase; import org.owasp.dependencycheck.BaseTest; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.data.nvdcve.DatabaseException; import org.owasp.dependencycheck.dependency.Dependency; +import org.owasp.dependencycheck.exception.InitializationException; import java.io.File; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.util.List; import java.util.regex.Pattern; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.*; -import org.owasp.dependencycheck.BaseDBTestCase; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * Unit tests for CmakeAnalyzer. @@ -150,4 +158,21 @@ public class CMakeAnalyzerTest extends BaseDBTestCase { assertTrue("Expected version evidence to contain \"" + version + "\".", result.getVersionEvidence().toString().contains(version)); } + + @Test(expected = InitializationException.class) + public void analyzerIsDisabledInCaseOfMissingMessageDigest() throws InitializationException { + new MockUp() { + @Mock + MessageDigest getInstance(String ignore) throws NoSuchAlgorithmException { + throw new NoSuchAlgorithmException(); + } + }; + + analyzer = new CMakeAnalyzer(); + analyzer.setFilesMatched(true); + assertTrue(analyzer.isEnabled()); + analyzer.initialize(); + + assertFalse(analyzer.isEnabled()); + } } diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/ComposerLockAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/ComposerLockAnalyzerTest.java index 725c5ca87..fc60e58f4 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/ComposerLockAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/ComposerLockAnalyzerTest.java @@ -17,19 +17,25 @@ */ package org.owasp.dependencycheck.analyzer; +import mockit.Mock; +import mockit.MockUp; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.owasp.dependencycheck.BaseDBTestCase; import org.owasp.dependencycheck.BaseTest; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.dependency.Dependency; +import org.owasp.dependencycheck.exception.InitializationException; import java.io.File; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import org.owasp.dependencycheck.BaseDBTestCase; /** * Unit tests for NodePackageAnalyzer. @@ -96,4 +102,22 @@ public class ComposerLockAnalyzerTest extends BaseDBTestCase { "composer.lock")); analyzer.analyze(result, engine); } + + + @Test(expected = InitializationException.class) + public void analyzerIsDisabledInCaseOfMissingMessageDigest() throws InitializationException { + new MockUp() { + @Mock + MessageDigest getInstance(String ignore) throws NoSuchAlgorithmException { + throw new NoSuchAlgorithmException(); + } + }; + + analyzer = new ComposerLockAnalyzer(); + analyzer.setFilesMatched(true); + assertTrue(analyzer.isEnabled()); + analyzer.initialize(); + + assertFalse(analyzer.isEnabled()); + } } diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerTest.java index 6ae4bf167..3e5c594a8 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerTest.java @@ -17,17 +17,25 @@ */ package org.owasp.dependencycheck.analyzer; -import static org.junit.Assert.assertEquals; +import mockit.Mocked; +import mockit.Verifications; import org.junit.Test; import org.owasp.dependencycheck.BaseTest; +import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.dependency.Dependency; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + /** - * * @author Jeremy Long */ public class DependencyBundlingAnalyzerTest extends BaseTest { + @Mocked + Engine engineMock; + /** * Test of getName method, of class DependencyBundlingAnalyzer. */ @@ -52,15 +60,27 @@ public class DependencyBundlingAnalyzerTest extends BaseTest { /** * Test of analyze method, of class DependencyBundlingAnalyzer. + * The actually passed dependency does not matter. The analyzer only runs once. */ @Test public void testAnalyze() throws Exception { -// Dependency ignore = null; -// Engine engine = null; -// DependencyBundlingAnalyzer instance = new DependencyBundlingAnalyzer(); -// instance.analyze(ignore, engine); -// // TODO review the generated test code and remove the default call to fail. -// fail("The test case is a prototype."); + DependencyBundlingAnalyzer instance = new DependencyBundlingAnalyzer(); + + // the actual dependency does not matter + assertFalse(instance.analyzed); + instance.analyze(null, engineMock); + + // the second runs basically does nothing + assertTrue(instance.analyzed); + instance.analyze(null, engineMock); + instance.analyze(null, engineMock); + instance.analyze(null, engineMock); + assertTrue(instance.analyzed); + + new Verifications() {{ + engineMock.getDependencies(); + times = 2; + }}; } /** @@ -119,7 +139,5 @@ public class DependencyBundlingAnalyzerTest extends BaseTest { expResult = true; result = instance.firstPathIsShortest(left, right); assertEquals(expResult, result); - } - }