Skip to content

Commit

Permalink
feat(updater): use version comparison workarounds in Maven updater (#…
Browse files Browse the repository at this point in the history 
…1647)

This PR applies the Maven version comparison workarounds in the updater.
To make the function reusable, moves the version comparison to
`internal/utility/maven`.

Also considering that all version strings are parsed to semver first for
comparison, this PR pre-parse the versions and store them in a map.
  • Loading branch information
@cuixq
cuixq authored Feb 20, 2025
1 parent 110ec91 commit 5058d47
Show file tree
Hide file tree
Showing 5 changed files with 298 additions and 69 deletions.
77 changes: 18 additions & 59 deletions internal/remediation/override.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import (
"context"
"errors"
"fmt"
"log"
"slices"
"strings"

"deps.dev/util/resolve"
"deps.dev/util/resolve/dep"
"deps.dev/util/semver"
"github.com/google/osv-scanner/v2/internal/remediation/upgrade"
"github.com/google/osv-scanner/v2/internal/resolution"
"github.com/google/osv-scanner/v2/internal/resolution/client"
Expand Down Expand Up @@ -256,8 +257,23 @@ func getVersionsGreater(ctx context.Context, cl client.DependencyClient, vk reso
if err != nil {
return nil, err
}
semvers := make(map[resolve.VersionKey]*semver.Version)
for _, ver := range versions {
parsed, err := semver.Maven.Parse(ver.Version)
if err != nil {
log.Printf("parsing Maven version %s: %v", parsed, err)
continue
}
semvers[ver.VersionKey] = parsed
}

cmpFunc := func(a, b resolve.Version) int {
if vk.System == resolve.Maven {
return maven.CompareVersions(vk, semvers[a.VersionKey], semvers[b.VersionKey])
}

cmpFunc := comparisonFunctionWithWorkarounds(vk)
return vk.Semver().Compare(a.Version, b.Version)
}
slices.SortFunc(versions, cmpFunc)
// Find the index of the next higher version
offset, vkFound := slices.BinarySearchFunc(versions, resolve.Version{VersionKey: vk}, cmpFunc)
Expand All @@ -268,63 +284,6 @@ func getVersionsGreater(ctx context.Context, cl client.DependencyClient, vk reso
return versions[offset:], nil
}

// comparisonFunctionWithWorkarounds returns a version comparison function with special behaviour for specific packages,
// producing more desirable ordering using non-standard comparison.
// TODO: Move this and make it re-usable for other remediation strategies & osv-scanner update.
func comparisonFunctionWithWorkarounds(vk resolve.VersionKey) func(resolve.Version, resolve.Version) int {
sys := vk.Semver()

if vk.System == resolve.Maven && vk.Name == "com.google.guava:guava" {
// com.google.guava:guava has 'flavors' with versions ending with -jre or -android.
// https://github.com/google/guava/wiki/ReleasePolicy#flavors
// To preserve the flavor in updates, we make the opposite flavor considered the earliest versions.

// Old versions have '22.0' and '22.0-android', and even older version don't have any flavors.
// Only check for the android flavor, and assume its jre otherwise.
wantAndroid := strings.HasSuffix(vk.Version, "-android")
return func(a, b resolve.Version) int {
aIsAndroid := strings.HasSuffix(a.Version, "-android")
bIsAndroid := strings.HasSuffix(b.Version, "-android")

if aIsAndroid == bIsAndroid {
return sys.Compare(a.Version, b.Version)
}

if aIsAndroid == wantAndroid {
return 1
}

return -1
}
}

if vk.System == resolve.Maven && strings.HasPrefix(vk.Name, "commons-") {
// Old versions of apache commons-* libraries (commons-io:commons-io, commons-math:commons-math, etc.)
// used date-based versions (e.g. 20040118.003354), which naturally sort after the more recent semver versions.
// We manually force the date versions to come before the others to prevent downgrades.
return func(a, b resolve.Version) int {
// All date-based versions of these packages seem to be in the years 2002-2005.
// It's extremely unlikely we'd see any versions dated before 1999 or after 2010.
// It's also unlikely we'd see any major versions of these packages reach up to 200.0.0.
// Checking if the version starts with "200" should therefore be sufficient to determine if it's a year.
aCal := strings.HasPrefix(a.Version, "200")
bCal := strings.HasPrefix(b.Version, "200")

if aCal == bCal {
return sys.Compare(a.Version, b.Version)
}

if aCal {
return -1
}

return 1
}
}

return func(a, b resolve.Version) int { return sys.Compare(a.Version, b.Version) }
}

// patchManifest applies the overridePatches to the manifest in-memory. Returns a copy of the manifest that has been patched.
func patchManifest(patches []overridePatch, m manifest.Manifest) (manifest.Manifest, error) {
if m.System() != resolve.Maven {
Expand Down
12 changes: 6 additions & 6 deletions internal/remediation/suggest/maven.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"deps.dev/util/semver"
"github.com/google/osv-scanner/v2/internal/remediation/upgrade"
"github.com/google/osv-scanner/v2/internal/resolution/manifest"
"github.com/google/osv-scanner/v2/internal/utility/maven"
"golang.org/x/exp/slices"
)

Expand Down Expand Up @@ -78,12 +79,12 @@ func suggestMavenVersion(ctx context.Context, cl resolve.Client, req resolve.Req
}
semvers := make([]*semver.Version, 0, len(versions))
for _, ver := range versions {
v, err := semver.Maven.Parse(ver.Version)
parsed, err := semver.Maven.Parse(ver.Version)
if err != nil {
log.Printf("parsing Maven version %s: %v", v, err)
log.Printf("parsing Maven version %s: %v", parsed, err)
continue
}
semvers = append(semvers, v)
semvers = append(semvers, parsed)
}

constraint, err := semver.Maven.ParseConstraint(req.Version)
Expand All @@ -109,12 +110,11 @@ func suggestMavenVersion(ctx context.Context, cl resolve.Client, req resolve.Req

var newReq *semver.Version
for _, v := range semvers {
if v.Compare(newReq) < 0 {
if maven.CompareVersions(req.VersionKey, v, newReq) < 0 {
// Skip versions smaller than the current requirement
continue
}
_, diff := v.Difference(current)
if !level.Allows(diff) {
if _, diff := v.Difference(current); !level.Allows(diff) {
continue
}
newReq = v
Expand Down
121 changes: 118 additions & 3 deletions internal/remediation/suggest/maven_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,10 +396,10 @@ func Test_suggestMavenVersion(t *testing.T) {
{"1.0.0", upgrade.Patch, "1.0.1"},
// Version range requirement is not outdated
{"[1.0.0,)", upgrade.Major, "[1.0.0,)"},
{"[2.0.0, 2.3.4]", upgrade.Major, "[2.0.0, 2.3.4]"},
{"[2.0.0,2.3.4]", upgrade.Major, "[2.0.0,2.3.4]"},
// Version range requirement is outdated
{"[2.0.0, 2.3.4)", upgrade.Major, "2.3.4"},
{"[2.0.0, 2.2.2]", upgrade.Major, "2.3.4"},
{"[2.0.0,2.3.4)", upgrade.Major, "2.3.4"},
{"[2.0.0,2.2.2]", upgrade.Major, "2.3.4"},
// Version range requirement is outdated but latest version is a major update
{"[1.0.0,2.0.0)", upgrade.Major, "2.3.4"},
{"[1.0.0,2.0.0)", upgrade.Minor, "[1.0.0,2.0.0)"},
Expand All @@ -426,3 +426,118 @@ func Test_suggestMavenVersion(t *testing.T) {
}
}
}

func TestSuggestVersion_Guava(t *testing.T) {
t.Parallel()
ctx := context.Background()
lc := resolve.NewLocalClient()

pk := resolve.PackageKey{
System: resolve.Maven,
Name: "com.google.guava:guava",
}
for _, version := range []string{"1.0.0", "1.0.1-android", "1.0.1-jre", "1.1.0-android", "1.1.0-jre", "2.0.0-android", "2.0.0-jre"} {
lc.AddVersion(resolve.Version{
VersionKey: resolve.VersionKey{
PackageKey: pk,
VersionType: resolve.Concrete,
Version: version,
}}, nil)
}

tests := []struct {
requirement string
level upgrade.Level
want string
}{
{"1.0.0", upgrade.Major, "2.0.0-jre"},
// Update to the version with the same flavour
{"1.0.1-jre", upgrade.Major, "2.0.0-jre"},
{"1.0.1-android", upgrade.Major, "2.0.0-android"},
{"1.0.1-jre", upgrade.Minor, "1.1.0-jre"},
{"1.0.1-android", upgrade.Minor, "1.1.0-android"},
// Version range requirement is not outdated
{"[1.0.0,)", upgrade.Major, "[1.0.0,)"},
// Version range requirement is outdated and the latest version is a major update
{"[1.0.0,2.0.0)", upgrade.Major, "2.0.0-jre"},
{"[1.0.0,2.0.0)", upgrade.Minor, "[1.0.0,2.0.0)"},
}
for _, tt := range tests {
vk := resolve.VersionKey{
PackageKey: pk,
VersionType: resolve.Requirement,
Version: tt.requirement,
}
want := resolve.RequirementVersion{
VersionKey: resolve.VersionKey{
PackageKey: pk,
VersionType: resolve.Requirement,
Version: tt.want,
},
}
got, err := suggestMavenVersion(ctx, lc, resolve.RequirementVersion{VersionKey: vk}, tt.level)
if err != nil {
t.Fatalf("fail to suggest a new version for %v: %v", vk, err)
}
if !reflect.DeepEqual(got, want) {
t.Errorf("suggestMavenVersion(%v, %v): got %s want %s", vk, tt.level, got, want)
}
}
}

func TestSuggestVersion_Commons(t *testing.T) {
t.Parallel()
ctx := context.Background()
lc := resolve.NewLocalClient()

pk := resolve.PackageKey{
System: resolve.Maven,
Name: "commons-io:commons-io",
}
for _, version := range []string{"1.0.0", "1.0.1", "1.1.0", "2.0.0", "20010101.000000"} {
lc.AddVersion(resolve.Version{
VersionKey: resolve.VersionKey{
PackageKey: pk,
VersionType: resolve.Concrete,
Version: version,
}}, nil)
}

tests := []struct {
requirement string
level upgrade.Level
want string
}{
{"1.0.0", upgrade.Major, "2.0.0"},
// No major updates allowed
{"1.0.0", upgrade.Minor, "1.1.0"},
// Only allow patch updates
{"1.0.0", upgrade.Patch, "1.0.1"},
// Version range requirement is not outdated
{"[1.0.0,)", upgrade.Major, "[1.0.0,)"},
// Version range requirement is outdated and the latest version is a major update
{"[1.0.0,2.0.0)", upgrade.Major, "2.0.0"},
{"[1.0.0,2.0.0)", upgrade.Minor, "[1.0.0,2.0.0)"},
}
for _, tt := range tests {
vk := resolve.VersionKey{
PackageKey: pk,
VersionType: resolve.Requirement,
Version: tt.requirement,
}
want := resolve.RequirementVersion{
VersionKey: resolve.VersionKey{
PackageKey: pk,
VersionType: resolve.Requirement,
Version: tt.want,
},
}
got, err := suggestMavenVersion(ctx, lc, resolve.RequirementVersion{VersionKey: vk}, tt.level)
if err != nil {
t.Fatalf("fail to suggest a new version for %v: %v", vk, err)
}
if !reflect.DeepEqual(got, want) {
t.Errorf("suggestMavenVersion(%v, %v): got %s want %s", vk, tt.level, got, want)
}
}
}
62 changes: 62 additions & 0 deletions internal/utility/maven/maven.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ import (
"fmt"
"os"
"path/filepath"
"strings"

"deps.dev/util/maven"
"deps.dev/util/resolve"
"deps.dev/util/semver"
"github.com/google/osv-scanner/v2/internal/datasource"
)

Expand Down Expand Up @@ -151,3 +154,62 @@ func GetDependencyManagement(ctx context.Context, client *datasource.MavenRegist

return result.DependencyManagement, nil
}

// CompareVersions compares two Maven semver versions with special behaviour for specific packages,
// producing more desirable ordering using non-standard comparison.
func CompareVersions(vk resolve.VersionKey, a *semver.Version, b *semver.Version) int {
if a == nil || b == nil {
if a == nil {
return -1
}

return 1
}

if vk.Name == "com.google.guava:guava" {
// com.google.guava:guava has 'flavors' with versions ending with -jre or -android.
// https://github.com/google/guava/wiki/ReleasePolicy#flavors
// To preserve the flavor in updates, we make the opposite flavor considered the earliest versions.

// Old versions have '22.0' and '22.0-android', and even older version don't have any flavors.
// Only check for the android flavor, and assume its jre otherwise.
wantAndroid := strings.HasSuffix(vk.Version, "-android")

aIsAndroid := strings.HasSuffix(a.String(), "-android")
bIsAndroid := strings.HasSuffix(b.String(), "-android")

if aIsAndroid == bIsAndroid {
return a.Compare(b)
}

if aIsAndroid == wantAndroid {
return 1
}

return -1
}

// Old versions of apache commons-* libraries (commons-io:commons-io, commons-math:commons-math, etc.)
// used date-based versions (e.g. 20040118.003354), which naturally sort after the more recent semver versions.
// We manually force the date versions to come before the others to prevent downgrades.
if strings.HasPrefix(vk.Name, "commons-") {
// All date-based versions of these packages seem to be in the years 2002-2005.
// It's extremely unlikely we'd see any versions dated before 1999 or after 2010.
// It's also unlikely we'd see any major versions of these packages reach up to 200.0.0.
// Checking if the version starts with "200" should therefore be sufficient to determine if it's a year.
aCal := strings.HasPrefix(a.String(), "200")
bCal := strings.HasPrefix(b.String(), "200")

if aCal == bCal {
return a.Compare(b)
}

if aCal {
return -1
}

return 1
}

return a.Compare(b)
}
Loading

0 comments on commit 5058d47

Please sign in to comment.