Set the LastModified header for raw files

[zeripath]

Although the use of LastModified dates for caching of git objects should be
discouraged (as it is not native to git - and there are a LOT of ways this
could be incorrect) - LastModified dates can be a helpful somewhat more human
way of caching for simple cases.

This PR adds this header and handles the If-Modified-Since header to the /raw/
routes for branch, tag and commit.

Fix #18354

Signed-off-by: Andrew Thornton art27@cantab.net

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (main@ab7f701). Click here to learn what that means.
The diff coverage is 46.05%.

@@           Coverage Diff           @@
##             main   #18356   +/-   ##
=======================================
  Coverage        ?   46.03%           
=======================================
  Files           ?      840           
  Lines           ?    92931           
  Branches        ?        0           
=======================================
  Hits            ?    42779           
  Misses          ?    43354           
  Partials        ?     6798           

Impacted Files	Coverage Δ
routers/web/repo/wiki.go	39.55% <0.00%> (ø)
routers/web/repo/download.go	33.70% <38.70%> (ø)
routers/api/v1/repo/file.go	65.21% <45.00%> (ø)
routers/common/repo.go	72.22% <50.00%> (ø)
modules/httpcache/httpcache.go	77.96% <59.09%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab7f701...ff236a5. Read the comment docs.

[Gusted]

Hmm, if this is going to cause major problems, we should revert this PR. But from the code it's looking good, it should send a new copy of the file when CommitterDate is changed, which should be sufficient to have good caching.

[zeripath]

It's a very poor cache and could lead to things being cached when they shouldn't be - for example if there has been a force-push.

However if you want to use LastModified instead of the etag then this is the risk you take.

I guess we need to ask what @silverwind thinks - my only worry is that Browsers start sending both and the IfModifiedSince incorrectly detects no change when there is one.

[Gusted]

my only worry is that Browsers start sending both and the IfModifiedSince incorrectly detects no change when there is one.

What do you mean sending both?

[zeripath]

I mean an HTTP Request sends both If-Modified-Since and If-None-Match.

[zeripath]

Actually I guess this just means we should deal with both headers at the same time.

[Gusted]

I mean an HTTP Request sends both If-Modified-Since and If-None-Match.

When used in combination with If-Modified-Since, If-None-Match has precedence (if the server supports it).

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match

Official ref: https://httpwg.org/specs/rfc7232.html#precedence

[silverwind]

Browsers by default send none of the headers. If they encounter Etag, they will send If-None-Match on subsequent requests, and if they see Last-Modified, they will send If-Modified-Since, assuming no other directives like no-store prevent caching.

[zeripath]

So if we send both Last-Modified and Etag they will send both If-Modified-Since and If-None-Match?

If so this code is likely incorrect and we need to make it so that we only pay attention to If-Modified-Since iff If-None-Match

[Gusted]

LGTM

[wxiaoguang]

How about work on #18448 ? IIRC http.ServeContent can handle these headers correctly.

[6543]

🚀