client: fix shadow bug in cache index retrieval

The bug results in potentially empty index being returned. When index is
stale for more than 2 hours, a new index is fetched remotely. If the
remote server says there were no changes in the meantime, the cached
index is returned, which can be empty due to shadowing.

Change-Id: I2267d936d21096c574d1c88f6c83ccc916c6c78f
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/349189
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
diff --git a/client/client.go b/client/client.go
index 6aa10ad..74a53af 100644
--- a/client/client.go
+++ b/client/client.go
@@ -98,11 +98,12 @@
 	var cachedIndexRetrieved *time.Time
 
 	if hs.cache != nil {
-		cachedIndex, retrieved, err := hs.cache.ReadIndex(hs.dbName)
+		index, retrieved, err := hs.cache.ReadIndex(hs.dbName)
 		if err != nil {
 			return nil, err
 		}
 
+		cachedIndex = index
 		if cachedIndex != nil {
 			if time.Since(retrieved) < time.Hour*2 {
 				return cachedIndex, nil
diff --git a/client/client_test.go b/client/client_test.go
index cd36fa8..309982b 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -11,6 +11,7 @@
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"os"
 	"path"
 	"reflect"
@@ -217,3 +218,40 @@
 		t.Errorf("unexpected fetches, got %v, want %v", fetches, expectedFetches)
 	}
 }
+
+// Make sure that a cached index is used in the case it is stale
+// but there were no changes to it at the server side.
+func TestCorrectFetchesNoChangeIndex(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/index.json" {
+			w.WriteHeader(http.StatusNotModified)
+		}
+	}))
+	defer ts.Close()
+	url, _ := url.Parse(ts.URL)
+
+	// set timestamp so that cached index is stale,
+	// i.e., more than two hours old.
+	timeStamp := time.Now().Add(time.Hour * (-3))
+	index := osv.DBIndex{"a": timeStamp}
+	cache := freshTestCache()
+	cache.WriteIndex(url.Hostname(), index, timeStamp)
+
+	e := &osv.Entry{
+		ID:       "ID1",
+		Modified: timeStamp,
+	}
+	cache.WriteEntries(url.Hostname(), "a", []*osv.Entry{e})
+
+	client, err := NewClient([]string{ts.URL}, Options{HTTPCache: cache})
+	if err != nil {
+		t.Fatal(err)
+	}
+	vulns, err := client.Get("a")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !reflect.DeepEqual(vulns, []*osv.Entry{e}) {
+		t.Errorf("want %v vuln; got %v", e, vulns)
+	}
+}