Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix maven recursion issue with parent properties specified in relative path parent pom #3170

Merged
merged 2 commits into from
Aug 28, 2024
Merged

Fix maven recursion issue with parent properties specified in relative path parent pom #3170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
374ba61
fix: cycles resolving parent
kzantow Aug 27, 2024
c89cbef
Merge remote-tracking branch 'upstream/main' into fix/maven-recursion
kzantow Aug 27, 2024
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
4 changes: 2 additions & 2 deletions syft/pkg/cataloger/java/archive_parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func (j *archiveParser) findLicenseFromJavaMetadata(ctx context.Context, groupID
if parsedPom != nil {
pomLicenses, err = j.maven.resolveLicenses(ctx, parsedPom.project)
if err != nil {
log.WithFields("error", err, "mavenID", j.maven.resolveMavenID(ctx, parsedPom.project)).Debug("error attempting to resolve pom licenses")
log.WithFields("error", err, "mavenID", j.maven.getMavenID(ctx, parsedPom.project)).Debug("error attempting to resolve pom licenses")
}
}

Expand Down Expand Up @@ -352,7 +352,7 @@ func (j *archiveParser) discoverMainPackageFromPomInfo(ctx context.Context) (gro
version = pomProperties.Version

if parsedPom != nil && parsedPom.project != nil {
id := j.maven.resolveMavenID(ctx, parsedPom.project)
id := j.maven.getMavenID(ctx, parsedPom.project)
if group == "" {
group = id.GroupID
}
Expand Down
99 changes: 61 additions & 38 deletions syft/pkg/cataloger/java/maven_resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,17 @@ func newMavenResolver(fileResolver file.Resolver, cfg ArchiveCatalogerConfig) *m
// as well as supporting the project expressions like ${project.parent.groupId}.
// Properties which are not resolved result in empty string ""
func (r *mavenResolver) getPropertyValue(ctx context.Context, propertyValue *string, resolutionContext ...*gopom.Project) string {
return r.resolvePropertyValue(ctx, propertyValue, nil, resolutionContext...)
}

// resolvePropertyValue resolves property values by emulating maven property resolution logic, looking in the project's variables
// as well as supporting the project expressions like ${project.parent.groupId}.
// Properties which are not resolved result in empty string ""
func (r *mavenResolver) resolvePropertyValue(ctx context.Context, propertyValue *string, resolvingProperties []string, resolutionContext ...*gopom.Project) string {
if propertyValue == nil {
return ""
}
resolved, err := r.resolveExpression(ctx, resolutionContext, *propertyValue, nil)
resolved, err := r.resolveExpression(ctx, resolutionContext, *propertyValue, resolvingProperties)
if err != nil {
log.WithFields("error", err, "propertyValue", *propertyValue).Debug("error resolving maven property")
return ""
Expand All @@ -79,32 +86,35 @@ func (r *mavenResolver) getPropertyValue(ctx context.Context, propertyValue *str
}

// resolveExpression resolves an expression, which may be a plain string or a string with ${ property.references }
func (r *mavenResolver) resolveExpression(ctx context.Context, resolutionContext []*gopom.Project, expression string, resolving []string) (string, error) {
var err error
func (r *mavenResolver) resolveExpression(ctx context.Context, resolutionContext []*gopom.Project, expression string, resolvingProperties []string) (string, error) {
log.Tracef("resolving expression: '%v' in context: %v", expression, resolutionContext)

var errs error
return expressionMatcher.ReplaceAllStringFunc(expression, func(match string) string {
log.Tracef("resolving property: '%v' in context: %v", expression, resolutionContext)
propertyExpression := strings.TrimSpace(match[2 : len(match)-1]) // remove leading ${ and trailing }
resolved, e := r.resolveProperty(ctx, resolutionContext, propertyExpression, resolving)
if e != nil {
err = errors.Join(err, e)
resolved, err := r.resolveProperty(ctx, resolutionContext, propertyExpression, resolvingProperties)
if err != nil {
errs = errors.Join(errs, err)
return ""
}
return resolved
}), err
}), errs
}

// resolveProperty resolves properties recursively from the root project
func (r *mavenResolver) resolveProperty(ctx context.Context, resolutionContext []*gopom.Project, propertyExpression string, resolving []string) (string, error) {
func (r *mavenResolver) resolveProperty(ctx context.Context, resolutionContext []*gopom.Project, propertyExpression string, resolvingProperties []string) (string, error) {
// prevent cycles
if slices.Contains(resolving, propertyExpression) {
if slices.Contains(resolvingProperties, propertyExpression) {
return "", fmt.Errorf("cycle detected resolving: %s", propertyExpression)
}
if len(resolutionContext) == 0 {
return "", fmt.Errorf("no project variable resolution context provided for expression: '%s'", propertyExpression)
}
resolving = append(resolving, propertyExpression)
resolvingProperties = append(resolvingProperties, propertyExpression)

// only resolve project. properties in the context of the current project pom
value, err := r.resolveProjectProperty(ctx, resolutionContext, resolutionContext[len(resolutionContext)-1], propertyExpression, resolving)
value, err := r.resolveProjectProperty(ctx, resolutionContext, resolutionContext[len(resolutionContext)-1], propertyExpression, resolvingProperties)
if err != nil {
return value, err
}
Expand All @@ -120,10 +130,10 @@ func (r *mavenResolver) resolveProperty(ctx context.Context, resolutionContext [
}
if current.Properties != nil && current.Properties.Entries != nil {
if value, ok := current.Properties.Entries[propertyExpression]; ok {
return r.resolveExpression(ctx, resolutionContext, value, resolving) // property values can contain expressions
return r.resolveExpression(ctx, resolutionContext, value, resolvingProperties) // property values can contain expressions
}
}
current, err = r.resolveParent(ctx, current)
current, err = r.resolveParent(ctx, current, resolvingProperties...)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -200,23 +210,33 @@ func (r *mavenResolver) resolveProjectProperty(ctx context.Context, resolutionCo
return "", nil
}

// getMavenID creates a new mavenID from a pom, resolving parent information as necessary
func (r *mavenResolver) getMavenID(ctx context.Context, resolutionContext ...*gopom.Project) mavenID {
return r.resolveMavenID(ctx, nil, resolutionContext...)
}

// resolveMavenID creates a new mavenID from a pom, resolving parent information as necessary
func (r *mavenResolver) resolveMavenID(ctx context.Context, pom *gopom.Project) mavenID {
func (r *mavenResolver) resolveMavenID(ctx context.Context, resolvingProperties []string, resolutionContext ...*gopom.Project) mavenID {
if len(resolutionContext) == 0 || resolutionContext[0] == nil {
return mavenID{}
}
pom := resolutionContext[len(resolutionContext)-1] // get topmost pom
if pom == nil {
return mavenID{}
}
groupID := r.getPropertyValue(ctx, pom.GroupID, pom)
artifactID := r.getPropertyValue(ctx, pom.ArtifactID, pom)
version := r.getPropertyValue(ctx, pom.Version, pom)

groupID := r.resolvePropertyValue(ctx, pom.GroupID, resolvingProperties, resolutionContext...)
artifactID := r.resolvePropertyValue(ctx, pom.ArtifactID, resolvingProperties, resolutionContext...)
version := r.resolvePropertyValue(ctx, pom.Version, resolvingProperties, resolutionContext...)
if pom.Parent != nil {
if groupID == "" {
groupID = r.getPropertyValue(ctx, pom.Parent.GroupID, pom)
groupID = r.resolvePropertyValue(ctx, pom.Parent.GroupID, resolvingProperties, resolutionContext...)
}
if artifactID == "" {
artifactID = r.getPropertyValue(ctx, pom.Parent.ArtifactID, pom)
artifactID = r.resolvePropertyValue(ctx, pom.Parent.ArtifactID, resolvingProperties, resolutionContext...)
}
if version == "" {
version = r.getPropertyValue(ctx, pom.Parent.Version, pom)
version = r.resolvePropertyValue(ctx, pom.Parent.Version, resolvingProperties, resolutionContext...)
}
}
return mavenID{groupID, artifactID, version}
Expand All @@ -240,7 +260,7 @@ func (r *mavenResolver) resolveDependencyID(ctx context.Context, pom *gopom.Proj
depID := mavenID{groupID, artifactID, version}

if err != nil {
log.WithFields("error", err, "mavenID", r.resolveMavenID(ctx, pom), "dependencyID", depID)
log.WithFields("error", err, "mavenID", r.getMavenID(ctx, pom), "dependencyID", depID)
}

return depID
Expand Down Expand Up @@ -390,16 +410,16 @@ func (r *mavenResolver) cacheResolveReader(key string, resolve func() (io.ReadCl
}

// resolveParent attempts to resolve the parent for the given pom
func (r *mavenResolver) resolveParent(ctx context.Context, pom *gopom.Project) (*gopom.Project, error) {
func (r *mavenResolver) resolveParent(ctx context.Context, pom *gopom.Project, resolvingProperties ...string) (*gopom.Project, error) {
if pom == nil || pom.Parent == nil {
return nil, nil
}
parent := pom.Parent
pomWithoutParent := *pom
pomWithoutParent.Parent = nil
groupID := r.getPropertyValue(ctx, parent.GroupID, &pomWithoutParent)
artifactID := r.getPropertyValue(ctx, parent.ArtifactID, &pomWithoutParent)
version := r.getPropertyValue(ctx, parent.Version, &pomWithoutParent)
groupID := r.resolvePropertyValue(ctx, parent.GroupID, resolvingProperties, &pomWithoutParent)
artifactID := r.resolvePropertyValue(ctx, parent.ArtifactID, resolvingProperties, &pomWithoutParent)
version := r.resolvePropertyValue(ctx, parent.Version, resolvingProperties, &pomWithoutParent)

// check cache before resolving
parentID := mavenID{groupID, artifactID, version}
Expand All @@ -408,7 +428,7 @@ func (r *mavenResolver) resolveParent(ctx context.Context, pom *gopom.Project) (
}

// check if the pom exists in the fileResolver
parentPom := r.findParentPomByRelativePath(ctx, pom, parentID)
parentPom := r.findParentPomByRelativePath(ctx, pom, parentID, resolvingProperties)
if parentPom != nil {
return parentPom, nil
}
Expand All @@ -425,10 +445,10 @@ func (r *mavenResolver) findInheritedVersion(ctx context.Context, pom *gopom.Pro
return "", fmt.Errorf("nil pom provided to findInheritedVersion")
}
if r.cfg.MaxParentRecursiveDepth > 0 && len(resolutionContext) > r.cfg.MaxParentRecursiveDepth {
return "", fmt.Errorf("maximum depth reached attempting to resolve version for: %s:%s at: %v", groupID, artifactID, r.resolveMavenID(ctx, pom))
return "", fmt.Errorf("maximum depth reached attempting to resolve version for: %s:%s at: %v", groupID, artifactID, r.getMavenID(ctx, pom))
}
if slices.Contains(resolutionContext, pom) {
return "", fmt.Errorf("cycle detected attempting to resolve version for: %s:%s at: %v", groupID, artifactID, r.resolveMavenID(ctx, pom))
return "", fmt.Errorf("cycle detected attempting to resolve version for: %s:%s at: %v", groupID, artifactID, r.getMavenID(ctx, pom))
}
resolutionContext = append(resolutionContext, pom)

Expand All @@ -452,13 +472,13 @@ func (r *mavenResolver) findInheritedVersion(ctx context.Context, pom *gopom.Pro

depPom, err := r.findPom(ctx, depGroupID, depArtifactID, depVersion)
if err != nil || depPom == nil {
log.WithFields("error", err, "mavenID", r.resolveMavenID(ctx, pom), "dependencyID", mavenID{depGroupID, depArtifactID, depVersion}).
log.WithFields("error", err, "mavenID", r.getMavenID(ctx, pom), "dependencyID", mavenID{depGroupID, depArtifactID, depVersion}).
Debug("unable to find imported pom looking for managed dependencies")
continue
}
version, err = r.findInheritedVersion(ctx, depPom, groupID, artifactID, resolutionContext...)
if err != nil {
log.WithFields("error", err, "mavenID", r.resolveMavenID(ctx, pom), "dependencyID", mavenID{depGroupID, depArtifactID, depVersion}).
log.WithFields("error", err, "mavenID", r.getMavenID(ctx, pom), "dependencyID", mavenID{depGroupID, depArtifactID, depVersion}).
Debug("error during findInheritedVersion")
}
if version != "" {
Expand Down Expand Up @@ -508,7 +528,7 @@ func (r *mavenResolver) findLicenses(ctx context.Context, groupID, artifactID, v

// resolveLicenses searches the pom for license, traversing parent poms if needed
func (r *mavenResolver) resolveLicenses(ctx context.Context, pom *gopom.Project, processing ...mavenID) ([]gopom.License, error) {
id := r.resolveMavenID(ctx, pom)
id := r.getMavenID(ctx, pom)
if slices.Contains(processing, id) {
return nil, fmt.Errorf("cycle detected resolving licenses for: %v", id)
}
Expand Down Expand Up @@ -545,7 +565,7 @@ func (r *mavenResolver) pomLicenses(ctx context.Context, pom *gopom.Project) []g
return out
}

func (r *mavenResolver) findParentPomByRelativePath(ctx context.Context, pom *gopom.Project, parentID mavenID) *gopom.Project {
func (r *mavenResolver) findParentPomByRelativePath(ctx context.Context, pom *gopom.Project, parentID mavenID, resolvingProperties []string) *gopom.Project {
// don't resolve if no resolver
if r.fileResolver == nil {
return nil
Expand All @@ -555,39 +575,42 @@ func (r *mavenResolver) findParentPomByRelativePath(ctx context.Context, pom *go
if !hasPomLocation || pom == nil || pom.Parent == nil {
return nil
}
relativePath := r.getPropertyValue(ctx, pom.Parent.RelativePath, pom)
relativePath := r.resolvePropertyValue(ctx, pom.Parent.RelativePath, resolvingProperties, pom)
if relativePath == "" {
return nil
}
p := pomLocation.Path()
p = path.Dir(p)
p = path.Join(p, relativePath)
p = path.Clean(p)
if !strings.HasSuffix(p, ".xml") {
p = path.Join(p, "pom.xml")
}
parentLocations, err := r.fileResolver.FilesByPath(p)
if err != nil || len(parentLocations) == 0 {
log.WithFields("error", err, "mavenID", r.resolveMavenID(ctx, pom), "parentID", parentID, "relativePath", relativePath).
log.WithFields("error", err, "mavenID", r.resolveMavenID(ctx, resolvingProperties, pom), "parentID", parentID, "relativePath", relativePath).
Trace("parent pom not found by relative path")
return nil
}
parentLocation := parentLocations[0]

parentContents, err := r.fileResolver.FileContentsByLocation(parentLocation)
if err != nil || parentContents == nil {
log.WithFields("error", err, "mavenID", r.resolveMavenID(ctx, pom), "parentID", parentID, "parentLocation", parentLocation).
log.WithFields("error", err, "mavenID", r.resolveMavenID(ctx, resolvingProperties, pom), "parentID", parentID, "parentLocation", parentLocation).
Debug("unable to get contents of parent pom by relative path")
return nil
}
defer internal.CloseAndLogError(parentContents, parentLocation.RealPath)
parentPom, err := decodePomXML(parentContents)
if err != nil || parentPom == nil {
log.WithFields("error", err, "mavenID", r.resolveMavenID(ctx, pom), "parentID", parentID, "parentLocation", parentLocation).
log.WithFields("error", err, "mavenID", r.resolveMavenID(ctx, resolvingProperties, pom), "parentID", parentID, "parentLocation", parentLocation).
Debug("unable to parse parent pom")
return nil
}
// ensure parent matches
newParentID := r.resolveMavenID(ctx, parentPom)
newParentID := r.resolveMavenID(ctx, resolvingProperties, parentPom)
if newParentID.ArtifactID != parentID.ArtifactID {
log.WithFields("newParentID", newParentID, "mavenID", r.resolveMavenID(ctx, pom), "parentID", parentID, "parentLocation", parentLocation).
log.WithFields("newParentID", newParentID, "mavenID", r.resolveMavenID(ctx, resolvingProperties, pom), "parentID", parentID, "parentLocation", parentLocation).
Debug("parent IDs do not match resolving parent by relative path")
return nil
}
Expand Down
80 changes: 60 additions & 20 deletions syft/pkg/cataloger/java/maven_resolver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,32 +273,72 @@ func Test_relativePathParent(t *testing.T) {
resolver, err := fileresolver.NewFromDirectory("test-fixtures/pom/local", "")
require.NoError(t, err)

r := newMavenResolver(resolver, DefaultArchiveCatalogerConfig())
locs, err := resolver.FilesByPath("child-1/pom.xml")
require.NoError(t, err)
require.Len(t, locs, 1)
ctx := context.Background()

loc := locs[0]
contents, err := resolver.FileContentsByLocation(loc)
require.NoError(t, err)
defer internal.CloseAndLogError(contents, loc.RealPath)
tests := []struct {
name string
pom string
validate func(t *testing.T, r *mavenResolver, pom *gopom.Project)
}{
{
name: "basic",
pom: "child-1/pom.xml",
validate: func(t *testing.T, r *mavenResolver, pom *gopom.Project) {
parent, err := r.resolveParent(ctx, pom)
require.NoError(t, err)
require.Contains(t, r.pomLocations, parent)

pom, err := decodePomXML(contents)
require.NoError(t, err)
parent, err = r.resolveParent(ctx, parent)
require.NoError(t, err)
require.Contains(t, r.pomLocations, parent)

r.pomLocations[pom] = loc
got := r.getPropertyValue(ctx, ptr("${commons-exec_subversion}"), pom)
require.Equal(t, "3", got)

ctx := context.Background()
parent, err := r.resolveParent(ctx, pom)
require.NoError(t, err)
require.Contains(t, r.pomLocations, parent)
},
},
{
name: "parent property",
pom: "child-2/pom.xml",
validate: func(t *testing.T, r *mavenResolver, pom *gopom.Project) {
id := r.getMavenID(ctx, pom)
// child.parent.version = ${revision}
// parent.revision = 3.3.3
require.Equal(t, id.Version, "3.3.3")
},
},
{
name: "invalid parent",
pom: "child-3/pom.xml",
validate: func(t *testing.T, r *mavenResolver, pom *gopom.Project) {
require.NotNil(t, pom)
id := r.getMavenID(ctx, pom)
// version should not be resolved to anything
require.Equal(t, "", id.Version)
},
},
}

parent, err = r.resolveParent(ctx, parent)
require.NoError(t, err)
require.Contains(t, r.pomLocations, parent)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
r := newMavenResolver(resolver, DefaultArchiveCatalogerConfig())
locs, err := resolver.FilesByPath(test.pom)
require.NoError(t, err)
require.Len(t, locs, 1)

loc := locs[0]
contents, err := resolver.FileContentsByLocation(loc)
require.NoError(t, err)
defer internal.CloseAndLogError(contents, loc.RealPath)

pom, err := decodePomXML(contents)
require.NoError(t, err)

got := r.getPropertyValue(ctx, ptr("${commons-exec_subversion}"), pom)
require.Equal(t, "3", got)
r.pomLocations[pom] = loc

test.validate(t, r, pom)
})
}
}

// mockMavenRepo starts a remote maven repo serving all the pom files found in test-fixtures/pom/maven-repo
Expand Down
6 changes: 3 additions & 3 deletions syft/pkg/cataloger/java/parse_pom_xml.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (p pomXMLCataloger) Catalog(ctx context.Context, fileResolver file.Resolver

// store information about this pom for future lookups
r.pomLocations[pom] = pomLocation
r.resolved[r.resolveMavenID(ctx, pom)] = pom
r.resolved[r.getMavenID(ctx, pom)] = pom
}

var pkgs []pkg.Package
Expand All @@ -75,7 +75,7 @@ func readPomFromLocation(fileResolver file.Resolver, pomLocation file.Location)
func processPomXML(ctx context.Context, r *mavenResolver, pom *gopom.Project, loc file.Location) []pkg.Package {
var pkgs []pkg.Package

pomID := r.resolveMavenID(ctx, pom)
pomID := r.getMavenID(ctx, pom)
for _, dep := range pomDependencies(pom) {
depID := r.resolveDependencyID(ctx, pom, dep)
log.WithFields("pomLocation", loc, "mavenID", pomID, "dependencyID", depID).Trace("adding maven pom dependency")
Expand All @@ -100,7 +100,7 @@ func processPomXML(ctx context.Context, r *mavenResolver, pom *gopom.Project, lo
}

func newPomProject(ctx context.Context, r *mavenResolver, path string, pom *gopom.Project) *pkg.JavaPomProject {
id := r.resolveMavenID(ctx, pom)
id := r.getMavenID(ctx, pom)
name := r.getPropertyValue(ctx, pom.Name, pom)
projectURL := r.getPropertyValue(ctx, pom.URL, pom)

Expand Down
Loading
Loading