Skip to content

Commit d00824f

Browse files
committed
fileserver: Improve Vary handling (#5849)
1 parent 8f87c5d commit d00824f

File tree

3 files changed

+30
-10
lines changed

3 files changed

+30
-10
lines changed

modules/caddyhttp/encode/encode.go

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ func (rw *responseWriter) WriteHeader(status int) {
239239
// Not Modified must have certain headers set as if it was a 200 response, and according to the issue
240240
// we would miss the Vary header in this case when compression was also enabled; note that we set this
241241
// header in the responseWriter.init() method but that is only called if we are writing a response body
242-
if status == http.StatusNotModified {
242+
if status == http.StatusNotModified && !hasVaryValue(rw.Header(), "Accept-Encoding") {
243243
rw.Header().Add("Vary", "Accept-Encoding")
244244
}
245245

@@ -349,14 +349,17 @@ func (rw *responseWriter) Unwrap() http.ResponseWriter {
349349

350350
// init should be called before we write a response, if rw.buf has contents.
351351
func (rw *responseWriter) init() {
352-
if rw.Header().Get("Content-Encoding") == "" && isEncodeAllowed(rw.Header()) &&
352+
hdr := rw.Header()
353+
if hdr.Get("Content-Encoding") == "" && isEncodeAllowed(hdr) &&
353354
rw.config.Match(rw) {
354355
rw.w = rw.config.writerPools[rw.encodingName].Get().(Encoder)
355356
rw.w.Reset(rw.ResponseWriter)
356-
rw.Header().Del("Content-Length") // https://github.com/golang/go/issues/14975
357-
rw.Header().Set("Content-Encoding", rw.encodingName)
358-
rw.Header().Add("Vary", "Accept-Encoding")
359-
rw.Header().Del("Accept-Ranges") // we don't know ranges for dynamically-encoded content
357+
hdr.Del("Content-Length") // https://github.com/golang/go/issues/14975
358+
hdr.Set("Content-Encoding", rw.encodingName)
359+
if !hasVaryValue(hdr, "Accept-Encoding") {
360+
hdr.Add("Vary", "Accept-Encoding")
361+
}
362+
hdr.Del("Accept-Ranges") // we don't know ranges for dynamically-encoded content
360363

361364
// strong ETags need to be distinct depending on the encoding ("selected representation")
362365
// see RFC 9110 section 8.8.3.3:
@@ -365,11 +368,23 @@ func (rw *responseWriter) init() {
365368
// (We have to strip the value we append from If-None-Match headers before
366369
// sending subsequent requests back upstream, however, since upstream handlers
367370
// don't know about our appending to their Etag since they've already done their work)
368-
if etag := rw.Header().Get("Etag"); etag != "" && !strings.HasPrefix(etag, "W/") {
371+
if etag := hdr.Get("Etag"); etag != "" && !strings.HasPrefix(etag, "W/") {
369372
etag = fmt.Sprintf(`%s-%s"`, strings.TrimSuffix(etag, `"`), rw.encodingName)
370-
rw.Header().Set("Etag", etag)
373+
hdr.Set("Etag", etag)
374+
}
375+
}
376+
}
377+
378+
func hasVaryValue(hdr http.Header, target string) bool {
379+
for _, vary := range hdr.Values("Vary") {
380+
vals := strings.Split(vary, ",")
381+
for _, val := range vals {
382+
if strings.EqualFold(strings.TrimSpace(val), target) {
383+
return true
384+
}
371385
}
372386
}
387+
return false
373388
}
374389

375390
// AcceptedEncodings returns the list of encodings that the

modules/caddyhttp/fileserver/browse.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func (fsrv *FileServer) serveBrowse(fileSystem fs.FS, root, dirPath string, w ht
105105
return caddyhttp.Error(http.StatusInternalServerError, err)
106106
}
107107

108-
w.Header().Add("Vary", "Accept")
108+
w.Header().Add("Vary", "Accept, Accept-Encoding")
109109

110110
// speed up browser/client experience and caching by supporting If-Modified-Since
111111
if ifModSinceStr := r.Header.Get("If-Modified-Since"); ifModSinceStr != "" {

modules/caddyhttp/fileserver/staticfiles.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,12 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c
375375
// etag is usually unset, but if the user knows what they're doing, let them override it
376376
etag := w.Header().Get("Etag")
377377

378+
// static file responses are often compressed, either on-the-fly
379+
// or with precompressed sidecar files; in any case, the headers
380+
// should contain "Vary: Accept-Encoding" even when not compressed
381+
// so caches can craft a reliable key (according to REDbot results)
382+
w.Header().Add("Vary", "Accept-Encoding")
383+
378384
// check for precompressed files
379385
for _, ae := range encode.AcceptedEncodings(r, fsrv.PrecompressedOrder) {
380386
precompress, ok := fsrv.precompressors[ae]
@@ -400,7 +406,6 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c
400406
defer file.Close()
401407
w.Header().Set("Content-Encoding", ae)
402408
w.Header().Del("Accept-Ranges")
403-
w.Header().Add("Vary", "Accept-Encoding")
404409

405410
// try to get the etag from pre computed files if an etag suffix list was provided
406411
if etag == "" && fsrv.EtagFileExtensions != nil {

0 commit comments

Comments
 (0)