Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
52 changes: 52 additions & 0 deletions pkg/dependency/parser/java/jar/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ func (p *Parser) parsePackages(filePath string, size int64, r xio.ReadSeekerAt)

// Classify and attach the LICENSE file now that the jar's own artifact is resolved
// (it may have been added above from MANIFEST.MF / SHA-1 / file name).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment isn't related to the new attachManifestLicenses().

attachManifestLicenses(pkgs, fileProps.FilePath, m.licenses)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to add a comment about the priority.
Classifying a license from a file is an expensive operation, so we first try to detect licenses from the manifest file.

p.attachFileLicenses(pkgs, fileProps.FilePath, licenseFile)

return pkgs, nil, nil
Expand Down Expand Up @@ -327,6 +328,32 @@ func attachPomLicenses(pkgs []ftypes.Package, pomLicenses map[string][]string) {
}
}

// attachManifestLicenses attaches Jenkins plugin licenses declared in MANIFEST.MF
// to the jar's own package when a single unambiguous package belongs to this jar.
func attachManifestLicenses(pkgs []ftypes.Package, filePath string, licenses []string) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of duplicate code.
You can create a common function for attachFileLicenses() and attachManifestLicenses().
something like that:

 // ownJarPackage returns the single package that belongs to filePath and still
  // needs a license, or nil when there is none, the owner already has a license,
  // or the owner is ambiguous (multiple packages share the jar's file path, e.g.
  // an uber jar).
  func ownJarPackage(pkgs []ftypes.Package, filePath string) *ftypes.Package {
        var pkg *ftypes.Package
        for i := range pkgs {
                if pkgs[i].FilePath != filePath {
                        continue
                }
                if pkg != nil {
                        return nil // more than one package belongs to this jar
                }
                pkg = &pkgs[i]
        }
        if pkg == nil || len(pkg.Licenses) > 0 {
                return nil
        }
        return pkg
  }

if len(licenses) == 0 {
return
}

var pkg *ftypes.Package

for i := range pkgs {
if pkgs[i].FilePath != filePath {
continue
}
if pkg != nil {
return // more than one package belongs to this jar
}
pkg = &pkgs[i]
}

if pkg == nil || len(pkg.Licenses) > 0 {
return
}

pkg.Licenses = licenses
}

// attachFileLicenses classifies the LICENSE file packed in a jar and attaches it to the
// jar's own package, but only when the owner is unambiguous: a single LICENSE file, a
// single package belonging to this jar, and no license from its pom.xml yet.
Expand Down Expand Up @@ -583,6 +610,7 @@ type manifest struct {
bundleName string
bundleVersion string
bundleSymbolicName string
licenses []string
}

func parseManifest(f *zip.File) (manifest, error) {
Expand Down Expand Up @@ -626,6 +654,14 @@ func parseManifest(f *zip.File) (manifest, error) {
m.bundleName = strings.TrimPrefix(line, "Bundle-Name:")
case strings.HasPrefix(line, "Bundle-SymbolicName:"):
m.bundleSymbolicName = strings.TrimPrefix(line, "Bundle-SymbolicName:")
default:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a new case for Plugin-License-Name (don't use the default here).

key, value, ok := strings.Cut(line, ":")
if !ok || !isPluginLicenseNameKey(key) {
continue
}
if name := strings.TrimSpace(value); name != "" {
m.licenses = append(m.licenses, name)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd create a separate function for getting the license, and inside it do the Plugin-License-Name- check, trim the Plugin-License-Name* prefix, and return the licenses. If no licenses are found, return nil.

e.g. m.licenses = parseManifestLicenses(line)

}
}

Expand All @@ -635,6 +671,22 @@ func parseManifest(f *zip.File) (manifest, error) {
return m, nil
}

func isPluginLicenseNameKey(key string) bool {
if key == "Plugin-License-Name" {
return true
}
suffix, ok := strings.CutPrefix(key, "Plugin-License-Name-")
if !ok || suffix == "" {
return false
}
for _, r := range suffix {
if r < '0' || r > '9' {
return false
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's any chance that there won't be a number here. Let's simplify the check. And if users do report such a case, we'll add it in a separate PR.

}
return true
}

func (m manifest) properties(filePath string) Properties {
groupID, err := m.determineGroupID()
if err != nil {
Expand Down
98 changes: 98 additions & 0 deletions pkg/dependency/parser/java/jar/parse_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package jar_test

import (
"archive/zip"
"bytes"
"encoding/json"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -386,6 +388,102 @@ func TestParse(t *testing.T) {
}
}

func TestParseJenkinsPluginManifestLicenses(t *testing.T) {
tests := []struct {
name string
entries map[string]string
want []ftypes.Package
}{
{
name: "uses plugin license names from manifest",
entries: map[string]string{
"META-INF/MANIFEST.MF": strings.Join([]string{
"Manifest-Version: 1.0",
"Implementation-Vendor-Id: com.example",
"Implementation-Title: jenkins-plugin",
"Implementation-Version: 1.0.0",
"Plugin-License-Name: Apache License, Version 2.0",
"Plugin-License-Name-2: MIT License",
"",
}, "\n"),
},
want: []ftypes.Package{
{
Name: "com.example:jenkins-plugin",
Version: "1.0.0",
FilePath: "jenkins-plugin-1.0.0.jpi",
Licenses: []string{
"Apache License, Version 2.0",
"MIT License",
},
},
},
},
{
name: "keeps embedded pom license precedence",
entries: map[string]string{
"META-INF/MANIFEST.MF": strings.Join([]string{
"Manifest-Version: 1.0",
"Implementation-Vendor-Id: com.example",
"Implementation-Title: jenkins-plugin",
"Implementation-Version: 1.0.0",
"Plugin-License-Name: MIT License",
"",
}, "\n"),
"META-INF/maven/com.example/jenkins-plugin/pom.properties": strings.Join([]string{
"groupId=com.example",
"artifactId=jenkins-plugin",
"version=1.0.0",
"",
}, "\n"),
"META-INF/maven/com.example/jenkins-plugin/pom.xml": strings.Join([]string{
"<project>",
" <licenses>",
" <license><name>Apache-2.0</name></license>",
" </licenses>",
"</project>",
}, "\n"),
},
want: []ftypes.Package{
{
Name: "com.example:jenkins-plugin",
Version: "1.0.0",
FilePath: "jenkins-plugin-1.0.0.jpi",
Licenses: []string{"Apache-2.0"},
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := newZipReader(t, tt.entries)
p := jar.NewParser(nil, jar.WithFilePath("jenkins-plugin-1.0.0.jpi"), jar.WithOffline(true), jar.WithSize(int64(r.Len())))

got, _, err := p.Parse(t.Context(), r)
require.NoError(t, err)

assert.Equal(t, tt.want, got)
})
}
}

func newZipReader(t *testing.T, entries map[string]string) *bytes.Reader {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need a separate test that builds a jar file. Let's add one test case to TestParse. And also create a new test function for parseManifestLicenses() (use the export_test.go file).

t.Helper()

var buf bytes.Buffer
zw := zip.NewWriter(&buf)
for name, contents := range entries {
w, err := zw.Create(name)
require.NoError(t, err)
_, err = w.Write([]byte(contents))
require.NoError(t, err)
}
require.NoError(t, zw.Close())

return bytes.NewReader(buf.Bytes())
}

func TestEmbeddedPomGAV(t *testing.T) {
tests := []struct {
name string
Expand Down