diff --git a/maven/lib/dependabot/maven/file_parser.rb b/maven/lib/dependabot/maven/file_parser.rb index baca862b5b6..f1eee3b5411 100644 --- a/maven/lib/dependabot/maven/file_parser.rb +++ b/maven/lib/dependabot/maven/file_parser.rb @@ -267,7 +267,7 @@ def value_for_property(property_name, pom) # values from parent POMs) def property_value_finder @property_value_finder ||= - PropertyValueFinder.new(dependency_files: dependency_files) + PropertyValueFinder.new(dependency_files: dependency_files, credentials: credentials) end def pomfiles diff --git a/maven/lib/dependabot/maven/file_parser/pom_fetcher.rb b/maven/lib/dependabot/maven/file_parser/pom_fetcher.rb new file mode 100644 index 00000000000..801d2d12e79 --- /dev/null +++ b/maven/lib/dependabot/maven/file_parser/pom_fetcher.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +require "nokogiri" + +require "dependabot/dependency_file" +require "dependabot/maven/file_parser" +require "dependabot/registry_client" + +module Dependabot + module Maven + class FileParser + class PomFetcher + def initialize(dependency_files:) + @dependency_files = dependency_files + @poms = {} + end + + def internal_dependency_poms + return @internal_dependency_poms if @internal_dependency_poms + + @internal_dependency_poms = {} + dependency_files.each do |pom| + doc = Nokogiri::XML(pom.content) + group_id = doc.at_css("project > groupId") || + doc.at_css("project > parent > groupId") + artifact_id = doc.at_css("project > artifactId") + + next unless group_id && artifact_id + + dependency_name = [ + group_id.content.strip, + artifact_id.content.strip + ].join(":") + + @internal_dependency_poms[dependency_name] = pom + end + + @internal_dependency_poms + end + + def fetch_remote_parent_pom(group_id, artifact_id, version, urls_to_try) + pom_id = "#{group_id}:#{artifact_id}:#{version}" + return @poms[pom_id] if @poms.key?(pom_id) + + urls_to_try.each do |base_url| + url = + if version.include?("SNAPSHOT") + fetch_snapshot_pom_url(group_id, artifact_id, version, base_url) + else + remote_pom_url(group_id, artifact_id, version, base_url) + end + next if url.nil? + + response = fetch(url) + next unless response.status == 200 + next unless pom?(response.body) + + dependency_file = DependencyFile.new( + name: "remote_pom.xml", + content: response.body + ) + + @poms[pom_id] = dependency_file + return dependency_file + rescue Excon::Error::Socket, Excon::Error::Timeout, + Excon::Error::TooManyRedirects, URI::InvalidURIError + nil + end + + # If a parent POM couldn't be found, return `nil` + nil + end + + private + + def remote_pom_url(group_id, artifact_id, version, base_repo_url) + "#{base_repo_url}/" \ + "#{group_id.tr('.', '/')}/#{artifact_id}/#{version}/" \ + "#{artifact_id}-#{version}.pom" + end + + def remote_pom_snapshot_url(group_id, artifact_id, version, snapshot_version, base_repo_url) + "#{base_repo_url}/" \ + "#{group_id.tr('.', '/')}/#{artifact_id}/#{version}/" \ + "#{artifact_id}-#{snapshot_version}.pom" + end + + def remote_pom_snapshot_metadata_url(group_id, artifact_id, version, base_repo_url) + "#{base_repo_url}/" \ + "#{group_id.tr('.', '/')}/#{artifact_id}/#{version}/" \ + "maven-metadata.xml" + end + + def fetch_snapshot_pom_url(group_id, artifact_id, version, base_url) + url = remote_pom_snapshot_metadata_url(group_id, artifact_id, version, base_url) + response = fetch(url) + return nil unless response.status == 200 + + snapshot = Nokogiri::XML(response.body). + css("snapshotVersion"). + find { |node| node.at_css("extension").content == "pom" }&. + at_css("value")&. + content + return nil unless snapshot + + remote_pom_snapshot_url(group_id, artifact_id, version, snapshot, base_url) + end + + def fetch(url) + @maven_responses ||= {} + @maven_responses[url] ||= Dependabot::RegistryClient.get(url: url, options: { retry_limit: 1 }) + end + + def pom?(content) + !Nokogiri::XML(content).at_css("project > artifactId").nil? + end + + attr_reader :dependency_files + end + end + end +end diff --git a/maven/lib/dependabot/maven/file_parser/property_value_finder.rb b/maven/lib/dependabot/maven/file_parser/property_value_finder.rb index 3bde0ef8a06..e7b12b6271d 100644 --- a/maven/lib/dependabot/maven/file_parser/property_value_finder.rb +++ b/maven/lib/dependabot/maven/file_parser/property_value_finder.rb @@ -14,12 +14,14 @@ module Maven class FileParser class PropertyValueFinder require_relative "repositories_finder" + require_relative "pom_fetcher" DOT_SEPARATOR_REGEX = %r{\.(?!\d+([.\/_\-]|$)+)}.freeze def initialize(dependency_files:, credentials: []) @dependency_files = dependency_files @credentials = credentials + @pom_fetcher = PomFetcher.new(dependency_files: dependency_files) end def property_details(property_name:, callsite_pom:) @@ -61,29 +63,6 @@ def property_details(property_name:, callsite_pom:) attr_reader :dependency_files - def internal_dependency_poms - return @internal_dependency_poms if @internal_dependency_poms - - @internal_dependency_poms = {} - dependency_files.each do |pom| - doc = Nokogiri::XML(pom.content) - group_id = doc.at_css("project > groupId") || - doc.at_css("project > parent > groupId") - artifact_id = doc.at_css("project > artifactId") - - next unless group_id && artifact_id - - dependency_name = [ - group_id.content.strip, - artifact_id.content.strip - ].join(":") - - @internal_dependency_poms[dependency_name] = pom - end - - @internal_dependency_poms - end - def sanitize_property_name(property_name) property_name.sub(/^pom\./, "").sub(/^project\./, "") end @@ -101,11 +80,11 @@ def parent_pom(pom) name = [group_id, artifact_id].join(":") - return internal_dependency_poms[name] if internal_dependency_poms[name] + return @pom_fetcher.internal_dependency_poms[name] if @pom_fetcher.internal_dependency_poms[name] return unless version && !version.include?(",") - fetch_remote_parent_pom(group_id, artifact_id, version, pom) + @pom_fetcher.fetch_remote_parent_pom(group_id, artifact_id, version, parent_repository_urls(pom)) end # rubocop:enable Metrics/PerceivedComplexity @@ -119,45 +98,12 @@ def parent_repository_urls(pom) def repositories_finder @repositories_finder ||= RepositoriesFinder.new( + pom_fetcher: @pom_fetcher, dependency_files: dependency_files, credentials: @credentials, evaluate_properties: false ) end - - def fetch_remote_parent_pom(group_id, artifact_id, version, pom) - parent_repository_urls(pom).each do |base_url| - url = remote_pom_url(group_id, artifact_id, version, base_url) - - @maven_responses ||= {} - @maven_responses[url] ||= Dependabot::RegistryClient.get(url: url) - next unless @maven_responses[url].status == 200 - next unless pom?(@maven_responses[url].body) - - dependency_file = DependencyFile.new( - name: "remote_pom.xml", - content: @maven_responses[url].body - ) - - return dependency_file - rescue Excon::Error::Socket, Excon::Error::Timeout, - Excon::Error::TooManyRedirects, URI::InvalidURIError - nil - end - - # If a parent POM couldn't be found, return `nil` - nil - end - - def remote_pom_url(group_id, artifact_id, version, base_repo_url) - "#{base_repo_url}/" \ - "#{group_id.tr('.', '/')}/#{artifact_id}/#{version}/" \ - "#{artifact_id}-#{version}.pom" - end - - def pom?(content) - !Nokogiri::XML(content).at_css("project > artifactId").nil? - end end end end diff --git a/maven/lib/dependabot/maven/file_parser/repositories_finder.rb b/maven/lib/dependabot/maven/file_parser/repositories_finder.rb index 7448d1ea1ea..a953c640e63 100644 --- a/maven/lib/dependabot/maven/file_parser/repositories_finder.rb +++ b/maven/lib/dependabot/maven/file_parser/repositories_finder.rb @@ -15,13 +15,15 @@ module Maven class FileParser class RepositoriesFinder require_relative "property_value_finder" + require_relative "pom_fetcher" # In theory we should check the artifact type and either look in # or . In practice it's unlikely # anyone makes this distinction. REPOSITORY_SELECTOR = "repositories > repository, " \ "pluginRepositories > pluginRepository" - def initialize(dependency_files: [], credentials: [], evaluate_properties: true) + def initialize(pom_fetcher:, dependency_files: [], credentials: [], evaluate_properties: true) + @pom_fetcher = pom_fetcher @dependency_files = dependency_files @credentials = credentials @@ -94,74 +96,15 @@ def parent_pom(pom, repo_urls) name = [group_id, artifact_id].join(":") - return internal_dependency_poms[name] if internal_dependency_poms[name] + return @pom_fetcher.internal_dependency_poms[name] if @pom_fetcher.internal_dependency_poms[name] return unless version && !version.include?(",") - fetch_remote_parent_pom(group_id, artifact_id, version, repo_urls) + urls = urls_from_credentials + repo_urls + [central_repo_url] + @pom_fetcher.fetch_remote_parent_pom(group_id, artifact_id, version, urls) end # rubocop:enable Metrics/PerceivedComplexity - def internal_dependency_poms - return @internal_dependency_poms if @internal_dependency_poms - - @internal_dependency_poms = {} - dependency_files.each do |pom| - doc = Nokogiri::XML(pom.content) - group_id = doc.at_css("project > groupId") || - doc.at_css("project > parent > groupId") - artifact_id = doc.at_css("project > artifactId") - - next unless group_id && artifact_id - - dependency_name = [ - group_id.content.strip, - artifact_id.content.strip - ].join(":") - - @internal_dependency_poms[dependency_name] = pom - end - - @internal_dependency_poms - end - - def fetch_remote_parent_pom(group_id, artifact_id, version, repo_urls) - (urls_from_credentials + repo_urls + [central_repo_url]).uniq.each do |base_url| - url = remote_pom_url(group_id, artifact_id, version, base_url) - - @maven_responses ||= {} - @maven_responses[url] ||= Dependabot::RegistryClient.get( - url: url, - # We attempt to find dependencies in private repos before failing over to the central repository, - # but this can burn a lot of a job's time against slow servers due to our `read_timeout` being 20 seconds. - # - # In order to avoid the overall job timing out, we only make one retry attempt - options: { retry_limit: 1 } - ) - next unless @maven_responses[url].status == 200 - next unless pom?(@maven_responses[url].body) - - dependency_file = DependencyFile.new( - name: "remote_pom.xml", - content: @maven_responses[url].body - ) - - return dependency_file - rescue Excon::Error::Socket, Excon::Error::Timeout, - Excon::Error::TooManyRedirects, URI::InvalidURIError - nil - end - - # If a parent POM couldn't be found, return `nil` - nil - end - - def remote_pom_url(group_id, artifact_id, version, base_repo_url) - "#{base_repo_url}/" \ - "#{group_id.tr('.', '/')}/#{artifact_id}/#{version}/" \ - "#{artifact_id}-#{version}.pom" - end - def urls_from_credentials @credentials. select { |cred| cred["type"] == "maven_repository" }. @@ -200,16 +143,12 @@ def value_for_property(property_name, pom) # values from parent POMs) def property_value_finder @property_value_finder ||= - PropertyValueFinder.new(dependency_files: dependency_files) + PropertyValueFinder.new(dependency_files: dependency_files, credentials: @credentials) end def property_regex Maven::FileParser::PROPERTY_REGEX end - - def pom?(content) - !Nokogiri::XML(content).at_css("project > artifactId").nil? - end end end end diff --git a/maven/lib/dependabot/maven/update_checker.rb b/maven/lib/dependabot/maven/update_checker.rb index 05778a778ec..6142d536bcf 100644 --- a/maven/lib/dependabot/maven/update_checker.rb +++ b/maven/lib/dependabot/maven/update_checker.rb @@ -138,7 +138,7 @@ def property_updater def property_value_finder @property_value_finder ||= Maven::FileParser::PropertyValueFinder. - new(dependency_files: dependency_files) + new(dependency_files: dependency_files, credentials: credentials) end def version_comes_from_multi_dependency_property? diff --git a/maven/lib/dependabot/maven/update_checker/version_finder.rb b/maven/lib/dependabot/maven/update_checker/version_finder.rb index 0d78f1d490e..b7a07b093de 100644 --- a/maven/lib/dependabot/maven/update_checker/version_finder.rb +++ b/maven/lib/dependabot/maven/update_checker/version_finder.rb @@ -198,10 +198,18 @@ def repositories @repositories end + def repository_finder + @repository_finder ||= + Maven::FileParser::RepositoriesFinder.new( + pom_fetcher: Maven::FileParser::PomFetcher.new(dependency_files: dependency_files), + dependency_files: dependency_files, + credentials: credentials + ) + end + def pom_repository_details @pom_repository_details ||= - Maven::FileParser::RepositoriesFinder. - new(dependency_files: dependency_files, credentials: credentials). + repository_finder. repository_urls(pom: pom). map do |url| { "url" => url, "auth_headers" => {} } @@ -271,9 +279,7 @@ def version_class end def central_repo_urls - central_url_without_protocol = - Maven::FileParser::RepositoriesFinder.new(credentials: credentials).central_repo_url. - gsub(%r{^.*://}, "") + central_url_without_protocol = repository_finder.central_repo_url.gsub(%r{^.*://}, "") %w(http:// https://).map { |p| p + central_url_without_protocol } end diff --git a/maven/spec/dependabot/maven/file_parser/pom_fetcher_spec.rb b/maven/spec/dependabot/maven/file_parser/pom_fetcher_spec.rb new file mode 100644 index 00000000000..fc579e8e6b5 --- /dev/null +++ b/maven/spec/dependabot/maven/file_parser/pom_fetcher_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require "spec_helper" +require "dependabot/dependency_file" +require "dependabot/maven/file_parser/pom_fetcher" + +RSpec.describe Dependabot::Maven::FileParser::PomFetcher do + let(:fetcher) { described_class.new(dependency_files: dependency_files) } + let(:dependency_files) { [] } + + describe "#fetch_remote_parent_pom" do + subject(:fetch_remote_parent_pom) { fetcher.fetch_remote_parent_pom(group_id, artifact_id, version, urls_to_try) } + let(:group_id) { "org.springframework.boot" } + let(:artifact_id) { "spring-boot-starter-parent" } + let(:version) { "1.5.9.RELEASE" } + let(:urls_to_try) { ["https://repo.maven.apache.org/maven2"] } + + before do + stub_request(:get, "https://repo.maven.apache.org/maven2/" \ + "org/springframework/boot/" \ + "spring-boot-starter-parent/" \ + "1.5.9.RELEASE/" \ + "spring-boot-starter-parent-1.5.9.RELEASE.pom"). + to_return(status: 200, body: "spring-boot-dependencies") + end + + context "when the parent pom is a release" do + it "returns the parent pom" do + expect(fetch_remote_parent_pom).to be_a(Dependabot::DependencyFile) + expect(fetch_remote_parent_pom.name).to eq("remote_pom.xml") + expect(fetch_remote_parent_pom.content).to include("spring-boot-dependencies") + end + end + + context "when the parent pom is a snapshot" do + let(:version) { "1.5.10-SNAPSHOT" } + + before do + stub_request(:get, "https://repo.maven.apache.org/maven2/" \ + "org/springframework/boot/" \ + "spring-boot-starter-parent/" \ + "1.5.10-SNAPSHOT/" \ + "maven-metadata.xml"). + to_return(status: 200, body: fixture("maven_central_metadata", "snapshot.xml")) + stub_request(:get, "https://repo.maven.apache.org/maven2/" \ + "org/springframework/boot/" \ + "spring-boot-starter-parent/" \ + "1.5.10-SNAPSHOT/" \ + "spring-boot-starter-parent-14.9-20221018.091616-23.pom"). + to_return(status: 200, body: "snapshot") + end + + it "returns the parent pom" do + expect(fetch_remote_parent_pom).to be_a(Dependabot::DependencyFile) + expect(fetch_remote_parent_pom.name).to eq("remote_pom.xml") + expect(fetch_remote_parent_pom.content).to include("snapshot") + end + + context "but the response is malformed" do + before do + stub_request(:get, "https://repo.maven.apache.org/maven2/" \ + "org/springframework/boot/" \ + "spring-boot-starter-parent/" \ + "1.5.10-SNAPSHOT/" \ + "maven-metadata.xml"). + to_return(status: 200, body: "404") + end + + it "returns nil" do + expect(fetch_remote_parent_pom).to be_nil + end + end + end + end +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 index 9a9f2cdfe44..b04890847b2 100644 --- a/maven/spec/dependabot/maven/file_parser/repositories_finder_spec.rb +++ b/maven/spec/dependabot/maven/file_parser/repositories_finder_spec.rb @@ -3,10 +3,12 @@ require "spec_helper" require "dependabot/dependency_file" require "dependabot/maven/file_parser/repositories_finder" +require "dependabot/maven/file_parser/pom_fetcher" RSpec.describe Dependabot::Maven::FileParser::RepositoriesFinder do let(:finder) do described_class.new( + pom_fetcher: pom_fetcher, dependency_files: dependency_files, credentials: credentials ) @@ -19,6 +21,7 @@ content: fixture("poms", base_pom_fixture_name) ) end + let(:pom_fetcher) { Dependabot::Maven::FileParser::PomFetcher.new(dependency_files: dependency_files) } let(:base_pom_fixture_name) { "basic_pom.xml" } describe "#central_repo_url" do diff --git a/maven/spec/fixtures/maven_central_metadata/snapshot.xml b/maven/spec/fixtures/maven_central_metadata/snapshot.xml new file mode 100644 index 00000000000..c5911cdb9d7 --- /dev/null +++ b/maven/spec/fixtures/maven_central_metadata/snapshot.xml @@ -0,0 +1,29 @@ + + org.springframework.boot + spring-boot-starter-parent + 1.5.9-SNAPSHOT + + + 20221018.091616 + 23 + + 20221018091616 + + + jar + 14.9-20221018.091616-21 + 20221018091611 + + + war + 14.9-20221008.091616-21 + 2022118091611 + + + pom + 14.9-20221018.091616-23 + 20221018091616 + + + +