client: adds unit tests and addresses minor issues.
Change-Id: I9151991794618c11cca9dffb3b79ebbb42989d16
Reviewed-on: https://team-review.git.corp.google.com/c/golang/vulndb/+/1036403
Reviewed-by: Roland Shoemaker <bracewell@google.com>
diff --git a/client/cache.go b/client/cache.go
index 05d031c..dd5c3bb 100644
--- a/client/cache.go
+++ b/client/cache.go
@@ -16,7 +16,7 @@
// it does not implement file locking. When ported to the stdlib it
// should use cmd/go/internal/lockedfile.
-// The cahce uses a single JSON index file for each vulnerability database
+// The cache uses a single JSON index file for each vulnerability database
// which contains the map from packages to the time the last
// vulnerability for that package was added/modified and the time that
// the index was retrieved from the vulnerability database. The JSON
diff --git a/client/client.go b/client/client.go
index c42ac2f..a2d939b 100644
--- a/client/client.go
+++ b/client/client.go
@@ -7,7 +7,6 @@
"net/http"
"net/url"
"os"
- "path"
"path/filepath"
"strings"
"time"
@@ -57,7 +56,7 @@
}
type httpSource struct {
- url string
+ url string // the base URI of the source (without trailing "/"). e.g. https://vuln.golang.org
c *http.Client
cache Cache
dbName string
@@ -82,7 +81,7 @@
}
}
- req, err := http.NewRequest("GET", path.Join(hs.url, "index.json"), nil)
+ req, err := http.NewRequest("GET", fmt.Sprintf("%s/index.json", hs.url), nil)
if err != nil {
return nil, err
}
@@ -135,10 +134,10 @@
}
if cached, err := hs.cache.ReadEntries(hs.dbName, p); err != nil {
return nil, err
- } else if cached != nil {
+ } else if len(cached) != 0 {
var stale bool
- for _, e := range entries {
- if e.LastModified.Before(lastModified) {
+ for _, c := range cached {
+ if c.LastModified.Before(lastModified) {
stale = true
break
}
@@ -155,7 +154,7 @@
}
for _, p := range stillNeed {
- resp, err := hs.c.Get(path.Join(hs.url, p+".json"))
+ resp, err := hs.c.Get(fmt.Sprintf("%s/%s.json", hs.url, p))
if err != nil {
return nil, err
}
@@ -182,7 +181,7 @@
}
}
}
- return nil, nil
+ return entries, nil
}
type Client struct {
@@ -197,9 +196,10 @@
func NewClient(sources []string, opts Options) (*Client, error) {
c := &Client{}
for _, uri := range sources {
+ uri = strings.TrimRight(uri, "/")
// should parse the URI out here instead of in there
switch {
- case strings.HasPrefix("http://", uri) || strings.HasPrefix("https://", uri):
+ case strings.HasPrefix(uri, "http://") || strings.HasPrefix(uri, "https://"):
hs := &httpSource{url: uri}
url, err := url.Parse(uri)
if err != nil {
@@ -215,7 +215,7 @@
hs.c = new(http.Client)
}
c.sources = append(c.sources, hs)
- case strings.HasPrefix("file://", uri):
+ case strings.HasPrefix(uri, "file://"):
url, err := url.Parse(uri)
if err != nil {
return nil, err
diff --git a/client/client_test.go b/client/client_test.go
new file mode 100644
index 0000000..04b1fda
--- /dev/null
+++ b/client/client_test.go
@@ -0,0 +1,142 @@
+package client
+
+import (
+ "fmt"
+ "io/ioutil"
+ "net/http"
+ "os"
+ "path"
+ "testing"
+ "time"
+
+ "golang.org/x/vulndb/osv"
+)
+
+var testVuln1 string = `[
+ {"ID":"ID1","Package":{"Name":"golang.org/example/one","Ecosystem":"go"}, "Summary":"",
+ "Severity":2,"Affects":{"Ranges":[{"Type":2,"Introduced":"","Fixed":"v2.2.0"}]},
+ "ecosystem_specific":{"Symbols":["some_symbol_1"]
+ }}]`
+
+var testVuln2 string = `[
+ {"ID":"ID2","Package":{"Name":"golang.org/example/two","Ecosystem":"go"}, "Summary":"",
+ "Severity":2,"Affects":{"Ranges":[{"Type":2,"Introduced":"","Fixed":"v2.1.0"}]},
+ "ecosystem_specific":{"Symbols":["some_symbol_2"]
+ }}]`
+
+// index containing timestamps for packages in testVuln1 and testVuln2.
+var index string = `{
+ "golang.org/example/one": "2020-03-09T10:00:00.81362141-07:00",
+ "golang.org/example/two": "2019-02-05T09:00:00.31561157-07:00"
+ }`
+
+func serveTestVuln1(w http.ResponseWriter, req *http.Request) {
+ fmt.Fprintf(w, testVuln1)
+}
+
+func serveTestVuln2(w http.ResponseWriter, req *http.Request) {
+ fmt.Fprintf(w, testVuln2)
+}
+
+func serveIndex(w http.ResponseWriter, req *http.Request) {
+ fmt.Fprintf(w, index)
+}
+
+// cachedTestVuln2 returns a function creating a local cache
+// for db with `dbName` with a version of testVuln2 where
+// Summary="cached" and LastModified happened after entry
+// in the `index` for the same pkg.
+func cachedTestVuln2(dbName string) func() Cache {
+ return func() Cache {
+ c := &fsCache{}
+ e := &osv.Entry{
+ ID: "ID2",
+ Summary: "cached",
+ LastModified: time.Now(),
+ }
+ c.WriteEntries(dbName, "golang.org/example/two", []*osv.Entry{e})
+ return c
+ }
+}
+
+// createDirAndFile creates a directory `dir` if such directory does
+// not exist and creates a `file` with `content` in the directory.
+func createDirAndFile(dir, file, content string) error {
+ if err := os.MkdirAll(dir, 0755); err != nil {
+ return err
+ }
+ return ioutil.WriteFile(path.Join(dir, file), []byte(content), 0644)
+}
+
+// localDB creates a local db with testVuln1, testVuln2, and index as contents.
+func localDB(t *testing.T) (string, error) {
+ dbName := t.TempDir()
+
+ if err := createDirAndFile(path.Join(dbName, "/golang.org/example/"), "one.json", testVuln1); err != nil {
+ return "", err
+ }
+ if err := createDirAndFile(path.Join(dbName, "/golang.org/example/"), "two.json", testVuln2); err != nil {
+ return "", err
+ }
+ if err := createDirAndFile(path.Join(dbName, ""), "index.json", index); err != nil {
+ return "", err
+ }
+ return dbName, nil
+}
+
+func TestClient(t *testing.T) {
+ // Create a local http database.
+ http.HandleFunc("/golang.org/example/one.json", serveTestVuln1)
+ http.HandleFunc("/golang.org/example/two.json", serveTestVuln2)
+ http.HandleFunc("/index.json", serveIndex)
+ go func() { http.ListenAndServe(":8080", nil) }()
+
+ // Create a local file database.
+ localDBName, err := localDB(t)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer os.RemoveAll(localDBName)
+
+ for _, test := range []struct {
+ name string
+ source string
+ createCache func() Cache
+ noVulns int
+ summaries map[string]string
+ }{
+ // Test the http client without any cache.
+ {name: "http-no-cache", source: "http://localhost:8080", createCache: func() Cache { return nil }, noVulns: 2, summaries: map[string]string{"ID1": "", "ID2": ""}},
+ // Test the http client with empty cache.
+ {name: "http-empty-cache", source: "http://localhost:8080", createCache: func() Cache { return &fsCache{} }, noVulns: 2, summaries: map[string]string{"ID1": "", "ID2": ""}},
+ // Test the client with non-stale cache containing a version of testVuln2 where Summary="cached".
+ {name: "http-cache", source: "http://localhost:8080", createCache: cachedTestVuln2("localhost"), noVulns: 2, summaries: map[string]string{"ID1": "", "ID2": "cached"}},
+ // Repeat the same for local file client.
+ {name: "file-no-cache", source: "file://" + localDBName, createCache: func() Cache { return nil }, noVulns: 2, summaries: map[string]string{"ID1": "", "ID2": ""}},
+ {name: "file-empty-cache", source: "file://" + localDBName, createCache: func() Cache { return &fsCache{} }, noVulns: 2, summaries: map[string]string{"ID1": "", "ID2": ""}},
+ // Cache does not play a role in local file databases.
+ {name: "file-cache", source: "file://" + localDBName, createCache: cachedTestVuln2(localDBName), noVulns: 2, summaries: map[string]string{"ID1": "", "ID2": ""}},
+ } {
+ // Create fresh cache location each time.
+ cacheRoot = t.TempDir()
+
+ client, err := NewClient([]string{test.source}, Options{HTTPCache: test.createCache()})
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ vulns, err := client.Get([]string{"golang.org/example/one", "golang.org/example/two"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(vulns) != test.noVulns {
+ t.Errorf("want %v vulns for %s; got %v", test.noVulns, test.name, len(vulns))
+ }
+
+ for _, v := range vulns {
+ if s, ok := test.summaries[v.ID]; !ok || v.Summary != s {
+ t.Errorf("want '%s' summary for vuln with id %v in %s; got '%s'", s, v.ID, test.name, v.Summary)
+ }
+ }
+ }
+}
