internal/frontend: improve display of vul affected versions
Instead of "Introduced" and "Fixed" columns, construct a more readable
string that describes the vulnerable versions.
Change-Id: Iac7435a0b372bde23f0c6177490b39f16505b5a7
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/411077
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Julie Qiu <julieqiu@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
diff --git a/internal/frontend/vulns.go b/internal/frontend/vulns.go
index ffb22a5..b2c1253 100644
--- a/internal/frontend/vulns.go
+++ b/internal/frontend/vulns.go
@@ -8,6 +8,7 @@
"fmt"
"net/http"
"sort"
+ "strings"
"golang.org/x/mod/semver"
"golang.org/x/pkgsite/internal"
@@ -74,7 +75,13 @@
// VulnPage holds the information for a page that displays a single vuln entry.
type VulnPage struct {
basePage
- Entry *osv.Entry
+ Entry *osv.Entry
+ AffectedPackages []*AffectedPackage
+}
+
+type AffectedPackage struct {
+ Path string // Package.Name in the osv.Entry
+ Versions string
}
func entryVuln(e *osv.Entry, packagePath, version string) (Vuln, bool) {
@@ -140,8 +147,8 @@
if entry == nil {
return nil, derrors.NotFound
}
- addVersionPrefixes(entry)
- return &VulnPage{Entry: entry}, nil
+ affs := affectedPackages(entry)
+ return &VulnPage{Entry: entry, AffectedPackages: affs}, nil
}
func newVulnListPage(client vulnc.Client) (*VulnListPage, error) {
@@ -177,6 +184,78 @@
return &VulnListPage{Entries: entries}, nil
}
+// A pair is like an osv.Range, but each pair is a self-contained 2-tuple
+// (introduced version, fixed version).
+type pair struct {
+ intro, fixed string
+}
+
+// collectRangePairs turns a slice of osv Ranges into a more manageable slice of
+// formatted version pairs.
+func collectRangePairs(a osv.Affected) []pair {
+ var (
+ ps []pair
+ p pair
+ prefix string
+ )
+ if stdlib.Contains(a.Package.Name) {
+ prefix = "go"
+ } else {
+ prefix = "v"
+ }
+ for _, r := range a.Ranges {
+ isSemver := r.Type == osv.TypeSemver
+ for _, v := range r.Events {
+ if v.Introduced != "" {
+ if p.intro != "" {
+ // We expected Introduced and Fixed to alternate, but they don't.
+ // Keep going, ignoring the first Introduced.
+ }
+ p.intro = v.Introduced
+ if p.intro == "0" {
+ p.intro = ""
+ }
+ if isSemver && p.intro != "" {
+ p.intro = prefix + p.intro
+ }
+ }
+ if v.Fixed != "" {
+ p.fixed = v.Fixed
+ if isSemver && p.fixed != "" {
+ p.fixed = prefix + p.fixed
+ }
+ ps = append(ps, p)
+ p = pair{}
+ }
+ }
+ }
+ return ps
+}
+
+func affectedPackages(e *osv.Entry) []*AffectedPackage {
+ var affs []*AffectedPackage
+ for _, a := range e.Affected {
+ pairs := collectRangePairs(a)
+ var vs []string
+ for _, p := range pairs {
+ var s string
+ if p.intro == "" {
+ s = p.fixed + " and earlier"
+ } else if p.fixed == "" {
+ s = p.intro + " and later"
+ } else {
+ s = p.intro + " - " + p.fixed
+ }
+ vs = append(vs, s)
+ }
+ affs = append(affs, &AffectedPackage{
+ Path: a.Package.Name,
+ Versions: strings.Join(vs, ", "),
+ })
+ }
+ return affs
+}
+
func addVersionPrefix(semver, packagePath string) (res string) {
if semver == "" {
return ""
@@ -186,16 +265,3 @@
}
return "v" + semver
}
-
-func addVersionPrefixes(e *osv.Entry) {
- for i, a := range e.Affected {
- for j, r := range a.Ranges {
- if r.Type == osv.TypeSemver {
- for k, v := range r.Events {
- e.Affected[i].Ranges[j].Events[k].Introduced = addVersionPrefix(v.Introduced, a.Package.Name)
- e.Affected[i].Ranges[j].Events[k].Fixed = addVersionPrefix(v.Fixed, a.Package.Name)
- }
- }
- }
- }
-}
diff --git a/internal/frontend/vulns_test.go b/internal/frontend/vulns_test.go
index 6fdd240..82699f5 100644
--- a/internal/frontend/vulns_test.go
+++ b/internal/frontend/vulns_test.go
@@ -7,6 +7,7 @@
import (
"errors"
"fmt"
+ "reflect"
"testing"
"github.com/google/go-cmp/cmp"
@@ -96,63 +97,6 @@
}
}
-func TestAddVersionPrefixes(t *testing.T) {
- in := &osv.Entry{
- ID: "id",
- Affected: []osv.Affected{
- {
- Package: osv.Package{Name: "a.com/b"},
- Ranges: osv.Affects{{
- Type: osv.TypeSemver, Events: []osv.RangeEvent{
- {Introduced: "1.2.3"},
- {Fixed: "1.4.0"},
- {Introduced: "2.1.0", Fixed: "2.2.1"},
- }},
- },
- },
- {
- Package: osv.Package{Name: "crypto/math"},
- Ranges: osv.Affects{{
- Type: osv.TypeSemver, Events: []osv.RangeEvent{
- {Introduced: "1.2.3"},
- {Fixed: "1.4.0"},
- {Introduced: "2.1.0", Fixed: "2.2.1"},
- }},
- },
- },
- },
- }
- want := &osv.Entry{
- ID: "id",
- Affected: []osv.Affected{
- {
- Package: osv.Package{Name: "a.com/b"},
- Ranges: osv.Affects{{
- Type: osv.TypeSemver, Events: []osv.RangeEvent{
- {Introduced: "v1.2.3"},
- {Fixed: "v1.4.0"},
- {Introduced: "v2.1.0", Fixed: "v2.2.1"},
- }},
- },
- },
- {
- Package: osv.Package{Name: "crypto/math"},
- Ranges: osv.Affects{{
- Type: osv.TypeSemver, Events: []osv.RangeEvent{
- {Introduced: "go1.2.3"},
- {Fixed: "go1.4.0"},
- {Introduced: "go2.1.0", Fixed: "go2.2.1"},
- }},
- },
- },
- },
- }
- addVersionPrefixes(in)
- if diff := cmp.Diff(want, in); diff != "" {
- t.Errorf("mismatch (-want, +got)\n%s", diff)
- }
-}
-
type vulndbTestClient struct {
vulnc.Client
entries []*osv.Entry
@@ -178,3 +122,28 @@
}
return ids, nil
}
+
+func TestCollectRangePairs(t *testing.T) {
+ in := osv.Affected{
+ Package: osv.Package{Name: "github.com/a/b"},
+ Ranges: osv.Affects{
+ {Type: osv.TypeSemver, Events: []osv.RangeEvent{{Introduced: "", Fixed: "0.5"}}},
+ {Type: osv.TypeSemver, Events: []osv.RangeEvent{
+ {Introduced: "1.2"}, {Fixed: "1.5"},
+ {Introduced: "2.1", Fixed: "2.3"},
+ }},
+ {Type: osv.TypeGit, Events: []osv.RangeEvent{{Introduced: "a", Fixed: "b"}}},
+ },
+ }
+ got := collectRangePairs(in)
+ want := []pair{
+ {"", "v0.5"},
+ {"v1.2", "v1.5"},
+ {"v2.1", "v2.3"},
+ {"a", "b"},
+ }
+ if !reflect.DeepEqual(got, want) {
+ t.Errorf("\ngot %+v\nwant %+v", got, want)
+ }
+
+}
diff --git a/static/frontend/vuln/entry/entry.tmpl b/static/frontend/vuln/entry/entry.tmpl
index c4ad740..5cb2636 100644
--- a/static/frontend/vuln/entry/entry.tmpl
+++ b/static/frontend/vuln/entry/entry.tmpl
@@ -13,6 +13,7 @@
{{end}}
{{define "main-content"}}
+ {{/* . is internal/frontend.VulnPage */}}
<nav class="go-Breadcrumb" aria-label="Breadcrumb">
<ol>
<li>
@@ -25,45 +26,43 @@
</nav>
<h1 class="Vuln-title">Vulnerability Report: {{.Entry.ID}}</h1>
{{template "vuln-details" .Entry}}
- {{template "entry" .Entry}}
+ <div class="VulnEntry">
+ {{template "affected" .AffectedPackages}}
+ {{template "entry" .Entry}}
+ </div>
+{{end}}
+
+{{define "affected"}}
+ <h2>Affected Packages</h2>
+ <table class="VulnEntry-table">
+ <thead>
+ <tr>
+ <th>Package</th>
+ <th>Affected Versions</th>
+ </tr>
+ </thead>
+ <tbody>
+ {{range .}}
+ <tr>
+ <td><a href="/{{.Path}}">{{.Path}}</a></td>
+ <td>{{.Versions}}</td>
+ </tr>
+ {{end}}
+ </tbody>
+ </table>
{{end}}
{{define "entry"}}
- <div class="VulnEntry">
- <h2>Affected Packages</h2>
- <table class="VulnEntry-table">
- <thead>
- <tr>
- <th>Package</th>
- <th>Introduced</th>
- <th>Fixed</th>
- </tr>
- </thead>
- <tbody>
- {{range .Affected}}
- <tr>
- <td><a href="/{{.Package.Name}}">{{.Package.Name}}</a></td>
- <td>
- {{range .Ranges}}{{range .Events}}
- <div>{{if not (eq .Introduced "0")}}{{.Introduced}}{{end}}</div>
- {{end}}{{end}}
- </td>
- <td>{{range .Ranges}}{{range .Events}}<div>{{.Fixed}}</div>{{end}}{{end}}</td>
- </tr>
- {{end}}
- </tbody>
- </table>
- {{if .Aliases}}
- <h2>Aliases</h2>
- <ul>
- {{range .Aliases}}<li>{{.}}</li>{{end}}
- </ul>
- {{end}}
- {{if .References}}
- <h2>References</h2>
- <ul class="VulnEntry-referenceList">
- {{range .References}}<li><a href="{{.URL}}">{{.URL}}</a></li>{{end}}
- </ul>
- {{end}}
- </div>
+ {{if .Aliases}}
+ <h2>Aliases</h2>
+ <ul>
+ {{range .Aliases}}<li>{{.}}</li>{{end}}
+ </ul>
+ {{end}}
+ {{if .References}}
+ <h2>References</h2>
+ <ul class="VulnEntry-referenceList">
+ {{range .References}}<li><a href="{{.URL}}">{{.URL}}</a></li>{{end}}
+ </ul>
+ {{end}}
{{end}}
