diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs index f25d82fac..334cf304c 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs @@ -80,9 +80,10 @@ public class MvnCliComponentDetector : FileComponentDetector private const string GroupIdSelector = "groupId"; private const string ArtifactIdSelector = "artifactId"; private const string VersionSelector = "version"; + private const int MaxPropertyExpansionDepth = 100; - private static readonly Regex VersionRegex = new( - @"^\$\{(.*)\}$", + private static readonly Regex PropertyReferenceRegex = new( + @"\$\{([^{}]+)\}", RegexOptions.Compiled | RegexOptions.IgnoreCase); // Auth error patterns to detect in Maven error output @@ -162,7 +163,7 @@ public MvnCliComponentDetector( public override IEnumerable SupportedComponentTypes => [ComponentType.Maven]; - public override int Version => 5; + public override int Version => 6; public override IEnumerable Categories => [Enum.GetName(typeof(DetectorClass), DetectorClass.Maven)]; @@ -228,6 +229,60 @@ private static string NormalizeEndpointUrl(string rawUrl) return $"{uri.Scheme}://{uri.Host}{port}"; } + private static string ResolveProperties( + string template, + Func resolveProperty) + { + var resolvedProperties = new Dictionary(StringComparer.Ordinal); + var activeProperties = new HashSet(StringComparer.Ordinal); + + string ResolveTemplate(string value, int depth) => + depth >= MaxPropertyExpansionDepth + ? value + : PropertyReferenceRegex.Replace( + value, + match => + { + var variable = match.Groups[1].Value; + if (resolvedProperties.TryGetValue(variable, out var resolvedProperty)) + { + return resolvedProperty; + } + + if (!activeProperties.Add(variable)) + { + return match.Value; + } + + var propertyValue = resolveProperty(variable); + if (propertyValue == null) + { + activeProperties.Remove(variable); + return match.Value; + } + + resolvedProperty = ResolveTemplate(propertyValue, depth + 1); + activeProperties.Remove(variable); + resolvedProperties[variable] = resolvedProperty; + return resolvedProperty; + }); + + return ResolveTemplate(template, 0); + } + + private static string ResolvePropertiesFromLocalOnly( + string template, + Dictionary localVariables) + { + return ResolveProperties( + template, + variable => localVariables.TryGetValue(variable, out var localReplacement) + ? localReplacement + : null); + } + + private static bool ContainsPropertyReference(string value) => value.Contains("${", StringComparison.Ordinal); + private void LogDebugWithId(string message) => this.Logger.LogDebug("{DetectorId}: {Message}", this.Id, message); @@ -705,40 +760,28 @@ private void ProcessPomFileStatically(ProcessRequest processRequest) if (version != null && !version.InnerText.Contains(',')) { var versionRef = version.InnerText.Trim('[', ']'); + var resolvedGroupId = ResolvePropertiesFromLocalOnly(groupId, localVariables); + var resolvedArtifactId = ResolvePropertiesFromLocalOnly(artifactId, localVariables); + var resolvedVersion = ResolvePropertiesFromLocalOnly(versionRef, localVariables); - if (versionRef.StartsWith("${")) - { - // Only resolve immediately if local variable exists (highest priority) - // Otherwise, defer to second pass to ensure proper hierarchy-aware resolution - var resolvedVersion = this.ResolveVersionFromLocalOnly(versionRef, localVariables); - if (!resolvedVersion.StartsWith("${")) - { - // Local variable found - resolve immediately (highest priority) - var component = new MavenComponent(groupId, artifactId, resolvedVersion); - var detectedComponent = new DetectedComponent(component); - singleFileComponentRecorder.RegisterUsage(detectedComponent); - Interlocked.Increment(ref this.staticParserComponentCount); - } - else - { - // No local variable - defer to second pass for hierarchy-aware resolution - // This ensures we consider all variable definitions before resolving - this.pendingComponents.Enqueue(new PendingComponent( - groupId, - artifactId, - versionRef, - singleFileComponentRecorder, - filePath)); - } - } - else + if (!ContainsPropertyReference(resolvedGroupId) + && !ContainsPropertyReference(resolvedArtifactId) + && !ContainsPropertyReference(resolvedVersion)) { - // Direct version - register immediately - var component = new MavenComponent(groupId, artifactId, versionRef); + var component = new MavenComponent(resolvedGroupId, resolvedArtifactId, resolvedVersion); var detectedComponent = new DetectedComponent(component); singleFileComponentRecorder.RegisterUsage(detectedComponent); Interlocked.Increment(ref this.staticParserComponentCount); } + else + { + this.pendingComponents.Enqueue(new PendingComponent( + groupId, + artifactId, + versionRef, + singleFileComponentRecorder, + filePath)); + } } else { @@ -997,32 +1040,6 @@ private void TrackMavenProjectCoordinates(XmlDocument document, XmlNamespaceMana } } - /// - /// Resolves a version template using only local variables from the current file. - /// This ensures immediate resolution only when the variable is defined in the same file (highest priority). - /// - /// The version template with variables (e.g., "${revision}"). - /// Local variables from the current file. - /// The resolved version string, or the original template if local variable not found. - private string ResolveVersionFromLocalOnly(string versionTemplate, Dictionary localVariables) - { - var resolvedVersion = versionTemplate; - var match = VersionRegex.Match(versionTemplate); - - if (match.Success) - { - var variable = match.Groups[1].Captures[0].ToString(); - - // Only check local variables (same file priority) - if (localVariables.TryGetValue(variable, out var localReplacement)) - { - resolvedVersion = versionTemplate.Replace("${" + variable + "}", localReplacement); - } - } - - return resolvedVersion; - } - private IEnumerable ExtractFailedEndpoints(string errorMessage) { if (string.IsNullOrWhiteSpace(errorMessage)) @@ -1158,10 +1175,21 @@ private void ResolvePendingComponents() { try { - var resolvedVersion = this.ResolveVersionWithHierarchyAwareness(pendingComponent.VersionTemplate, pendingComponent.FilePath); - if (!resolvedVersion.StartsWith("${")) + var resolvedGroupId = this.ResolvePropertiesWithHierarchyAwareness( + pendingComponent.GroupIdTemplate, + pendingComponent.FilePath); + var resolvedArtifactId = this.ResolvePropertiesWithHierarchyAwareness( + pendingComponent.ArtifactIdTemplate, + pendingComponent.FilePath); + var resolvedVersion = this.ResolvePropertiesWithHierarchyAwareness( + pendingComponent.VersionTemplate, + pendingComponent.FilePath); + + if (!ContainsPropertyReference(resolvedGroupId) + && !ContainsPropertyReference(resolvedArtifactId) + && !ContainsPropertyReference(resolvedVersion)) { - var component = new MavenComponent(pendingComponent.GroupId, pendingComponent.ArtifactId, resolvedVersion); + var component = new MavenComponent(resolvedGroupId, resolvedArtifactId, resolvedVersion); var detectedComponent = new DetectedComponent(component); pendingComponent.Recorder.RegisterUsage(detectedComponent); Interlocked.Increment(ref this.staticParserComponentCount); @@ -1171,10 +1199,10 @@ private void ResolvePendingComponents() { skippedCount++; this.Logger.LogDebug( - "Version string {Version} for component {Group}/{Artifact} could not be resolved and a component will not be recorded. File: {File}", + "Coordinate {Group}/{Artifact}/{Version} could not be fully resolved and a component will not be recorded. File: {File}", + resolvedGroupId, + resolvedArtifactId, resolvedVersion, - pendingComponent.GroupId, - pendingComponent.ArtifactId, pendingComponent.FilePath); } } @@ -1184,8 +1212,8 @@ private void ResolvePendingComponents() this.Logger.LogError( e, "Failed to resolve pending component {Group}/{Artifact} from {File}", - pendingComponent.GroupId, - pendingComponent.ArtifactId, + pendingComponent.GroupIdTemplate, + pendingComponent.ArtifactIdTemplate, pendingComponent.FilePath); } } @@ -1194,32 +1222,29 @@ private void ResolvePendingComponents() } /// - /// Resolves a version template with hierarchy-aware precedence. + /// Resolves a coordinate template with hierarchy-aware precedence. /// When multiple variable definitions exist, picks the closest ancestor to the requesting file. /// This implements Maven's property inheritance rule: child properties take precedence over parent properties. /// - /// The version template with variables (e.g., "${revision}"). + /// The coordinate template containing Maven property references. /// The file path of the POM requesting the variable resolution. - /// The resolved version string, or the original template if variables cannot be resolved. - private string ResolveVersionWithHierarchyAwareness(string versionTemplate, string requestingFilePath) + /// The template with all hierarchy-defined properties resolved. + private string ResolvePropertiesWithHierarchyAwareness(string template, string requestingFilePath) { - var resolvedVersion = versionTemplate; - var match = VersionRegex.Match(versionTemplate); + var unresolvedVariables = new HashSet(StringComparer.Ordinal); - if (match.Success) + return ResolveProperties( + template, + variable => { - var variable = match.Groups[1].Captures[0].ToString(); - - // Use Maven-compliant hierarchy search: current → parent → grandparent var foundValue = this.FindVariableInMavenHierarchy(variable, requestingFilePath); if (foundValue != null) { - resolvedVersion = versionTemplate.Replace("${" + variable + "}", foundValue.Value.Value); + return foundValue.Value.Value; } - else + + if (unresolvedVariables.Add(variable)) { - // Variable not found in Maven hierarchy - log at debug level since unresolved - // properties are common (profiles, external parents, etc.) and aggregate count in telemetry Interlocked.Increment(ref this.unresolvedVariableCount); this.Logger.LogDebug( "{DetectorId}: Variable {Variable} not found in Maven hierarchy for {File}", @@ -1227,9 +1252,9 @@ private string ResolveVersionWithHierarchyAwareness(string versionTemplate, stri variable, Path.GetFileName(requestingFilePath)); } - } - return resolvedVersion; + return null; + }); } /// diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/PendingComponent.cs b/src/Microsoft.ComponentDetection.Detectors/maven/PendingComponent.cs index 82c92b6fc..8ab65ec10 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/PendingComponent.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/PendingComponent.cs @@ -7,8 +7,8 @@ namespace Microsoft.ComponentDetection.Detectors.Maven; /// Represents a component with unresolved variables that needs second-pass processing. /// internal record PendingComponent( - string GroupId, - string ArtifactId, + string GroupIdTemplate, + string ArtifactIdTemplate, string VersionTemplate, ISingleFileComponentRecorder Recorder, string FilePath); diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/MvnCliDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/MvnCliDetectorTests.cs index 3847b9ba0..b36647e30 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/MvnCliDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/MvnCliDetectorTests.cs @@ -86,6 +86,184 @@ public async Task WhenMavenCliNotAvailable_FallsBackToStaticParsing_Async() mavenComponent.Version.Should().Be("3.12.0"); } + [TestMethod] + public async Task StaticParser_ResolvesPropertiesInAllCoordinateFields_Async() + { + this.mavenCommandServiceMock.Setup(x => x.MavenCLIExistsAsync()) + .ReturnsAsync(false); + + var pomXmlContent = @" + + 4.0.0 + com.test + my-app + 1.0.0 + + org.apache.spark + spark + ${artifact.prefix}-core + 2.12 + 3.4.1 + + + + ${spark.group} + ${artifact.name}_${scala.version.major} + ${spark.version} + + +"; + + var (detectorResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("pom.xml", pomXmlContent) + .ExecuteDetectorAsync(); + + detectorResult.ResultCode.Should().Be(ProcessingResultCode.Success); + + var detectedComponents = componentRecorder.GetDetectedComponents(); + detectedComponents.Should().ContainSingle(); + + var mavenComponent = detectedComponents.First().Component as MavenComponent; + mavenComponent.Should().NotBeNull(); + mavenComponent.GroupId.Should().Be("org.apache.spark"); + mavenComponent.ArtifactId.Should().Be("spark-core_2.12"); + mavenComponent.Version.Should().Be("3.4.1"); + } + + [TestMethod] + public async Task StaticParser_SkipsComponentWithUnresolvedCoordinateProperty_Async() + { + this.mavenCommandServiceMock.Setup(x => x.MavenCLIExistsAsync()) + .ReturnsAsync(false); + + var pomXmlContent = @" + + 4.0.0 + com.test + my-app + 1.0.0 + + + org.apache.spark + spark-core_${scala.version.major} + 3.4.1 + + +"; + + var (detectorResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("pom.xml", pomXmlContent) + .ExecuteDetectorAsync(); + + detectorResult.ResultCode.Should().Be(ProcessingResultCode.Success); + componentRecorder.GetDetectedComponents().Should().BeEmpty(); + } + + [TestMethod] + public async Task StaticParser_SkipsComponentsWithMalformedPropertyReferences_Async() + { + this.mavenCommandServiceMock.Setup(x => x.MavenCLIExistsAsync()) + .ReturnsAsync(false); + + var pomXmlContent = @" + + 4.0.0 + com.test + my-app + 1.0.0 + + + org.apache.spark + spark-core + ${} + + + org.apache.spark + spark-sql + ${missing + + +"; + + var (detectorResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("pom.xml", pomXmlContent) + .ExecuteDetectorAsync(); + + detectorResult.ResultCode.Should().Be(ProcessingResultCode.Success); + componentRecorder.GetDetectedComponents().Should().BeEmpty(); + } + + [TestMethod] + public async Task StaticParser_SkipsComponentWithCyclicCoordinateProperties_Async() + { + this.mavenCommandServiceMock.Setup(x => x.MavenCLIExistsAsync()) + .ReturnsAsync(false); + + var pomXmlContent = @" + + 4.0.0 + com.test + my-app + 1.0.0 + + ${artifact.alias} + ${artifact.name} + + + + org.apache.spark + ${artifact.name} + 3.4.1 + + +"; + + var (detectorResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("pom.xml", pomXmlContent) + .ExecuteDetectorAsync(); + + detectorResult.ResultCode.Should().Be(ProcessingResultCode.Success); + componentRecorder.GetDetectedComponents().Should().BeEmpty(); + } + + [TestMethod] + public async Task StaticParser_SkipsComponentExceedingPropertyExpansionDepth_Async() + { + this.mavenCommandServiceMock.Setup(x => x.MavenCLIExistsAsync()) + .ReturnsAsync(false); + + var nestedProperties = string.Join( + "\n", + Enumerable.Range(0, 101) + .Select(index => $" ${{property{index + 1}}}")); + + var pomXmlContent = $@" + + 4.0.0 + com.test + my-app + 1.0.0 + +{nestedProperties} + spark-core + + + + org.apache.spark + ${{property0}} + 3.4.1 + + +"; + + var (detectorResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("pom.xml", pomXmlContent) + .ExecuteDetectorAsync(); + + detectorResult.ResultCode.Should().Be(ProcessingResultCode.Success); + componentRecorder.GetDetectedComponents().Should().BeEmpty(); + } + [TestMethod] public async Task WhenMavenCliNotAvailable_DetectsMultipleDependencies_Async() { @@ -435,7 +613,7 @@ public async Task StaticParser_IgnoresDependenciesWithUnresolvablePropertyVersio } [TestMethod] - public async Task StaticParser_ResolvesVariableFromPreviousFile_Async() + public async Task StaticParser_ResolvesCoordinatePropertiesFromParent_Async() { // Arrange - Test case 1: Variable defined in parent POM, referenced in child POM // Uses Maven's standard parent inheritance mechanism @@ -455,6 +633,8 @@ public async Task StaticParser_ResolvesVariableFromPreviousFile_Async() pom 3.12.0 + 2.12 + ${scala.version.major} "; @@ -470,7 +650,7 @@ public async Task StaticParser_ResolvesVariableFromPreviousFile_Async() org.apache.commons - commons-lang3 + commons-lang3_${scala.binary.version} ${commons.version} @@ -491,7 +671,7 @@ public async Task StaticParser_ResolvesVariableFromPreviousFile_Async() var mavenComponent = detectedComponents.First().Component as MavenComponent; mavenComponent.Should().NotBeNull(); mavenComponent.GroupId.Should().Be("org.apache.commons"); - mavenComponent.ArtifactId.Should().Be("commons-lang3"); + mavenComponent.ArtifactId.Should().Be("commons-lang3_2.12"); mavenComponent.Version.Should().Be("3.12.0"); }