fileserver: Don't set Etag if mtime is 0 or 1 (close #5548)

[mholt]

Apparently useful for Nix environments

/cc @charles-dyfis-net

[charles-dyfis-net]

This solves half the problem -- the other half, Last-Modified (and browsers' use, in its presence, of If-Modified-Since), is unfortunately internal to net/http, so I'm not sure what a clean solution for that piece would look like.

[mholt]

Ah... I see.

Yeah, either Go would need to be patched or you'll need some config that removes the header at write-time, like header -Last-Modified.

I could see this kind of proposal having some weight with the Go team though, I mean, zeroed timestamps aren't useful for Last-Modified in the first place.

I would recommend opening an issue there -- approach it like a bug report. Setting a last-modified header based on a zero-value timestamp seems wrong. 🤔

[charles-dyfis-net]

Alas, in the Nix case, the constant is 1, specifically to reduce zero-value special-case handling breaking software builds; that makes it a harder sell. I'll grant that it may be worth a try, though.

[mholt]

Interesting, wouldn't you want it to be a zero-value since you want it to disregard the value?

But anyway, yeah, worth a shot to ask them. If they refuse then I recommend the approach to simply remove the header with a line of config.

I will merge this since I think it's more correct, now that we know there are 0 (and 1) -values out in the wild.

[mholt]

@charles-dyfis-net Looking closer, the Go standard lib DOES skip setting Last-Modified if the time is zero (or unix epoch):

https://github.com/golang/go/blob/8c445b7c9fe6738cbef2040a1011bd11489b0806/src/net/http/fs.go#L551-L553

So maybe they just need to do the same thing if it's 1 second after.

[emilazy]

Just checking, but does this mean that it's possible to override the Last-Modified header through the configuration or a module when serving static files? When I looked at the file server code it seemed like there was a direct path from the file's mtime to the final delivery through the Go HTTP library, but I didn't pry further than that so it's possible I'm misunderstanding something.

Thanks for this change, also! I forget why exactly Nix uses mtime=1 but I think it's because a lot of programs behave weirdly or break if you use the epoch time...

[mholt]

@emilazy

Just checking, but does this mean that it's possible to override the Last-Modified header through the configuration or a module when serving static files? When I looked at the file server code it seemed like there was a direct path from the file's mtime to the final delivery through the Go HTTP library, but I didn't pry further than that so it's possible I'm misunderstanding something.

Good question -- the answer is yes! If deferred, Caddy's header deletions don't happen until an HTTP handler writes the header to the response. So yes, Go's file server does write the response directly directly, but it calls our ResponseWriter wrapper, which modifies the headers before writing.

So a config like this:

header -Last-Modified

will delete the Last-Modified header when the response gets written because delete operations are implicitly deferred.

However, adding or setting headers is not deferred by default, so:

header Last-Modified foobar

would not have the desired outcome. However, this would:

header Last-Modified foobar {
    defer
}

because it explicitly defers the manipulation to write-time.

I know that's confusing, but it didn't used to be this way and it was confusing that deletions happened right away by default because later HTTP handlers in the chain would then add the header that the user was trying to delete 🤦‍♂️ So we decided it was best for deletions to be implicitly deferred.

[emilazy]

Thanks, that's great to know! Hopefully this means that we can implement all the functionality we need with a module on the Nix side.

[mholt]

Should be pretty straightforward. Let me know if you have any questions.