From 3e71f46fc86aa492e861b9b01a8c9603de3e2159 Mon Sep 17 00:00:00 2001 From: William Murphy Date: Tue, 16 Apr 2024 15:44:02 -0400 Subject: [PATCH] Fix: repeatedly dereference pom variables (#2781) * Fix: repeatedly dereference pom variables Previously, if there was more than one layer of variable indirection in the pom property (propert A says it has the same value as property B, property B says it has the same value as property C), then Syft would only dereference one layer. Add a loop to dereference variables until either dereferencing fails, or until the variable is completely dereferenced back to a literal. Signed-off-by: Will Murphy * switch to recursive implementation Signed-off-by: Will Murphy * add test cases for degenerate poms Signed-off-by: Will Murphy * switch to recursive implementation Signed-off-by: Will Murphy * remove redundant pieces of test cases Signed-off-by: Will Murphy --------- Signed-off-by: Will Murphy --- syft/pkg/cataloger/java/parse_pom_xml.go | 16 +++- syft/pkg/cataloger/java/parse_pom_xml_test.go | 80 +++++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/syft/pkg/cataloger/java/parse_pom_xml.go b/syft/pkg/cataloger/java/parse_pom_xml.go index 64b8d614a5d..befe0537d9f 100644 --- a/syft/pkg/cataloger/java/parse_pom_xml.go +++ b/syft/pkg/cataloger/java/parse_pom_xml.go @@ -230,16 +230,26 @@ func cleanDescription(original *string) (cleaned string) { // resolveProperty emulates some maven property resolution logic by looking in the project's variables // as well as supporting the project expressions like ${project.parent.groupId}. // If no match is found, the entire expression including ${} is returned -// -//nolint:gocognit func resolveProperty(pom gopom.Project, property *string, propertyName string) string { propertyCase := safeString(property) log.WithFields("existingPropertyValue", propertyCase, "propertyName", propertyName).Trace("resolving property") + seenBeforePropertyNames := map[string]struct{}{ + propertyName: {}, + } + return recursiveResolveProperty(pom, propertyCase, seenBeforePropertyNames) +} + +//nolint:gocognit +func recursiveResolveProperty(pom gopom.Project, propertyCase string, seenPropertyNames map[string]struct{}) string { return propertyMatcher.ReplaceAllStringFunc(propertyCase, func(match string) string { propertyName := strings.TrimSpace(match[2 : len(match)-1]) // remove leading ${ and trailing } + if _, seen := seenPropertyNames[propertyName]; seen { + return propertyCase + } entries := pomProperties(pom) if value, ok := entries[propertyName]; ok { - return value + seenPropertyNames[propertyName] = struct{}{} + return recursiveResolveProperty(pom, value, seenPropertyNames) // recursively resolve in case a variable points to a variable. } // if we don't find anything directly in the pom properties, diff --git a/syft/pkg/cataloger/java/parse_pom_xml_test.go b/syft/pkg/cataloger/java/parse_pom_xml_test.go index c845233b125..450f33fcdff 100644 --- a/syft/pkg/cataloger/java/parse_pom_xml_test.go +++ b/syft/pkg/cataloger/java/parse_pom_xml_test.go @@ -514,6 +514,86 @@ func Test_resolveProperty(t *testing.T) { }, expected: "${project.parent.groupId}", }, + { + name: "double dereference", + property: "${springboot.version}", + pom: gopom.Project{ + Parent: &gopom.Parent{ + Version: stringPointer("1.2.3"), + }, + Properties: &gopom.Properties{ + Entries: map[string]string{ + "springboot.version": "${project.parent.version}", + }, + }, + }, + expected: "1.2.3", + }, + { + name: "map missing stops double dereference", + property: "${springboot.version}", + pom: gopom.Project{ + Parent: &gopom.Parent{ + Version: stringPointer("1.2.3"), + }, + }, + expected: "${springboot.version}", + }, + { + name: "resolution halts even if it resolves to a variable", + property: "${springboot.version}", + pom: gopom.Project{ + Parent: &gopom.Parent{ + Version: stringPointer("${undefined.version}"), + }, + Properties: &gopom.Properties{ + Entries: map[string]string{ + "springboot.version": "${project.parent.version}", + }, + }, + }, + expected: "${undefined.version}", + }, + { + name: "resolution halts even if cyclic", + property: "${springboot.version}", + pom: gopom.Project{ + Properties: &gopom.Properties{ + Entries: map[string]string{ + "springboot.version": "${springboot.version}", + }, + }, + }, + expected: "${springboot.version}", + }, + { + name: "resolution halts even if cyclic more steps", + property: "${cyclic.version}", + pom: gopom.Project{ + Properties: &gopom.Properties{ + Entries: map[string]string{ + "other.version": "${cyclic.version}", + "springboot.version": "${other.version}", + "cyclic.version": "${springboot.version}", + }, + }, + }, + expected: "${cyclic.version}", + }, + { + name: "resolution halts even if cyclic involving parent", + property: "${cyclic.version}", + pom: gopom.Project{ + Properties: &gopom.Properties{ + Entries: map[string]string{ + "other.version": "${cyclic.version}", + "springboot.version": "${other.version}", + "cyclic.version": "${springboot.version}", + }, + }, + }, + expected: "${cyclic.version}", + }, } for _, test := range tests {