From 676bdf98164216aeac5d0355a43bdac45873100d Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Tue, 20 Apr 2021 12:19:28 -0400 Subject: [PATCH] refactor pom properties to modify parent pkg less often (#392) Signed-off-by: Alex Goodman --- syft/pkg/cataloger/java/archive_parser.go | 140 ++++----- .../pkg/cataloger/java/archive_parser_test.go | 271 ++++++++++++++++++ 2 files changed, 346 insertions(+), 65 deletions(-) diff --git a/syft/pkg/cataloger/java/archive_parser.go b/syft/pkg/cataloger/java/archive_parser.go index 711ca0d1b..03d9d5060 100644 --- a/syft/pkg/cataloger/java/archive_parser.go +++ b/syft/pkg/cataloger/java/archive_parser.go @@ -98,7 +98,7 @@ func (j *archiveParser) parse() ([]pkg.Package, error) { } // find aux packages from pom.properties - auxPkgs, err := j.discoverPkgsFromPomProperties(parentPkg) + auxPkgs, err := j.discoverPkgsFromAllPomProperties(parentPkg) if err != nil { return nil, err } @@ -159,12 +159,9 @@ func (j *archiveParser) discoverMainPackage() (*pkg.Package, error) { }, nil } -// discoverPkgsFromPomProperties parses Maven POM properties for a given parent package, returning all listed Java packages found and -// associating each discovered package to the given parent package. -// nolint:funlen,gocognit -func (j *archiveParser) discoverPkgsFromPomProperties(parentPkg *pkg.Package) ([]pkg.Package, error) { +// discoverPkgsFromAllPomProperties parses Maven POM properties for a given parent package, returning all listed Java packages found for each pom properties discovered. +func (j *archiveParser) discoverPkgsFromAllPomProperties(parentPkg *pkg.Package) ([]pkg.Package, error) { var pkgs = make([]pkg.Package, 0) - parentKey := uniquePkgKey(parentPkg) // search and parse pom.properties files & fetch the contents contents, err := file.ContentsFromZip(j.archivePath, j.fileManifest.GlobMatch(pomPropertiesGlob)...) @@ -184,71 +181,84 @@ func (j *archiveParser) discoverPkgsFromPomProperties(parentPkg *pkg.Package) ([ continue } - if propsObj.Version != "" && propsObj.ArtifactID != "" { + if propsObj.Version == "" || propsObj.ArtifactID == "" { // TODO: if there is no parentPkg (no java manifest) one of these poms could be the parent. We should discover the right parent and attach the correct info accordingly to each discovered package - - // keep the artifact name within the virtual path if this package does not match the parent package - vPathSuffix := "" - if parentPkg != nil && !strings.HasPrefix(propsObj.ArtifactID, parentPkg.Name) { - vPathSuffix += ":" + propsObj.ArtifactID - } - virtualPath := j.virtualPath + vPathSuffix - - // discovered props = new package - p := pkg.Package{ - Name: propsObj.ArtifactID, - Version: propsObj.Version, - Language: pkg.Java, - Type: pkg.JavaPkg, - MetadataType: pkg.JavaMetadataType, - Metadata: pkg.JavaMetadata{ - VirtualPath: virtualPath, - PomProperties: propsObj, - Parent: parentPkg, - }, - } - - pkgKey := uniquePkgKey(&p) - - // the name/version pair matches... - matchesParentPkg := pkgKey == parentKey - - if parentPkg != nil { - // the virtual path matches... - matchesParentPkg = matchesParentPkg || parentPkg.Metadata.(pkg.JavaMetadata).VirtualPath == virtualPath - - // the pom artifactId has the parent name or vice versa - if propsObj.ArtifactID != "" { - matchesParentPkg = matchesParentPkg || strings.Contains(parentPkg.Name, propsObj.ArtifactID) || strings.Contains(propsObj.ArtifactID, parentPkg.Name) - } - - if matchesParentPkg { - // we've run across more information about our parent package, add this info to the parent package metadata - // the pom properties is typically a better source of information for name and version than the manifest - if p.Name != parentPkg.Name { - parentPkg.Name = p.Name - } - if p.Version != parentPkg.Version { - parentPkg.Version = p.Version - } - - parentMetadata, ok := parentPkg.Metadata.(pkg.JavaMetadata) - if ok { - parentMetadata.PomProperties = propsObj - parentPkg.Metadata = parentMetadata - } - } - } - - if !matchesParentPkg && !j.discoveredPkgs.Contains(pkgKey) { - // only keep packages we haven't seen yet (and are not related to the parent package) - pkgs = append(pkgs, p) - } + continue } + + if parentPkg == nil { + continue + } + + pkgs = append(pkgs, j.packagesFromPomProperties(propsObj, parentPkg)...) } return pkgs, nil } +// packagesFromPomProperties processes a single Maven POM properties for a given parent package, returning all listed Java packages found and +// associating each discovered package to the given parent package. +func (j *archiveParser) packagesFromPomProperties(propsObj *pkg.PomProperties, parentPkg *pkg.Package) (pkgs []pkg.Package) { + parentKey := uniquePkgKey(parentPkg) + + // keep the artifact name within the virtual path if this package does not match the parent package + vPathSuffix := "" + if !strings.HasPrefix(propsObj.ArtifactID, parentPkg.Name) { + vPathSuffix += ":" + propsObj.ArtifactID + } + virtualPath := j.virtualPath + vPathSuffix + + // discovered props = new package + p := pkg.Package{ + Name: propsObj.ArtifactID, + Version: propsObj.Version, + Language: pkg.Java, + Type: pkg.JavaPkg, + MetadataType: pkg.JavaMetadataType, + Metadata: pkg.JavaMetadata{ + VirtualPath: virtualPath, + PomProperties: propsObj, + Parent: parentPkg, + }, + } + + pkgKey := uniquePkgKey(&p) + + // the name/version pair matches... + matchesParentPkg := pkgKey == parentKey + + // the virtual path matches... + matchesParentPkg = matchesParentPkg || parentPkg.Metadata.(pkg.JavaMetadata).VirtualPath == virtualPath + + // the pom artifactId has the parent name or vice versa + if propsObj.ArtifactID != "" { + matchesParentPkg = matchesParentPkg || strings.Contains(parentPkg.Name, propsObj.ArtifactID) || strings.Contains(propsObj.ArtifactID, parentPkg.Name) + } + + if matchesParentPkg { + // we've run across more information about our parent package, add this info to the parent package metadata + // the pom properties is typically a better source of information for name and version than the manifest + if parentPkg.Name == "" { + parentPkg.Name = p.Name + } + if parentPkg.Version == "" { + parentPkg.Version = p.Version + } + + // keep the pom properties, but don't overwrite existing pom properties + parentMetadata, ok := parentPkg.Metadata.(pkg.JavaMetadata) + if ok && parentMetadata.PomProperties == nil { + parentMetadata.PomProperties = propsObj + parentPkg.Metadata = parentMetadata + } + } + + if !matchesParentPkg && !j.discoveredPkgs.Contains(pkgKey) { + // only keep packages we haven't seen yet (and are not related to the parent package) + pkgs = append(pkgs, p) + } + return pkgs +} + // discoverPkgsFromNestedArchives finds Java archives within Java archives, returning all listed Java packages found and // associating each discovered package to the given parent package. func (j *archiveParser) discoverPkgsFromNestedArchives(parentPkg *pkg.Package) ([]pkg.Package, error) { diff --git a/syft/pkg/cataloger/java/archive_parser_test.go b/syft/pkg/cataloger/java/archive_parser_test.go index 9b936e0a1..e044c4e8e 100644 --- a/syft/pkg/cataloger/java/archive_parser_test.go +++ b/syft/pkg/cataloger/java/archive_parser_test.go @@ -11,6 +11,8 @@ import ( "syscall" "testing" + "github.com/stretchr/testify/assert" + "github.com/anchore/syft/internal" "github.com/anchore/syft/syft/pkg" @@ -558,3 +560,272 @@ func TestParseNestedJar(t *testing.T) { }) } } + +func TestPackagesFromPomProperties(t *testing.T) { + virtualPath := "given/virtual/path" + tests := []struct { + name string + props *pkg.PomProperties + parent *pkg.Package + expectedParent pkg.Package + expectedPackages []pkg.Package + }{ + { + name: "go case: get a single package from pom properties", + props: &pkg.PomProperties{ + Name: "some-name", + GroupID: "some-group-id", + ArtifactID: "some-artifact-id", + Version: "1.0", + }, + parent: &pkg.Package{ + Name: "some-parent-name", + Version: "2.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: "some-parent-virtual-path", + Manifest: nil, + PomProperties: nil, + Parent: nil, + }, + }, + // note: the SAME as the original parent values + expectedParent: pkg.Package{ + Name: "some-parent-name", + Version: "2.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: "some-parent-virtual-path", + Manifest: nil, + PomProperties: nil, + Parent: nil, + }, + }, + expectedPackages: []pkg.Package{ + { + Name: "some-artifact-id", + Version: "1.0", + Language: pkg.Java, + Type: pkg.JavaPkg, + MetadataType: pkg.JavaMetadataType, + Metadata: pkg.JavaMetadata{ + VirtualPath: virtualPath + ":" + "some-artifact-id", + PomProperties: &pkg.PomProperties{ + Name: "some-name", + GroupID: "some-group-id", + ArtifactID: "some-artifact-id", + Version: "1.0", + }, + Parent: &pkg.Package{ + Name: "some-parent-name", + Version: "2.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: "some-parent-virtual-path", + Manifest: nil, + PomProperties: nil, + Parent: nil, + }, + }, + }, + }, + }, + }, + { + name: "child matches parent by key", + props: &pkg.PomProperties{ + Name: "some-name", + GroupID: "some-group-id", + ArtifactID: "some-parent-name", // note: matches parent package + Version: "2.0", // note: matches parent package + }, + parent: &pkg.Package{ + Name: "some-parent-name", + Version: "2.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: "some-parent-virtual-path", + Manifest: nil, + PomProperties: nil, + Parent: nil, + }, + }, + // note: the SAME as the original parent values + expectedParent: pkg.Package{ + Name: "some-parent-name", + Version: "2.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: "some-parent-virtual-path", + Manifest: nil, + // note: we attach the discovered pom properties data + PomProperties: &pkg.PomProperties{ + Name: "some-name", + GroupID: "some-group-id", + ArtifactID: "some-parent-name", // note: matches parent package + Version: "2.0", // note: matches parent package + }, + Parent: nil, + }, + }, + expectedPackages: nil, + }, + { + name: "child matches parent by virtual path", + props: &pkg.PomProperties{ + Name: "some-name", + GroupID: "some-group-id", + ArtifactID: "some-parent-name", // note: matches parent package + Version: "NOT_THE_PARENT_VERSION", // note: DOES NOT match parent package + }, + parent: &pkg.Package{ + Name: "some-parent-name", + Version: "2.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: virtualPath + ":some-parent-name", // note: matching virtual path + Manifest: nil, + PomProperties: nil, + Parent: nil, + }, + }, + expectedParent: pkg.Package{ + Name: "some-parent-name", + Version: "2.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: virtualPath + ":some-parent-name", // note: matching virtual path + Manifest: nil, + // note: we attach the discovered pom properties data + PomProperties: &pkg.PomProperties{ + Name: "some-name", + GroupID: "some-group-id", + ArtifactID: "some-parent-name", // note: matches parent package + Version: "NOT_THE_PARENT_VERSION", // note: DOES NOT match parent package + }, + Parent: nil, + }, + }, + expectedPackages: nil, + }, + { + name: "child matches parent by virtual path -- override name and version", + props: &pkg.PomProperties{ + Name: "some-name", + GroupID: "some-group-id", + ArtifactID: "some-parent-name", // note: DOES NOT match parent package + Version: "3.0", // note: DOES NOT match parent package + }, + parent: &pkg.Package{ + Name: "", // note: empty + Version: "", // note: empty + Metadata: pkg.JavaMetadata{ + VirtualPath: virtualPath + ":some-parent-name", // note: matching virtual path + Manifest: nil, + PomProperties: nil, + Parent: nil, + }, + }, + expectedParent: pkg.Package{ + Name: "some-parent-name", + Version: "3.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: virtualPath + ":some-parent-name", // note: matching virtual path + Manifest: nil, + // note: we attach the discovered pom properties data + PomProperties: &pkg.PomProperties{ + Name: "some-name", + GroupID: "some-group-id", + ArtifactID: "some-parent-name", // note: DOES NOT match parent package + Version: "3.0", // note: DOES NOT match parent package + }, + Parent: nil, + }, + }, + expectedPackages: nil, + }, + { + name: "child matches parent by virtual path -- do not override existing pom properties", + props: &pkg.PomProperties{ + Name: "some-name", + GroupID: "some-group-id", + ArtifactID: "some-parent-name", // note: matches parent package + Version: "NOT_THE_PARENT_VERSION", // note: DOES NOT match parent package + }, + parent: &pkg.Package{ + Name: "some-parent-name", + Version: "2.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: virtualPath + ":some-parent-name", // note: matching virtual path + Manifest: nil, + PomProperties: &pkg.PomProperties{ + Name: "EXISTS", //note: this already exists and should not be overridden + }, + Parent: nil, + }, + }, + expectedParent: pkg.Package{ + Name: "some-parent-name", + Version: "2.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: virtualPath + ":some-parent-name", // note: matching virtual path + Manifest: nil, + // note: we attach the discovered pom properties data + PomProperties: &pkg.PomProperties{ + Name: "EXISTS", //note: this already exists and should not be overridden + }, + Parent: nil, + }, + }, + expectedPackages: nil, + }, + { + name: "child matches parent by artifact id", + props: &pkg.PomProperties{ + Name: "some-name", + GroupID: "some-group-id", + ArtifactID: "some-parent-name", // note: matches parent package + Version: "NOT_THE_PARENT_VERSION", // note: DOES NOT match parent package + }, + parent: &pkg.Package{ + Name: "some-parent-name", + Version: "2.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: virtualPath + ":NEW_VIRTUAL_PATH", // note: DOES NOT match the existing virtual path + Manifest: nil, + PomProperties: nil, + Parent: nil, + }, + }, + // note: the SAME as the original parent values + expectedParent: pkg.Package{ + Name: "some-parent-name", + Version: "2.0", + Metadata: pkg.JavaMetadata{ + VirtualPath: virtualPath + ":NEW_VIRTUAL_PATH", + Manifest: nil, + // note: we attach the discovered pom properties data + PomProperties: &pkg.PomProperties{ + Name: "some-name", + GroupID: "some-group-id", + ArtifactID: "some-parent-name", // note: matches parent package + Version: "NOT_THE_PARENT_VERSION", // note: DOES NOT match parent package + }, + Parent: nil, + }, + }, + expectedPackages: nil, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // note: this zip doesn't matter, as long as it is a zip + nop, err := os.Open("test-fixtures/java-builds/packages/spring-boot-0.0.1-SNAPSHOT.jar") + assert.NoError(t, err) + + // make the parser + parser, cleanup, err := newJavaArchiveParser(virtualPath, nop, false) + assert.NoError(t, err) + t.Cleanup(cleanup) + + // get the test data + actualPackages := parser.packagesFromPomProperties(test.props, test.parent) + assert.Equal(t, test.expectedPackages, actualPackages) + assert.Equal(t, test.expectedParent, *test.parent) + }) + } +}