Repository: caddyserver/caddy (language: go)

Issue:
Consider dropping ETag, Last-Modified-Time generation for files where epoch <= mtime <= epoch+1
#5547, as a fix for #5546, makes it _possible_ to override caddy's default ETag generation.

That said, I'd like to suggest considering making that ETag generation conditional in the first place.

Not letting clients do caching at all is a better failure mode than encouraging clients to cache things that aren't safely cacheable. When content doesn't have a meaningful mtime, generating an ETag off that mtime leads to absurd results (effectively, leaving file size as the only cache-invalidation key). As a special case, an mtime of 0 or 1 (as measured in seconds-since-UNIX-epoch) is clearly not meaningful.

On its own, the existing PR makes it possible for the administrator of a system where mtimes are not meaningful to instruct Caddy to do something different instead.

If we implemented a safeguard such as that proposed here in addition to that PR, then the default for administrators who failed to take the actions enabled by that PR would be less harmful -- more client requests, rather than clients using out-of-date data.


Discussion / hints:
Does `header -Etag` and `header -Last-Modified` do what you're looking for?
Yes, it does, if one uses `defer` on the Etag one.

The tricky part -- the reason I filed this ticket -- is trying to reduce the pitfall left for users who don't realize that they need any such configuration or clause for correct out-of-the-box function.
The defer is automatic/implied with `-` (removal). :+1: (no need to specify `defer`)

Fix patch:
diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go
index feef66de834..f335d314fa4 100644
--- a/modules/caddyhttp/fileserver/staticfiles.go
+++ b/modules/caddyhttp/fileserver/staticfiles.go
@@ -616,7 +616,11 @@ func (fsrv *FileServer) notFound(w http.ResponseWriter, r *http.Request, next ca
 // Prefix the etag with "W/" to convert it into a weak etag.
 // See: https://tools.ietf.org/html/rfc7232#section-2.3
 func calculateEtag(d os.FileInfo) string {
-	t := strconv.FormatInt(d.ModTime().Unix(), 36)
+	mtime := d.ModTime().Unix()
+	if mtime == 0 || mtime == 1 {
+		return "" // not useful anyway; see issue #5548
+	}
+	t := strconv.FormatInt(mtime, 36)
 	s := strconv.FormatInt(d.Size(), 36)
 	return `"` + t + s + `"`
 }


Test patch: