Merge pull request #5878 from dependabot/jakecoffman/maven-overwrite-…

…repository-by-id Fix Maven inability to overwrite repository urls by ID
dependabot · Oct 13, 2022 · 5ae1a8c · 5ae1a8c
2 parents 57c10a7 + 02bcc0b
commit 5ae1a8c
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 15 deletions.
diff --git a/maven/lib/dependabot/maven/file_parser/repositories_finder.rb b/maven/lib/dependabot/maven/file_parser/repositories_finder.rb
@@ -24,6 +24,7 @@ class RepositoriesFinder
         # The Central Repository is included in the Super POM, which is
         # always inherited from.
         CENTRAL_REPO_URL = "https://repo.maven.apache.org/maven2"
+        SUPER_POM = { url: CENTRAL_REPO_URL, id: "central" }
 
         def initialize(dependency_files:, evaluate_properties: true)
           @dependency_files = dependency_files
@@ -36,27 +37,39 @@ def initialize(dependency_files:, evaluate_properties: true)
 
         # Collect all repository URLs from this POM and its parents
         def repository_urls(pom:, exclude_inherited: false)
-          repo_urls_in_pom =
+          entries = gather_repository_urls(pom: pom, exclude_inherited: exclude_inherited)
+          ids = Set.new
+          entries.map do |entry|
+            next if entry[:id] && ids.include?(entry[:id])
+
+            ids.add(entry[:id]) unless entry[:id].nil?
+            entry[:url]
+          end.uniq.compact
+        end
+
+        private
+
+        attr_reader :dependency_files
+
+        def gather_repository_urls(pom:, exclude_inherited: false)
+          repos_in_pom =
             Nokogiri::XML(pom.content).
             css(REPOSITORY_SELECTOR).
-            map { |node| node.at_css("url").content.strip.gsub(%r{/$}, "") }.
-            reject { |url| contains_property?(url) && !evaluate_properties? }.
-            select { |url| url.start_with?("http") }.
-            map { |url| evaluated_value(url, pom) }
+            map { |node| { url: node.at_css("url").content.strip, id: node.at_css("id").content.strip } }.
+            reject { |entry| contains_property?(entry[:url]) && !evaluate_properties? }.
+            select { |entry| entry[:url].start_with?("http") }.
+            map { |entry| { url: evaluated_value(entry[:url], pom).gsub(%r{/$}, ""), id: entry[:id] } }
 
-          return repo_urls_in_pom + [CENTRAL_REPO_URL] if exclude_inherited
+          return repos_in_pom + [SUPER_POM] if exclude_inherited
 
-          unless (parent = parent_pom(pom, repo_urls_in_pom))
-            return repo_urls_in_pom + [CENTRAL_REPO_URL]
+          urls_in_pom = repos_in_pom.map { |repo| repo[:url] }
+          unless (parent = parent_pom(pom, urls_in_pom))
+            return repos_in_pom + [SUPER_POM]
           end
 
-          repo_urls_in_pom + repository_urls(pom: parent)
+          repos_in_pom + gather_repository_urls(pom: parent)
         end
 
-        private
-
-        attr_reader :dependency_files
-
         def evaluate_properties?
           @evaluate_properties
         end

diff --git a/maven/spec/dependabot/maven/file_parser/repositories_finder_spec.rb b/maven/spec/dependabot/maven/file_parser/repositories_finder_spec.rb
@@ -39,6 +39,18 @@
         )
       end
 
+      context "that overwrites central" do
+        let(:base_pom_fixture_name) { "overwrite_central_pom.xml" }
+
+        it "does not include central" do
+          expect(repository_urls).to eq(
+            %w(
+              https://example.com
+            )
+          )
+        end
+      end
+
       context "that use properties" do
         let(:base_pom_fixture_name) { "property_repo_pom.xml" }
 

diff --git a/maven/spec/fixtures/poms/overwrite_central_pom.xml b/maven/spec/fixtures/poms/overwrite_central_pom.xml
@@ -0,0 +1,45 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>com.dependabot</groupId>
+  <artifactId>basic-pom</artifactId>
+  <version>0.0.1-RELEASE</version>
+  <name>Dependabot Basic POM</name>
+
+  <packaging>pom</packaging>
+
+  <repositories>
+    <repository>
+      <id>central</id>
+      <url>https://example.com</url>
+      <snapshots>
+        <enabled>false</enabled>
+      </snapshots>
+    </repository>
+  </repositories>
+
+  <dependencies>
+    <dependency>
+      <groupId>com.google.guava</groupId>
+      <artifactId>guava</artifactId>
+      <version>23.3-jre</version>
+      <scope>compile</scope>
+    </dependency>
+
+    <dependency>
+      <groupId>org.apache.httpcomponents</groupId>
+      <artifactId>httpclient</artifactId>
+      <version>4.5.3</version>
+      <scope>test</scope>
+    </dependency>
+
+    <dependency>
+      <groupId>io.mockk</groupId>
+      <artifactId>mockk</artifactId>
+      <version>1.0.0</version>
+      <classifier>sources</classifier>
+    </dependency>
+  </dependencies>
+</project>
diff --git a/maven/spec/fixtures/projects/invalid_repository_url/pom.xml b/maven/spec/fixtures/projects/invalid_repository_url/pom.xml
@@ -12,13 +12,13 @@
 
   <repositories>
     <repository>
-      <id>central</id>
+      <id>wrong</id>
       <url>http://host:port/content/groups/public</url>
     </repository>
   </repositories>
   <pluginRepositories>
     <pluginRepository>
-      <id>central</id>
+      <id>wrong</id>
       <url>http://host:port/content/groups/public</url>
     </pluginRepository>
   </pluginRepositories>