CentralAnalyzer: Implement retry for fetching MavenArtifacts due to sporadic issues

dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CentralAnalyzer.java

@@ -74,6 +74,13 @@ public class CentralAnalyzer extends AbstractFileTypeAnalyzer {
      */
     private static final String SUPPORTED_EXTENSIONS = "jar";
 
+    /**
+     * There may be temporary issues when connecting to MavenCentral.
+     * In order to compensate for 99% of the issues, we perform a retry
+     * before finally raising the {@link #errorFlag}.
+     */
+    private static final int NUMBER_OF_TRIES = 5;
+
     /**
      * The analyzer should be disabled if there are errors, so this is a flag to
      * determine if such an error has occurred.
@@ -83,7 +90,8 @@ public class CentralAnalyzer extends AbstractFileTypeAnalyzer {
     /**
      * The searcher itself.
      */
-    private CentralSearch searcher;
+    CentralSearch searcher;
+
     /**
      * Field indicating if the analyzer is enabled.
      */
@@ -212,7 +220,7 @@ public class CentralAnalyzer extends AbstractFileTypeAnalyzer {
         }
 
         try {
-            final List<MavenArtifact> mas = searcher.searchSha1(dependency.getSha1sum());
+            final List<MavenArtifact> 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 +264,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);
+            LOGGER.warn("Could not connect to Central search. Disabling this analyzer.", ioe);
             errorFlag = true;
         }
     }
+
+    /**
+     * Downloads the corresponding list of MavenArtifacts of the given
+     * dependency from MavenCentral.
+     * <p>
+     * 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
+     */
+    List<MavenArtifact> 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();
+                    }
+                }
+            }
+        }
+
+        LOGGER.warn("Finally failed connecting to Central search." +
+                        " Giving up after {} tries. Last exception was: {}",
+                NUMBER_OF_TRIES, lastException);
+        throw lastException;
+    }
 }

dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java

@@ -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<MavenArtifact> 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;
     }

dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CentralAnalyzerTest.java

@@ -0,0 +1,127 @@
+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.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
+    public void testFetchMavenArtifactsWithoutException(@Mocked final CentralSearch centralSearch,
+                                                        @Mocked final Dependency dependency)
+            throws IOException {
+
+        CentralAnalyzer instance = new CentralAnalyzer();
+        instance.searcher = centralSearch;
+
+        final List<MavenArtifact> expectedMavenArtifacts = Collections.emptyList();
+        new Expectations() {{
+            dependency.getSha1sum();
+            returns(SHA1_SUM);
+
+            centralSearch.searchSha1(SHA1_SUM);
+            returns(expectedMavenArtifacts);
+        }};
+
+        final List<MavenArtifact> actualMavenArtifacts = instance.fetchMavenArtifacts(dependency);
+
+        assertEquals(expectedMavenArtifacts, actualMavenArtifacts);
+    }
+
+    @Test
+    public void testFetchMavenArtifactsWithSporadicIOException(@Mocked final CentralSearch centralSearch,
+                                                               @Mocked final Dependency dependency)
+            throws IOException {
+
+        CentralAnalyzer instance = new CentralAnalyzer();
+        instance.searcher = centralSearch;
+
+        final List<MavenArtifact> expectedMavenArtifacts = Collections.emptyList();
+        new Expectations() {{
+            dependency.getSha1sum();
+            returns(SHA1_SUM);
+
+            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<MavenArtifact> actualMavenArtifacts = instance.fetchMavenArtifacts(dependency);
+
+        assertEquals(expectedMavenArtifacts, actualMavenArtifacts);
+    }
+
+    @Test(expected = FileNotFoundException.class)
+    public void testFetchMavenArtifactsRethrowsFileNotFoundException(@Mocked final CentralSearch centralSearch,
+                                                                     @Mocked final Dependency dependency)
+            throws IOException {
+
+        CentralAnalyzer instance = new CentralAnalyzer();
+        instance.searcher = centralSearch;
+
+        new Expectations() {{
+            dependency.getSha1sum();
+            returns(SHA1_SUM);
+
+            centralSearch.searchSha1(SHA1_SUM);
+            result = new FileNotFoundException("Artifact not found in Central");
+        }};
+
+        instance.fetchMavenArtifacts(dependency);
+    }
+
+    @Test(expected = IOException.class)
+    public void testFetchMavenArtifactsAlwaysThrowsIOException(@Mocked final CentralSearch centralSearch,
+                                                               @Mocked final Dependency dependency)
+            throws IOException {
+
+        CentralAnalyzer instance = new CentralAnalyzer();
+        instance.searcher = centralSearch;
+
+        new Expectations() {{
+            dependency.getSha1sum();
+            returns(SHA1_SUM);
+
+            centralSearch.searchSha1(SHA1_SUM);
+            result = new IOException("no internet connection");
+        }};
+
+        instance.fetchMavenArtifacts(dependency);
+    }
+
+    /**
+     * We do not want to waste time in unit tests.
+     */
+    private static void doNotSleepBetweenRetries() {
+        new MockUp<Thread>() {
+            @Mock
+            void sleep(long millis) {
+                // do not sleep
+            }
+        };
+    }
+}