diff --git a/caddyhttp/staticfiles/fileserver.go b/caddyhttp/staticfiles/fileserver.go index f6ca23ab..57df52ea 100644 --- a/caddyhttp/staticfiles/fileserver.go +++ b/caddyhttp/staticfiles/fileserver.go @@ -1,7 +1,6 @@ package staticfiles import ( - "fmt" "math/rand" "net/http" "os" @@ -40,6 +39,14 @@ func (fs FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, err return fs.serveFile(w, r, r.URL.Path) } +// calculateEtag produces a strong etag by default. Prefix the result with "W/" to convert this into a weak one. +// see https://tools.ietf.org/html/rfc7232#section-2.3 +func calculateEtag(d os.FileInfo) string { + t := strconv.FormatInt(d.ModTime().Unix(), 36) + s := strconv.FormatInt(d.Size(), 36) + return `"` + t + s + `"` +} + // serveFile writes the specified file to the HTTP response. // name is '/'-separated, not filepath.Separator. func (fs FileServer) serveFile(w http.ResponseWriter, r *http.Request, name string) (int, error) { @@ -138,9 +145,19 @@ func (fs FileServer) serveFile(w http.ResponseWriter, r *http.Request, name stri } filename := d.Name() + etag := calculateEtag(d) // strong for _, encoding := range staticEncodingPriority { - if !strings.Contains(r.Header.Get("Accept-Encoding"), encoding) { + acceptEncoding := strings.Split(r.Header.Get("Accept-Encoding"), ",") + + accepted := false + for _, acc := range acceptEncoding { + if accepted || strings.TrimSpace(acc) == encoding { + accepted = true + } + } + + if !accepted { continue } @@ -158,8 +175,7 @@ func (fs FileServer) serveFile(w http.ResponseWriter, r *http.Request, name stri // Close previous file - release fd f.Close() - // Stat is needed for generating valid ETag - d = encodedFileInfo + etag = calculateEtag(encodedFileInfo) // Encoded file will be served f = encodedFile @@ -169,12 +185,11 @@ func (fs FileServer) serveFile(w http.ResponseWriter, r *http.Request, name stri defer f.Close() break - } - // Experimental ETag header - e := fmt.Sprintf(`W/"%x-%x"`, d.ModTime().Unix(), d.Size()) - w.Header().Set("ETag", e) + // Set the ETag returned to the user-agent. Note that a conditional If-None-Match + // request is handled in http.ServeContent below, which checks against this ETag value. + w.Header().Set("ETag", etag) // Note: Errors generated by ServeContent are written immediately // to the response. This usually only happens if seeking fails (rare). diff --git a/caddyhttp/staticfiles/fileserver_test.go b/caddyhttp/staticfiles/fileserver_test.go index 6753ecac..1182dde2 100644 --- a/caddyhttp/staticfiles/fileserver_test.go +++ b/caddyhttp/staticfiles/fileserver_test.go @@ -19,6 +19,19 @@ var ( testWebRoot = filepath.Join(testDir, "webroot") ) +var ( + webrootFile1Html = filepath.Join("webroot", "file1.html") + webrootDirFile2Html = filepath.Join("webroot", "dir", "file2.html") + webrootDirHiddenHtml = filepath.Join("webroot", "dir", "hidden.html") + webrootDirwithindexIndeHtml = filepath.Join("webroot", "dirwithindex", "index.html") + webrootSubGzippedHtml = filepath.Join("webroot", "sub", "gzipped.html") + webrootSubGzippedHtmlGz = filepath.Join("webroot", "sub", "gzipped.html.gz") + webrootSubGzippedHtmlBr = filepath.Join("webroot", "sub", "gzipped.html.br") + webrootSubBrotliHtml = filepath.Join("webroot", "sub", "brotli.html") + webrootSubBrotliHtmlGz = filepath.Join("webroot", "sub", "brotli.html.gz") + webrootSubBrotliHtmlBr = filepath.Join("webroot", "sub", "brotli.html.br") +) + // testFiles is a map with relative paths to test files as keys and file content as values. // The map represents the following structure: // - $TEMP/caddy_testdir/ @@ -31,17 +44,17 @@ var ( // '------ file2.html // '------ hidden.html var testFiles = map[string]string{ - "unreachable.html": "

must not leak

", - filepath.Join("webroot", "file1.html"): "

file1.html

", - filepath.Join("webroot", "sub", "gzipped.html"): "

gzipped.html

", - filepath.Join("webroot", "sub", "gzipped.html.gz"): "gzipped.html.gz", - filepath.Join("webroot", "sub", "gzipped.html.gz"): "gzipped.html.gz", - filepath.Join("webroot", "sub", "brotli.html"): "brotli.html", - filepath.Join("webroot", "sub", "brotli.html.gz"): "brotli.html.gz", - filepath.Join("webroot", "sub", "brotli.html.br"): "brotli.html.br", - filepath.Join("webroot", "dirwithindex", "index.html"): "

dirwithindex/index.html

", - filepath.Join("webroot", "dir", "file2.html"): "

dir/file2.html

", - filepath.Join("webroot", "dir", "hidden.html"): "

dir/hidden.html

", + "unreachable.html": "

must not leak

", + webrootFile1Html: "

file1.html

", + webrootDirFile2Html: "

dir/file2.html

", + webrootDirwithindexIndeHtml: "

dirwithindex/index.html

", + webrootDirHiddenHtml: "

dir/hidden.html

", + webrootSubGzippedHtml: "

gzipped.html

", + webrootSubGzippedHtmlGz: "1.gzipped.html.gz", + webrootSubGzippedHtmlBr: "2.gzipped.html.br", + webrootSubBrotliHtml: "3.brotli.html", + webrootSubBrotliHtmlGz: "4.brotli.html.gz", + webrootSubBrotliHtmlBr: "5.brotli.html.br", } // TestServeHTTP covers positive scenarios when serving files. @@ -58,11 +71,14 @@ func TestServeHTTP(t *testing.T) { movedPermanently := "Moved Permanently" tests := []struct { - url string + url string + acceptEncoding string expectedStatus int expectedBodyContent string expectedEtag string + expectedVary string + expectedEncoding string }{ // Test 0 - access without any path { @@ -78,15 +94,15 @@ func TestServeHTTP(t *testing.T) { { url: "https://foo/file1.html", expectedStatus: http.StatusOK, - expectedBodyContent: testFiles[filepath.Join("webroot", "file1.html")], - expectedEtag: `W/"1e240-13"`, + expectedBodyContent: testFiles[webrootFile1Html], + expectedEtag: `"2n9cj"`, }, // Test 3 - access folder with index file with trailing slash { url: "https://foo/dirwithindex/", expectedStatus: http.StatusOK, - expectedBodyContent: testFiles[filepath.Join("webroot", "dirwithindex", "index.html")], - expectedEtag: `W/"1e240-20"`, + expectedBodyContent: testFiles[webrootDirwithindexIndeHtml], + expectedEtag: `"2n9cw"`, }, // Test 4 - access folder with index file without trailing slash { @@ -125,8 +141,8 @@ func TestServeHTTP(t *testing.T) { { url: "https://foo/dirwithindex/index.html", expectedStatus: http.StatusOK, - expectedBodyContent: testFiles[filepath.Join("webroot", "dirwithindex", "index.html")], - expectedEtag: `W/"1e240-20"`, + expectedBodyContent: testFiles[webrootDirwithindexIndeHtml], + expectedEtag: `"2n9cw"`, }, // Test 11 - send a request with query params { @@ -152,6 +168,7 @@ func TestServeHTTP(t *testing.T) { // Test 15 - attempt to bypass hidden file { url: "https://foo/dir/hidden.html%20.", + acceptEncoding: "br, gzip", expectedStatus: http.StatusNotFound, }, // Test 16 - serve another file with same name as hidden file. @@ -167,16 +184,32 @@ func TestServeHTTP(t *testing.T) { // Test 18 - try to get pre-gzipped file. { url: "https://foo/sub/gzipped.html", + acceptEncoding: "gzip", expectedStatus: http.StatusOK, - expectedBodyContent: testFiles[filepath.Join("webroot", "sub", "gzipped.html.gz")], - expectedEtag: `W/"1e240-f"`, + expectedBodyContent: testFiles[webrootSubGzippedHtmlGz], + expectedEtag: `"2n9ch"`, + expectedVary: "Accept-Encoding", + expectedEncoding: "gzip", }, // Test 19 - try to get pre-brotli encoded file. { url: "https://foo/sub/brotli.html", + acceptEncoding: "br,gzip", expectedStatus: http.StatusOK, - expectedBodyContent: testFiles[filepath.Join("webroot", "sub", "brotli.html.br")], - expectedEtag: `W/"1e240-e"`, + expectedBodyContent: testFiles[webrootSubBrotliHtmlBr], + expectedEtag: `"2n9cg"`, + expectedVary: "Accept-Encoding", + expectedEncoding: "br", + }, + // Test 20 - not allowed to get pre-brotli encoded file. + { + url: "https://foo/sub/brotli.html", + acceptEncoding: "nicebrew", // contains "br" substring but not "br" + expectedStatus: http.StatusOK, + expectedBodyContent: testFiles[webrootSubBrotliHtml], + expectedEtag: `"2n9cd"`, + expectedVary: "", + expectedEncoding: "", }, // Test 20 - treat existing file as a directory. { @@ -189,7 +222,7 @@ func TestServeHTTP(t *testing.T) { responseRecorder := httptest.NewRecorder() request, err := http.NewRequest("GET", test.url, nil) - request.Header.Add("Accept-Encoding", "br,gzip") + request.Header.Add("Accept-Encoding", test.acceptEncoding) if err != nil { t.Errorf("Test %d: Error making request: %v", i, err) @@ -200,6 +233,9 @@ func TestServeHTTP(t *testing.T) { } status, err := fileserver.ServeHTTP(responseRecorder, request) etag := responseRecorder.Header().Get("Etag") + body := responseRecorder.Body.String() + vary := responseRecorder.Header().Get("Vary") + encoding := responseRecorder.Header().Get("Content-Encoding") // check if error matches expectations if err != nil { @@ -216,9 +252,19 @@ func TestServeHTTP(t *testing.T) { t.Errorf("Test %d: Expected Etag header %s, found %s", i, test.expectedEtag, etag) } + // check vary + if test.expectedVary != vary { + t.Errorf("Test %d: Expected Vary header %s, found %s", i, test.expectedVary, vary) + } + + // check content-encoding + if test.expectedEncoding != encoding { + t.Errorf("Test %d: Expected Content-Encoding header %s, found %s", i, test.expectedEncoding, encoding) + } + // check body content - if !strings.Contains(responseRecorder.Body.String(), test.expectedBodyContent) { - t.Errorf("Test %d: Expected body to contain %q, found %q", i, test.expectedBodyContent, responseRecorder.Body.String()) + if !strings.Contains(body, test.expectedBodyContent) { + t.Errorf("Test %d: Expected body to contain %q, found %q", i, test.expectedBodyContent, body) } } @@ -418,3 +464,53 @@ func TestServeHTTPFailingStat(t *testing.T) { } } } + +//------------------------------------------------------------------------------------------------- + +type fileInfo struct { + name string + size int64 + mode os.FileMode + modTime time.Time + isDir bool +} + +func (fi fileInfo) Name() string { + return fi.name +} + +func (fi fileInfo) Size() int64 { + return fi.size +} + +func (fi fileInfo) Mode() os.FileMode { + return fi.mode +} + +func (fi fileInfo) ModTime() time.Time { + return fi.modTime +} + +func (fi fileInfo) IsDir() bool { + return fi.isDir +} + +func (fi fileInfo) Sys() interface{} { + return nil +} + +var _ os.FileInfo = fileInfo{} + +//------------------------------------------------------------------------------------------------- + +func BenchmarkEtag(b *testing.B) { + d := fileInfo{ + size: 1234567890, + modTime: time.Now(), + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + calculateEtag(d) + } +}