Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 57 additions & 31 deletions maven/lib/dependabot/maven/file_parser/property_value_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class PropertyValueFinder
require_relative "pom_fetcher"

DOT_SEPARATOR_REGEX = %r{\.(?!\d+([.\/_\-]|$)+)}
MAVEN_PROPERTY_REGEX = /\$\{.+?/

sig do
params(
Expand All @@ -37,10 +38,15 @@ def initialize(dependency_files:, credentials: [])
)
end

# rubocop:disable Metrics/PerceivedComplexity
sig do
params(property_name: String, callsite_pom: DependencyFile).returns(T.nilable(T::Hash[Symbol, T.untyped]))
params(
property_name: String,
callsite_pom: DependencyFile,
seen_properties: T::Set[String]
).returns(T.nilable(T::Hash[Symbol, T.untyped]))
end
def property_details(property_name:, callsite_pom:)
def property_details(property_name:, callsite_pom:, seen_properties: Set.new)
pom = callsite_pom
doc = Nokogiri::XML(pom.content)
doc.remove_namespaces!
Expand All @@ -63,53 +69,73 @@ def property_details(property_name:, callsite_pom:)
raise DependencyFileNotEvaluatable, e.message
end

# and value is an expression
if node && /\$\{(?<expression>.+)\}/.match(node.content.strip)
return extract_value_from_expression(
expression: node.content.strip,
if node.nil? && parent_pom(pom)
return property_details(
property_name: property_name,
callsite_pom: callsite_pom
callsite_pom: T.must(parent_pom(pom)),
seen_properties: seen_properties
)
end
# If the property can’t be resolved for any reason, we return nil which
# causes Dependabot to skip the dependency.
# This differs from Maven’s behavior, where an unresolved property would fail the entire build.
# We intentionally choose this as a compromise so Dependabot can continue parsing the rest of the project,
# rather than failing completely due to a single unknown property.
# The trade-off is that some dependencies may not be updated as expected.
Dependabot.logger.warn "Could not resolve property '#{property_name}'" unless node
return nil unless node

content = node.content.strip

# Detect infinite recursion such as ${property1} where property1=${property1}
if seen_properties.include?(property_name)
raise Dependabot::DependencyFileNotParseable.new(
callsite_pom.name,
"Error trying to resolve recursive expression '${#{property_name}}'."
)
end

# If we found a property, return it
return { file: pom.name, node: node, value: node.content.strip } if node
seen_properties << property_name

# Otherwise, look for a value in this pom's parent
return unless (parent = parent_pom(pom))
# If the content has no placeholders, return it as-is
return { file: pom.name, node: node, value: content } unless content.match?(MAVEN_PROPERTY_REGEX)

property_details(
property_name: property_name,
callsite_pom: parent
)
resolve_property_placeholder(content, callsite_pom, pom, node, seen_properties)
end
# rubocop:enable Metrics/PerceivedComplexity

private

sig { returns(T::Array[DependencyFile]) }
attr_reader :dependency_files

# Extract property placeholders from a string and resolve them
# These properties can be simple properties such as ${project.version}
# or more complex such as ${my.property.${other.property}} or constant.${property}
# See https://maven.apache.org/pom.html#properties
sig do
params(
expression: String,
property_name: String,
callsite_pom: DependencyFile
)
.returns(T.nilable(T::Hash[Symbol, String]))
content: String,
callsite_pom: DependencyFile,
pom: DependencyFile,
node: T.untyped,
seen_properties: T::Set[String]
).returns(T.nilable(T::Hash[Symbol, T.untyped]))
end
def extract_value_from_expression(expression:, property_name:, callsite_pom:)
# and the expression is pointing to self then raise the error
if expression.eql?("${#{property_name}}")
raise Dependabot::DependencyFileNotParseable.new(
callsite_pom.name,
"Error trying to resolve recursive expression '#{expression}'."
def resolve_property_placeholder(content, callsite_pom, pom, node, seen_properties)
resolved_value = content.gsub(/\$\{(.+?)}/) do
inner_name = Regexp.last_match(1)
resolved = property_details(
property_name: T.must(inner_name),
callsite_pom: callsite_pom,
seen_properties: seen_properties
)
T.must(resolved)[:value]
end

# and the expression is pointing to another tag, then get the value of that tag
property_details(property_name: T.must(expression.slice(2..-2)), callsite_pom: callsite_pom)
{ file: pom.name, node: node, value: resolved_value }
end

sig { returns(T::Array[DependencyFile]) }
attr_reader :dependency_files

sig { params(property_name: String).returns(String) }
def sanitize_property_name(property_name)
property_name.sub(/^pom\./, "").sub(/^project\./, "")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,20 @@
its([:value]) { is_expected.to eq("0.0.2-RELEASE") }
end

context "when the property is an nested property using project definition properties" do
let(:base_pom_fixture_name) { "pom_with_nested_properties.xml" }
let(:property_name) { "channels.maven.groupId" }

its([:value]) { is_expected.to eq("org.reproducer.channels") }
end

context "when the property is nested multiple times" do
let(:base_pom_fixture_name) { "pom_with_nested_properties.xml" }
let(:property_name) { "channels.maven.groupId.nested" }

its([:value]) { is_expected.to eq("org.reproducer.channels2") }
end

context "when the property name starts with 'project' but not an attribute of the project" do
let(:base_pom_fixture_name) { "property_name_starts_with_project_pom.xml" }
let(:property_name) { "project.dependency.spring-boot.version" }
Expand Down Expand Up @@ -76,6 +90,25 @@
end
end

context "when the property cannot be resolved" do
subject(:property_result) { property_details }

let(:base_pom_fixture_name) { "property_pom_duplicate_tags.xml" }
let(:property_name) { "property.that.does.not.exist" }

it "returns nil" do
expect(property_result).to be_nil
end

it "logs a warning" do
expect(Dependabot.logger)
.to receive(:warn)
.with("Could not resolve property '#{property_name}'")

property_result
end
end

context "when the latest tag is pointing to another tag, then get the value of that tag" do
let(:base_pom_fixture_name) { "property_pom_duplicate_tags.xml" }
let(:property_name) { "orika.version" }
Expand Down
31 changes: 31 additions & 0 deletions maven/spec/fixtures/poms/pom_with_nested_properties.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
This POM verifies that properties can reference other properties, even across multiple levels.
Some complex projects (e.g., WildFly) rely on this pattern to dynamically manage project variants.
For more context, see https://github.com/dependabot/dependabot-core/issues/13713
-->
<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/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>org.reproducer</groupId>
<artifactId>test</artifactId>
<version>1.0.0-SNAPSHOT</version>

<properties>
<channels>channels2</channels>
<channels.maven.groupId.nested>${project.groupId}.${channels}</channels.maven.groupId.nested>
<channels.maven.groupId>${project.groupId}.channels</channels.maven.groupId>
</properties>

<dependencyManagement>
<dependencies>

<dependency>
<groupId>${channels.maven.groupId}</groupId>
<artifactId>wildfly-preview</artifactId>
<version>19.0.0.Final</version>
</dependency>

</dependencies>
</dependencyManagement>
</project>
Loading