caddyhttp: omit Last-Modified for unusable mod times (#7740)

See #5548 and #7730

modules/caddyhttp/fileserver/staticfiles.go

@@ -29,6 +29,7 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
+	"time"
 
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
@@ -579,7 +580,17 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c
 	// that errors generated by ServeContent are written immediately
 	// to the response, so we cannot handle them (but errors there
 	// are rare)
-	http.ServeContent(w, r, info.Name(), info.ModTime(), file.(io.ReadSeeker))
+	//
+	// There are a few file modification times that aren't useful
+	// to send in Last-Modified headers, but the golang http library only
+	// omits Last-Modified headers for the Unix epoch time. So, force
+	// the modification time to the epoch time if it's not useful.
+	zeroTime := time.Time{}
+	modTime := info.ModTime()
+	if !usefulModTime(modTime) {
+		modTime = zeroTime
+	}
+	http.ServeContent(w, r, info.Name(), modTime, file.(io.ReadSeeker))
 
 	return nil
 }
@@ -726,6 +737,14 @@ func (fsrv *FileServer) notFound(w http.ResponseWriter, r *http.Request, next ca
 	return caddyhttp.Error(http.StatusNotFound, nil)
 }
 
+// Indicates whether a file's modification time is useful for validator
+// generation purposes (i.e. inclusion in ETag and Last-Modified headers).
+// See issues #5548 and #7730.
+func usefulModTime(modTime time.Time) bool {
+	mtimeunix := modTime.Unix()
+	return mtimeunix != 0 && mtimeunix != 1
+}
+
 // calculateEtag computes an entity tag using a strong validator
 // without consuming the contents of the file. It requires the
 // file info contain the correct size and modification time.
@@ -743,8 +762,8 @@ func (fsrv *FileServer) notFound(w http.ResponseWriter, r *http.Request, next ca
 // which we consider precise enough to qualify as a strong validator.
 func calculateEtag(d os.FileInfo) string {
 	mtime := d.ModTime()
-	if mtimeUnix := mtime.Unix(); mtimeUnix == 0 || mtimeUnix == 1 {
-		return "" // not useful anyway; see issue #5548
+	if !usefulModTime(mtime) {
+		return ""
 	}
 	var sb strings.Builder
 	sb.WriteRune('"')

modules/caddyhttp/fileserver/staticfiles_test.go

@@ -15,10 +15,17 @@
 package fileserver
 
 import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"os"
 	"path/filepath"
 	"runtime"
 	"strings"
 	"testing"
+	"time"
+
+	"github.com/caddyserver/caddy/v2"
 )
 
 func TestFileHidden(t *testing.T) {
@@ -128,3 +135,52 @@ func TestFileHidden(t *testing.T) {
 		}
 	}
 }
+
+// Check to make sure that we don't serve ETag and Last-Modified headers
+// for files with invalid modification times
+func TestModTimeHeaders(t *testing.T) {
+	check_validator_headers(time.Now(), true, t)
+	check_validator_headers(time.Unix(0, 0), false, t)
+	check_validator_headers(time.Unix(1, 0), false, t)
+	check_validator_headers(time.Unix(2, 0), true, t)
+}
+
+func check_validator_headers(modTime time.Time, expect_headers bool, t *testing.T) {
+	f := false
+	fsrv := FileServer{
+		Root:          "./testdata",
+		CanonicalURIs: &f,
+	}
+	w := httptest.NewRecorder()
+	r, err := http.NewRequest("GET", "/modtime.txt", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	repl := caddy.NewReplacer()
+	ctx := context.WithValue(r.Context(), caddy.ReplacerCtxKey, repl)
+	r = r.WithContext(ctx)
+
+	ctx2, _ := caddy.NewContext(caddy.Context{Context: context.Background()}) // module will be nil by default
+	fsrv.Provision(ctx2)
+
+	path := "testdata/modtime.txt"
+	os.Chtimes(path, modTime, modTime)
+
+	fsrv.ServeHTTP(w, r, nil)
+
+	if expect_headers {
+		if w.Header().Get("ETag") == "" {
+			t.Errorf("Didn't get ETag header for file with valid mod time %s", modTime)
+		}
+		if w.Header().Get("Last-Modified") == "" {
+			t.Errorf("Didn't get Last-Modified header for file with valid mod time %s", modTime)
+		}
+	} else {
+		if w.Header().Get("ETag") != "" {
+			t.Errorf("Got ETag header for file with invalid mod time %s", modTime)
+		}
+		if w.Header().Get("Last-Modified") != "" {
+			t.Errorf("Got Last-Modified header for file with invalid mod time %s", modTime)
+		}
+	}
+}