Revised fileserver Accept-Encoding and ETag

caddyhttp/staticfiles/fileserver.go

@@ -40,6 +40,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 fmt.Sprintf(`"%s%s"`, 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) {
@@ -135,9 +143,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
 		}
 
@@ -155,8 +173,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
@@ -166,12 +183,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).

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":                                     "<h1>must not leak</h1>",
-	filepath.Join("webroot", "file1.html"):                 "<h1>file1.html</h1>",
-	filepath.Join("webroot", "sub", "gzipped.html"):        "<h1>gzipped.html</h1>",
-	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"): "<h1>dirwithindex/index.html</h1>",
-	filepath.Join("webroot", "dir", "file2.html"):          "<h1>dir/file2.html</h1>",
-	filepath.Join("webroot", "dir", "hidden.html"):         "<h1>dir/hidden.html</h1>",
+	"unreachable.html":          "<h1>must not leak</h1>",
+	webrootFile1Html:            "<h1>file1.html</h1>",
+	webrootDirFile2Html:         "<h1>dir/file2.html</h1>",
+	webrootDirwithindexIndeHtml: "<h1>dirwithindex/index.html</h1>",
+	webrootDirHiddenHtml:        "<h1>dir/hidden.html</h1>",
+	webrootSubGzippedHtml:       "<h1>gzipped.html</h1>",
+	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,24 +184,40 @@ 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:    "",
 		},
 	}
 
 	for i, test := range tests {
 		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)
@@ -195,6 +228,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 {
@@ -211,9 +247,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)
 		}
 	}