diff --git a/CHANGELOG.md b/CHANGELOG.md index dc3298d03..49d4d1087 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/main_test.go b/main_test.go index ab4224ee6..19d1e013a 100644 --- a/main_test.go +++ b/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") diff --git a/packages/http.go b/packages/http.go index e47ef82b6..19d61054a 100644 --- a/packages/http.go +++ b/packages/http.go @@ -8,7 +8,6 @@ import ( "log" "net/http" "os" - "time" "go.elastic.co/apm" @@ -59,11 +58,18 @@ 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("stat failed for %s: %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) @@ -71,5 +77,5 @@ func ServeFile(w http.ResponseWriter, r *http.Request, p *Package, name string) } defer f.Close() - http.ServeContent(w, r, name, time.Time{}, f) + http.ServeContent(w, r, name, stat.ModTime(), f) }