feat: ArtifactoryAnalyzer updated to use the HTTPClient5-based Downloader and skip unusable results (#7293)

Author: aikebah

core/src/main/java/org/owasp/dependencycheck/data/artifactory/ArtifactorySearch.java

@@ -19,33 +19,26 @@
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.io.InputStreamReader;
-import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
-import java.util.Base64;
 import java.util.List;
 import java.util.UUID;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 import javax.annotation.concurrent.ThreadSafe;
 
+import org.apache.hc.core5.http.message.BasicHeader;
 import org.owasp.dependencycheck.data.nexus.MavenArtifact;
 import org.owasp.dependencycheck.dependency.Dependency;
 import org.owasp.dependencycheck.utils.Checksum;
-import org.owasp.dependencycheck.utils.InvalidSettingException;
+import org.owasp.dependencycheck.utils.Downloader;
+import org.owasp.dependencycheck.utils.ResourceNotFoundException;
 import org.owasp.dependencycheck.utils.Settings;
-import org.owasp.dependencycheck.utils.URLConnectionFactory;
+import org.owasp.dependencycheck.utils.TooManyRequestsException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.fasterxml.jackson.core.JsonParser;
-import com.fasterxml.jackson.databind.DeserializationFeature;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.ObjectReader;
 
 /**
  * Class of methods to search Artifactory for hashes and determine Maven GAV
@@ -56,22 +49,13 @@
  * @author nhenneaux
  */
 @ThreadSafe
-@SuppressWarnings("squid:S2647")
 public class ArtifactorySearch {
 
     /**
      * Used for logging.
      */
     private static final Logger LOGGER = LoggerFactory.getLogger(ArtifactorySearch.class);
 
-    /**
-     * Pattern to match the path returned by the Artifactory AQL API.
-     */
-    private static final Pattern PATH_PATTERN = Pattern.compile("^/(?<groupId>.+)/(?<artifactId>[^/]+)/(?<version>[^/]+)/[^/]+$");
-    /**
-     * Extracted duplicateArtifactorySearchIT.java comment.
-     */
-    private static final String WHILE_ACTUAL_IS = " while actual is ";
     /**
      * The URL for the Central service.
      */
@@ -80,46 +64,29 @@ public class ArtifactorySearch {
     /**
      * Whether to use the Proxy when making requests.
      */
-    private final boolean useProxy;
-
-    /**
-     * The configured settings.
-     */
-    private final Settings settings;
+    private final boolean allowUsingProxy;
 
-    /**
-     * Search result reader
-     */
-    private final ObjectReader objectReader;
 
     /**
-     * Creates a NexusSearch for the given repository URL.
+     * Creates a ArtifactorySearch for the given repository URL.
      *
      * @param settings the configured settings
      */
     public ArtifactorySearch(Settings settings) {
-        this.settings = settings;
 
         final String searchUrl = settings.getString(Settings.KEYS.ANALYZER_ARTIFACTORY_URL);
 
         this.rootURL = searchUrl;
         LOGGER.debug("Artifactory Search URL {}", searchUrl);
 
         if (null != settings.getString(Settings.KEYS.PROXY_SERVER)) {
-            boolean useProxySettings = false;
-            try {
-                useProxySettings = settings.getBoolean(Settings.KEYS.ANALYZER_ARTIFACTORY_USES_PROXY);
-            } catch (InvalidSettingException e) {
-                LOGGER.error("Settings {} is invalid, only, true/false is valid", Settings.KEYS.ANALYZER_ARTIFACTORY_USES_PROXY, e);
-            }
-            this.useProxy = useProxySettings;
-            LOGGER.debug("Using proxy? {}", useProxy);
+            this.allowUsingProxy = settings.getBoolean(Settings.KEYS.ANALYZER_ARTIFACTORY_USES_PROXY, false);
+            LOGGER.debug("Using proxy configuration? {}", allowUsingProxy);
         } else {
-            useProxy = false;
-            LOGGER.debug("Not using proxy");
+            this.allowUsingProxy = settings.getBoolean(Settings.KEYS.ANALYZER_ARTIFACTORY_USES_PROXY, true);
+            LOGGER.debug("Using default non-legacy proxy configuration");
         }
 
-        objectReader = new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false).readerFor(FileImpl.class);
     }
 
     /**
@@ -137,50 +104,19 @@ public List<MavenArtifact> search(Dependency dependency) throws IOException {
 
         final String sha1sum = dependency.getSha1sum();
         final URL url = buildUrl(sha1sum);
-        final HttpURLConnection conn = connect(url);
-        final int responseCode = conn.getResponseCode();
-        if (responseCode == 200) {
-            return processResponse(dependency, conn);
-        }
-        throw new IOException("Could not connect to Artifactory " + url + " (" + responseCode + "): " + conn.getResponseMessage());
-
-    }
-
-    /**
-     * Makes an connection to the given URL.
-     *
-     * @param url the URL to connect to
-     * @return the HTTP URL Connection
-     * @throws IOException thrown if there is an error making the connection
-     */
-    private HttpURLConnection connect(URL url) throws IOException {
-        LOGGER.debug("Searching Artifactory url {}", url);
-
-        // Determine if we need to use a proxy. The rules:
-        // 1) If the proxy is set, AND the setting is set to true, use the proxy
-        // 2) Otherwise, don't use the proxy (either the proxy isn't configured,
-        // or proxy is specifically set to false)
-        final URLConnectionFactory factory = new URLConnectionFactory(settings);
-        final HttpURLConnection conn = factory.createHttpURLConnection(url, useProxy);
-        conn.setDoOutput(true);
-
-        conn.addRequestProperty("X-Result-Detail", "info");
-
-        final String username = settings.getString(Settings.KEYS.ANALYZER_ARTIFACTORY_API_USERNAME);
-        final String apiToken = settings.getString(Settings.KEYS.ANALYZER_ARTIFACTORY_API_TOKEN);
-        if (username != null && apiToken != null) {
-            final String userpassword = username + ":" + apiToken;
-            final String encodedAuthorization = Base64.getEncoder().encodeToString(userpassword.getBytes(StandardCharsets.UTF_8));
-            conn.addRequestProperty("Authorization", "Basic " + encodedAuthorization);
-        } else {
-            final String bearerToken = settings.getString(Settings.KEYS.ANALYZER_ARTIFACTORY_BEARER_TOKEN);
-            if (bearerToken != null) {
-                conn.addRequestProperty("Authorization", "Bearer " + bearerToken);
-            }
+        final StringBuilder msg = new StringBuilder("Could not connect to Artifactory at")
+                .append(url);
+        try {
+            final BasicHeader artifactoryResultDetail = new BasicHeader("X-Result-Detail", "info");
+            return Downloader.getInstance().fetchAndHandle(url, new ArtifactorySearchResponseHandler(dependency), List.of(artifactoryResultDetail),
+                    allowUsingProxy);
+        } catch (TooManyRequestsException e) {
+            throw new IOException(msg.append(" (429): Too manu requests").toString(), e);
+        } catch (URISyntaxException e) {
+            throw new IOException(msg.append(" (400): Invalid URL").toString(), e);
+        } catch (ResourceNotFoundException e) {
+            throw new IOException(msg.append(" (404): Not found").toString(), e);
         }
-
-        conn.connect();
-        return conn;
     }
 
     /**
@@ -196,99 +132,6 @@ private URL buildUrl(String sha1sum) throws MalformedURLException {
         return new URL(rootURL + "/api/search/checksum?sha1=" + sha1sum);
     }
 
-    /**
-     * Process the Artifactory response.
-     *
-     * @param dependency the dependency
-     * @param conn the HTTP URL Connection
-     * @return a list of the Maven Artifact information
-     * @throws IOException thrown if there is an I/O error
-     */
-    protected List<MavenArtifact> processResponse(Dependency dependency, HttpURLConnection conn) throws IOException {
-        final List<MavenArtifact> result = new ArrayList<>();
-
-        try (InputStreamReader streamReader = new InputStreamReader(conn.getInputStream(), StandardCharsets.UTF_8);
-                JsonParser parser = objectReader.getFactory().createParser(streamReader)) {
-
-            if (init(parser) && parser.nextToken() == com.fasterxml.jackson.core.JsonToken.START_OBJECT) {
-                // at least one result
-                do {
-                    final FileImpl file = objectReader.readValue(parser);
-
-                    checkHashes(dependency, file.getChecksums());
-
-                    final Matcher pathMatcher = PATH_PATTERN.matcher(file.getPath());
-                    if (!pathMatcher.matches()) {
-                        throw new IllegalStateException("Cannot extract the Maven information from the path "
-                                + "retrieved in Artifactory " + file.getPath());
-                    }
-
-                    final String groupId = pathMatcher.group("groupId").replace('/', '.');
-                    final String artifactId = pathMatcher.group("artifactId");
-                    final String version = pathMatcher.group("version");
-
-                    result.add(new MavenArtifact(groupId, artifactId, version, file.getDownloadUri(),
-                            MavenArtifact.derivePomUrl(artifactId, version, file.getDownloadUri())));
-
-                } while (parser.nextToken() == com.fasterxml.jackson.core.JsonToken.START_OBJECT);
-            } else {
-                throw new FileNotFoundException("Artifact " + dependency + " not found in Artifactory");
-            }
-        }
-
-        return result;
-    }
-
-    protected boolean init(JsonParser parser) throws IOException {
-        com.fasterxml.jackson.core.JsonToken nextToken = parser.nextToken();
-        if (nextToken != com.fasterxml.jackson.core.JsonToken.START_OBJECT) {
-            throw new IOException("Expected " + com.fasterxml.jackson.core.JsonToken.START_OBJECT + ", got " + nextToken);
-        }
-
-        do {
-            nextToken = parser.nextToken();
-            if (nextToken == null) {
-                break;
-            }
-
-            if (nextToken.isStructStart()) {
-                if (nextToken == com.fasterxml.jackson.core.JsonToken.START_ARRAY && "results".equals(parser.currentName())) {
-                    return true;
-                } else {
-                    parser.skipChildren();
-                }
-            }
-        } while (true);
-
-        return false;
-    }
-
-    /**
-     * Validates the hashes of the dependency.
-     *
-     * @param dependency the dependency
-     * @param checksums the collection of checksums (md5, sha1, sha256)
-     * @throws FileNotFoundException thrown if one of the checksums does not
-     * match
-     */
-    private void checkHashes(Dependency dependency, ChecksumsImpl checksums) throws FileNotFoundException {
-        final String md5sum = dependency.getMd5sum();
-        if (!checksums.getMd5().equals(md5sum)) {
-            throw new FileNotFoundException("Artifact found by API is not matching the md5 "
-                    + "of the artifact (repository hash is " + checksums.getMd5() + WHILE_ACTUAL_IS + md5sum + ") !");
-        }
-        final String sha1sum = dependency.getSha1sum();
-        if (!checksums.getSha1().equals(sha1sum)) {
-            throw new FileNotFoundException("Artifact found by API is not matching the SHA1 "
-                    + "of the artifact (repository hash is " + checksums.getSha1() + WHILE_ACTUAL_IS + sha1sum + ") !");
-        }
-        final String sha256sum = dependency.getSha256sum();
-        if (checksums.getSha256() != null && !checksums.getSha256().equals(sha256sum)) {
-            throw new FileNotFoundException("Artifact found by API is not matching the SHA-256 "
-                    + "of the artifact (repository hash is " + checksums.getSha256() + WHILE_ACTUAL_IS + sha256sum + ") !");
-        }
-    }
-
     /**
      * Performs a pre-flight request to ensure the Artifactory service is
      * reachable.
@@ -297,17 +140,20 @@ private void checkHashes(Dependency dependency, ChecksumsImpl checksums) throws
      * <code>false</code>.
      */
     public boolean preflightRequest() {
+        URL url = null;
         try {
-            final URL url = buildUrl(Checksum.getSHA1Checksum(UUID.randomUUID().toString()));
-            final HttpURLConnection connection = connect(url);
-            if (connection.getResponseCode() != 200) {
-                LOGGER.warn("Expected 200 result from Artifactory ({}), got {}", url, connection.getResponseCode());
-                return false;
-            }
+            url = buildUrl(Checksum.getSHA1Checksum(UUID.randomUUID().toString()));
+            Downloader.getInstance().fetchContent(url, StandardCharsets.UTF_8);
             return true;
         } catch (IOException e) {
             LOGGER.error("Cannot connect to Artifactory", e);
             return false;
+        } catch (TooManyRequestsException e) {
+            LOGGER.warn("Expected 200 result from Artifactory ({}), got 429", url);
+            return false;
+        } catch (ResourceNotFoundException e) {
+            LOGGER.warn("Expected 200 result from Artifactory ({}), got 404", url);
+            return false;
         }
 
     }