Handle Last-Modified/If-Modified-Since headers

CHANGELOG.md

@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Bugfixes
 
+* Properly handle modification headers (`If-Modified-Since`, `Last-Modified`) for static resources. [#756](https://github.com/elastic/package-registry/pull/756)
+
 ### Added
 
 ### Deprecated

main_test.go

@@ -141,6 +141,99 @@ func TestStatics(t *testing.T) {
 
 }
 
+func TestStaticsModifiedTime(t *testing.T) {
+	const ifModifiedSinceHeader = "If-Modified-Since"
+	const lastModifiedHeader = "Last-Modified"
+
+	tests := []struct {
+		title    string
+		endpoint string
+		headers  map[string]string
+		code     int
+	}{
+		{
+			title:    "No cache headers",
+			endpoint: "/package/example/1.0.0/img/kibana-envoyproxy.jpg",
+			code:     200,
+		},
+		{
+			title:    "Doesn't exist",
+			endpoint: "/package/none/1.0.0/img/foo.jpg",
+			code:     404,
+		},
+		{
+			title:    "Cached entry",
+			endpoint: "/package/example/1.0.0/img/kibana-envoyproxy.jpg",
+			headers: map[string]string{
+				// Assuming that the file hasn't been modified in the future.
+				ifModifiedSinceHeader: time.Now().UTC().Format(http.TimeFormat),
+			},
+			code: 304,
+		},
+		{
+			title:    "Old cached entry",
+			endpoint: "/package/example/1.0.0/img/kibana-envoyproxy.jpg",
+			headers: map[string]string{
+				ifModifiedSinceHeader: time.Time{}.Format(http.TimeFormat),
+			},
+			code: 200,
+		},
+
+		// From zip
+		{
+			title:    "No cache headers from zip",
+			endpoint: "/package/example/1.0.1/img/kibana-envoyproxy.jpg",
+			code:     200,
+		},
+		{
+			title:    "Cached entry from zip",
+			endpoint: "/package/example/1.0.1/img/kibana-envoyproxy.jpg",
+			headers: map[string]string{
+				// Assuming that the file hasn't been modified in the future.
+				ifModifiedSinceHeader: time.Now().UTC().Format(http.TimeFormat),
+			},
+			code: 304,
+		},
+		{
+			title:    "Old cached entry from zip",
+			endpoint: "/package/example/1.0.1/img/kibana-envoyproxy.jpg",
+			headers: map[string]string{
+				ifModifiedSinceHeader: time.Time{}.Format(http.TimeFormat),
+			},
+			code: 200,
+		},
+	}
+
+	indexer := NewCombinedIndexer(
+		packages.NewFileSystemIndexer("./testdata/package"),
+		packages.NewZipFileSystemIndexer("./testdata/local-storage"),
+	)
+	err := indexer.Init(context.Background())
+	require.NoError(t, err)
+
+	router := mux.NewRouter()
+	router.HandleFunc(staticRouterPath, staticHandler(indexer, testCacheTime))
+
+	for _, test := range tests {
+		t.Run(test.title, func(t *testing.T) {
+			req, err := http.NewRequest("GET", test.endpoint, nil)
+			require.NoError(t, err)
+
+			for k, v := range test.headers {
+				req.Header.Add(k, v)
+			}
+
+			recorder := httptest.NewRecorder()
+			router.ServeHTTP(recorder, req)
+
+			assert.Equal(t, test.code, recorder.Code)
+			if test.code >= 0 && test.code < 400 {
+				assert.NotEmpty(t, recorder.Header().Values(lastModifiedHeader))
+			}
+		})
+	}
+}
+
 func TestZippedArtifacts(t *testing.T) {
 	indexer := packages.NewZipFileSystemIndexer("./testdata/local-storage")
 

packages/http.go

@@ -8,7 +8,6 @@ import (
 	"log"
 	"net/http"
 	"os"
-	"time"
 
 	"go.elastic.co/apm"
 
@@ -59,17 +58,24 @@ func ServeFile(w http.ResponseWriter, r *http.Request, p *Package, name string)
 		return
 	}
 
-	f, err := fs.Open(name)
+	stat, err := fs.Stat(name)
 	if os.IsNotExist(err) {
 		http.Error(w, "resource not found", http.StatusNotFound)
 		return
 	}
+	if err != nil {
+		log.Printf("failed to check file modification time (%s) in package: %v", name, err)
+		http.Error(w, "internal server error", http.StatusInternalServerError)
+		return
+	}
+
+	f, err := fs.Open(name)
 	if err != nil {
 		log.Printf("failed to open file (%s) in package: %v", name, err)
 		http.Error(w, "internal server error", http.StatusInternalServerError)
 		return
 	}
 	defer f.Close()
 
-	http.ServeContent(w, r, name, time.Time{}, f)
+	http.ServeContent(w, r, name, stat.ModTime(), f)
 }