Merge branch 'main' into harry/pnpm-trust-downgrade-version-fallback

thavaahariharangit · web-flow · commit e42be256a7f5 · 2026-02-19T06:16:01.000Z
diff --git a/maven/lib/dependabot/maven/file_parser/property_value_finder.rb b/maven/lib/dependabot/maven/file_parser/property_value_finder.rb
@@ -20,6 +20,7 @@ class PropertyValueFinder
         require_relative "pom_fetcher"
 
         DOT_SEPARATOR_REGEX = %r{\.(?!\d+([.\/_\-]|$)+)}
+        MAVEN_PROPERTY_REGEX = /\$\{.+?/
 
         sig do
           params(
@@ -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!
@@ -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\./, "")
diff --git a/maven/spec/dependabot/maven/file_parser/property_value_finder_spec.rb b/maven/spec/dependabot/maven/file_parser/property_value_finder_spec.rb
@@ -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" }
@@ -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" }
diff --git a/maven/spec/fixtures/poms/pom_with_nested_properties.xml b/maven/spec/fixtures/poms/pom_with_nested_properties.xml
@@ -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>
