diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java index d542171e6..c1e15538c 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java @@ -217,20 +217,6 @@ public class ArchiveAnalyzer extends AbstractFileTypeAnalyzer { } } - /** - * Does not support parallel processing as it both modifies and iterates - * over the engine's list of dependencies. - * - * @return true if the analyzer supports parallel processing; - * otherwise false - * @see #analyzeDependency(Dependency, Engine) - * @see #findMoreDependencies(Engine, File) - */ - @Override - public boolean supportsParallelProcessing() { - return true; - } - /** * Analyzes a given dependency. If the dependency is an archive, such as a * WAR or EAR, the contents are extracted, scanned, and added to the list of diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CentralAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CentralAnalyzer.java index 55d267832..1ab49f346 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CentralAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CentralAnalyzer.java @@ -75,19 +75,16 @@ public class CentralAnalyzer extends AbstractFileTypeAnalyzer { private static final String SUPPORTED_EXTENSIONS = "jar"; /** - * The analyzer should be disabled if there are errors, so this is a flag to - * determine if such an error has occurred. + * There may be temporary issues when connecting to MavenCentral. + * In order to compensate for 99% of the issues, we perform a retry + * before finally failing the analysis. */ - private volatile boolean errorFlag = false; + private static final int NUMBER_OF_TRIES = 5; /** * The searcher itself. */ - private CentralSearch searcher; - /** - * Field indicating if the analyzer is enabled. - */ - private boolean enabled = true; + protected CentralSearch searcher; /** * Initializes the analyzer with the configured settings. @@ -97,17 +94,7 @@ public class CentralAnalyzer extends AbstractFileTypeAnalyzer { @Override public void initialize(Settings settings) { super.initialize(settings); - enabled = checkEnabled(); - } - - /** - * Determine whether to enable this analyzer or not. - * - * @return whether the analyzer should be enabled - */ - @Override - public boolean isEnabled() { - return enabled; + setEnabled(checkEnabled()); } /** @@ -202,17 +189,13 @@ public class CentralAnalyzer extends AbstractFileTypeAnalyzer { * Performs the analysis. * * @param dependency the dependency to analyze - * @param engine the engine + * @param engine the engine * @throws AnalysisException when there's an exception during analysis */ @Override public void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { - if (errorFlag || !isEnabled()) { - return; - } - try { - final List mas = searcher.searchSha1(dependency.getSha1sum()); + final List mas = fetchMavenArtifacts(dependency); final Confidence confidence = mas.size() > 1 ? Confidence.HIGH : Confidence.HIGHEST; for (MavenArtifact ma : mas) { LOGGER.debug("Central analyzer found artifact ({}) for dependency ({})", ma, dependency.getFileName()); @@ -256,8 +239,52 @@ public class CentralAnalyzer extends AbstractFileTypeAnalyzer { } catch (FileNotFoundException fnfe) { LOGGER.debug("Artifact not found in repository: '{}", dependency.getFileName()); } catch (IOException ioe) { - LOGGER.debug("Could not connect to Central search", ioe); - errorFlag = true; + final String message = "Could not connect to Central search. Analysis failed."; + LOGGER.error(message, ioe); + throw new AnalysisException(message, ioe); } } + + /** + * Downloads the corresponding list of MavenArtifacts of the given + * dependency from MavenCentral. + *

+ * As the connection to MavenCentral is known to be unreliable, we implement + * a simple retry logic in order to compensate for 99% of the issues. + * + * @param dependency the dependency to analyze + * @return the downloaded list of MavenArtifacts + * @throws FileNotFoundException if the specified artifact is not found + * @throws IOException if connecting to MavenCentral finally failed + */ + protected List fetchMavenArtifacts(Dependency dependency) throws IOException { + IOException lastException = null; + long sleepingTimeBetweenRetriesInMillis = 1000; + int triesLeft = NUMBER_OF_TRIES; + while (triesLeft-- > 0) { + try { + return searcher.searchSha1(dependency.getSha1sum()); + } catch (FileNotFoundException fnfe) { + // retry does not make sense, just throw the exception + throw fnfe; + } catch (IOException ioe) { + LOGGER.debug("Could not connect to Central search (tries left: {}): {}", + triesLeft, ioe.getMessage()); + lastException = ioe; + + if (triesLeft > 0) { + try { + Thread.sleep(sleepingTimeBetweenRetriesInMillis); + sleepingTimeBetweenRetriesInMillis *= 2; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } + } + + final String message = "Finally failed connecting to Central search." + + " Giving up after " + NUMBER_OF_TRIES + " tries."; + throw new IOException(message, lastException); + } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java index 8466741ef..0db54763b 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java @@ -74,7 +74,7 @@ public class CentralSearch { * * @param settings the configured settings * @throws MalformedURLException thrown if the configured URL is - * invalid + * invalid */ public CentralSearch(Settings settings) throws MalformedURLException { this.settings = settings; @@ -101,8 +101,9 @@ public class CentralSearch { * * @param sha1 the SHA-1 hash string for which to search * @return the populated Maven GAV. - * @throws IOException if it's unable to connect to the specified repository - * or if the specified artifact is not found. + * @throws FileNotFoundException if the specified artifact is not found + * @throws IOException if it's unable to connect to the specified + * repository */ public List searchSha1(String sha1) throws IOException { if (null == sha1 || !sha1.matches("^[0-9A-Fa-f]{40}$")) { @@ -178,9 +179,8 @@ public class CentralSearch { throw new FileNotFoundException("Artifact not found in Central"); } } else { - LOGGER.debug("Could not connect to Central received response code: {} {}", - conn.getResponseCode(), conn.getResponseMessage()); - throw new IOException("Could not connect to Central"); + String errorMessage = "Could not connect to MavenCentral (" + conn.getResponseCode() + "): " + conn.getResponseMessage(); + throw new IOException(errorMessage); } return result; } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java index e7f305635..ee5bb1037 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java @@ -164,7 +164,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp * Constructs a new Dependency object. */ public Dependency() { - //empty contructor + //empty constructor } /** diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java index eacb5d40c..93862d579 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java @@ -25,8 +25,6 @@ import javax.annotation.concurrent.ThreadSafe; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.owasp.dependencycheck.utils.Filter; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Used to maintain a collection of Evidence. @@ -40,10 +38,6 @@ class EvidenceCollection implements Serializable { * The serial version UID for serialization. */ private static final long serialVersionUID = 1L; - /** - * The logger. - */ - private static final Logger LOGGER = LoggerFactory.getLogger(EvidenceCollection.class); /** * A collection of vendor evidence. */ diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CentralAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CentralAnalyzerTest.java new file mode 100644 index 000000000..3a52a769b --- /dev/null +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CentralAnalyzerTest.java @@ -0,0 +1,172 @@ +/* + * This file is part of dependency-check-core. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Copyright (c) 2017 Jeremy Long. All Rights Reserved. + */ +package org.owasp.dependencycheck.analyzer; + +import mockit.Expectations; +import mockit.Mock; +import mockit.MockUp; +import mockit.Mocked; +import org.junit.BeforeClass; +import org.junit.Test; +import org.owasp.dependencycheck.analyzer.exception.AnalysisException; +import org.owasp.dependencycheck.data.central.CentralSearch; +import org.owasp.dependencycheck.data.nexus.MavenArtifact; +import org.owasp.dependencycheck.dependency.Dependency; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.assertEquals; + +/** + * Tests for the CentralAnalyzer. + */ +public class CentralAnalyzerTest { + + private static final String SHA1_SUM = "my-sha1-sum"; + + @BeforeClass + public static void beforeClass() { + doNotSleepBetweenRetries(); + } + + @Test + @SuppressWarnings("PMD.NonStaticInitializer") + public void testFetchMavenArtifactsWithoutException(@Mocked final CentralSearch centralSearch, + @Mocked final Dependency dependency) + throws IOException { + + CentralAnalyzer instance = new CentralAnalyzer(); + instance.searcher = centralSearch; + specifySha1SumFor(dependency); + + final List expectedMavenArtifacts = Collections.emptyList(); + new Expectations() {{ + centralSearch.searchSha1(SHA1_SUM); + returns(expectedMavenArtifacts); + }}; + + final List actualMavenArtifacts = instance.fetchMavenArtifacts(dependency); + + assertEquals(expectedMavenArtifacts, actualMavenArtifacts); + } + + @Test + @SuppressWarnings("PMD.NonStaticInitializer") + public void testFetchMavenArtifactsWithSporadicIOException(@Mocked final CentralSearch centralSearch, + @Mocked final Dependency dependency) + throws IOException { + + CentralAnalyzer instance = new CentralAnalyzer(); + instance.searcher = centralSearch; + specifySha1SumFor(dependency); + + final List expectedMavenArtifacts = Collections.emptyList(); + new Expectations() {{ + centralSearch.searchSha1(SHA1_SUM); + result = new IOException("Could not connect to MavenCentral (500): Internal Server Error"); + result = new IOException("Could not connect to MavenCentral (500): Internal Server Error"); + result = expectedMavenArtifacts; + }}; + + final List actualMavenArtifacts = instance.fetchMavenArtifacts(dependency); + + assertEquals(expectedMavenArtifacts, actualMavenArtifacts); + } + + @Test(expected = FileNotFoundException.class) + @SuppressWarnings("PMD.NonStaticInitializer") + public void testFetchMavenArtifactsRethrowsFileNotFoundException(@Mocked final CentralSearch centralSearch, + @Mocked final Dependency dependency) + throws IOException { + + CentralAnalyzer instance = new CentralAnalyzer(); + instance.searcher = centralSearch; + specifySha1SumFor(dependency); + + new Expectations() {{ + centralSearch.searchSha1(SHA1_SUM); + result = new FileNotFoundException("Artifact not found in Central"); + }}; + + instance.fetchMavenArtifacts(dependency); + } + + @Test(expected = IOException.class) + @SuppressWarnings("PMD.NonStaticInitializer") + public void testFetchMavenArtifactsAlwaysThrowsIOException(@Mocked final CentralSearch centralSearch, + @Mocked final Dependency dependency) + throws IOException { + + CentralAnalyzer instance = new CentralAnalyzer(); + instance.searcher = centralSearch; + specifySha1SumFor(dependency); + + new Expectations() {{ + centralSearch.searchSha1(SHA1_SUM); + result = new IOException("no internet connection"); + }}; + + instance.fetchMavenArtifacts(dependency); + } + + @Test(expected = AnalysisException.class) + @SuppressWarnings("PMD.NonStaticInitializer") + public void testFetchMavenArtifactsAlwaysThrowsIOExceptionLetsTheAnalysisFail(@Mocked final CentralSearch centralSearch, + @Mocked final Dependency dependency) + throws AnalysisException, IOException { + + CentralAnalyzer instance = new CentralAnalyzer(); + instance.searcher = centralSearch; + specifySha1SumFor(dependency); + + new Expectations() {{ + centralSearch.searchSha1(SHA1_SUM); + result = new IOException("no internet connection"); + }}; + + instance.analyze(dependency, null); + } + + /** + * We do not want to waste time in unit tests. + */ + private static void doNotSleepBetweenRetries() { + new MockUp() { + @Mock + void sleep(long millis) { + // do not sleep + } + }; + } + + /** + * Specifies the mock dependency's SHA1 sum. + * + * @param dependency then dependency + */ + @SuppressWarnings("PMD.NonStaticInitializer") + private void specifySha1SumFor(final Dependency dependency) { + new Expectations() {{ + dependency.getSha1sum(); + returns(SHA1_SUM); + }}; + } +}