fileserver: Define request vars for tracking content served

[charles-dyfis-net]

Adds two variables for requests being handled as static files:

• handlers.file_server.path - filesystem path as used by caddy to open file being served; may be an index file, a compressed proxy, or otherwise something other than the original content.
• handlers.file_server.info - fs.FileInfo structure with basename and stat data.

Per discussion in NixOS/nixpkgs#222354 (with thanks to @emilazy and @mholt)

---

Also, adopts a patch from @francislavoie to avoid setting Last-Modified when mtime is 1 second past epoch (as is the case for files in the Nix store).

[francislavoie]

I have no objections to this on its own, this seems fine.

But I do think we could probably just special case modtime=1 by comparing before & after http.ServeContent whether the Last-Modified header was modified with beforeLastModified != w.Header().Get('Last-Modified') && modtime == 1 then delete the header. This would directly solve it for Nix I think in the base case. As Matt said before, I don't think anyone other than Nix can reasonably expect to have files with modtime=1 so that should be safe.

[francislavoie]

These should go at the bottom of the file. Also, it should be http.file_server.*, see the reverse proxy which sets up similar variables. Actually for the proxy we're just setting it in the replacer directly instead of as a var. I'm not sure which way we should go, both have valid arguments and are equally as usable.

[charles-dyfis-net]

Attempting to follow convention more closely in 0609137.

[emilazy]

The mtime=1 case is already handled for the ETag (but unfortunately not Last-Modified due to the Go HTTP library): #5550

But that replaces incorrect caching with no caching at all; on the Nix side we want to set good ETags to get caching back, which we can do with domain-specific knowledge (paths in the Nix store are a good proxy for their content due to their immutability and the presence of the hash of the complete build environment); we plan to use a Caddy plugin to handle this, but we need access to the file path and stat info in order to determine whether the file is in the Nix store (there can be symlinks into the store from outside, etc.) and set the ETag appropriately.

We're open to whatever way of accomplishing this works best for Caddy and has the lowest maintenance and performance overhead; this is just what we came up with in the nixpkgs PR.

[francislavoie]

My suggested diff:

diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go
index 2b5cc3de3..48bf5c797 100644
--- a/modules/caddyhttp/fileserver/staticfiles.go
+++ b/modules/caddyhttp/fileserver/staticfiles.go
@@ -462,11 +462,23 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c
 		w = statusOverrideResponseWriter{ResponseWriter: w, code: statusCodeOverride}
 	}
 
+	// Nix uses modtime=1 for files in its store, and it's useless to set
+	// the Last-Modified header in that case, so we need to check if the
+	// header was set by anything else first (i.e. user config or another
+	// module) so that we can later check if http.ServeContent set the header.
+	modTime := info.ModTime()
+	beforeLastModified := w.Header().Get("Last-Modified")
+
 	// let the standard library do what it does best; note, however,
 	// 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))
+	http.ServeContent(w, r, info.Name(), modTime, file.(io.ReadSeeker))
+
+	// Delete the Last-Modified header if it was changed and modtime=1 (i.e. Nix store)
+	if beforeLastModified != w.Header().Get("Last-Modified") && modTime.Unix() == 1 {
+		w.Header().Del("Last-Modified")
+	}
 
 	return nil
 }

Re Etag, makes sense. So 👍 to me to adding to vars or replacer to allow your plugin to have the context it needs. My diff above is a suggestion to make it behave better in the case the plugin isn't used.

[charles-dyfis-net]

nod -- I was thinking of doing some work to make the plugin usage automatic, but even then that would only work for people running Caddy on NixOS, configured via Nix, not people running Caddy with configuration that isn't Nix-managed (as might typically be the case with folks running Nix on MacOS or an arbitrary non-NixOS Linux distro). Certainly, if y'all are comfortable with having codepaths that only Nix users are likely to need, it's better for our mutual userbase to have a less-surprising out-of-the-box experience.

[charles-dyfis-net]

Hmm -- unless I'm missing something, there's a big-picture problem here.

If we're using request context or vars set by staticfiles, that means we have to run after staticfiles.

To set an ETag honored by the go standard-library ServeHTTP implementation, we have to run before staticfiles.

[emilazy]

(You're missing that you can adjust the response after calling next.ServeHTTP in a middleware handler; see the handler docs. But there are some other considerations we should think about too; currently in the process of writing a comment on the nixpkgs PR.)

Edit: Actually this might not be true because the headers will already be written out after that? But I think that's what the Caddy header deferral stuff that I don't fully understand is for.

[francislavoie]

That's correct, you can adjust the response headers on the way out by doing it after calling next. That's how the header directive's defer mode works.

But for it to work correctly, you'll need to wrap the http.ResponseWriter with your own which will intercept writing the status code to apply your header changes just before that.

See these parts to get an idea how it works:

caddy/modules/caddyhttp/headers/headers.go

Line 99 in 942fbb3

w = &responseWriterWrapper{

and

caddy/modules/caddyhttp/headers/headers.go

Line 347 in 942fbb3

func (rww *responseWriterWrapper) WriteHeader(status int) {

[emilazy]

Ah, thank you; I missed the detail that you can pass down a different ResponseWriter to the next handler. Is there any meaningful performance impact to that?

[francislavoie]

Barely. Nanoseconds at worst.

[mholt]

Thanks for working with us to make something viable. We're making progress, I think we're almost there! :)

[mholt]

Suggested change

	// Files with an mtime of 1 second past epoch do not get an Etag.
	// Etag is omitted for files with a Unix epoch mod time of 0 or 1.

[mholt]

Might be useful to describe what the variables are actually named -- and since this is a terminal handler (it writes the response and does not call a next middleware), the variable can only be used in response wrappers (i.e. returning back "up" the middleware chain). Not sure how to word that exactly but I can offer a suggestion if you'd like :D

[mholt]

Does this work? When http.ServeContent() returns, the response is already written.

[francislavoie]

Yeah, good point, I'm not sure. I didn't try it.

[emilazy]

This is something we could handle with the same ResponseWriterWrapper trick, right? So maybe the mtime=1 special casing doesn't need to be upstream at all.

[mholt]

Can you just move these lines up to before ServeContent is called?

[francislavoie]

No because ServeContent is what is setting Last-Modified. That's what we're trying to intercept in this case.

[mholt]

Oh right, duh.

Yeah, a ResponseWriter wrapper is probably best then.

[mholt]

We'll probably tag 2.7 beta 2 in a few moments -- if this isn't ready before the release candidate (or stable, whichever comes first), might just bump this back to after 2.7.