[BUG] CompressHandler if combined with http.Fileserver misses "Content-Encoding: deflate" on 404 status codes

[Lercher]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

curl --output - -i --compressed http://localhost:9000/notfound.html outputs

HTTP/1.1 404 Not Found
Content-Type: text/plain; charset=utf-8
Vary: Accept-Encoding
X-Content-Type-Options: nosniff
Date: Mon, 18 Nov 2024 13:26:14 GMT
Content-Length: 25

210Q(HLOU��/QH�/�K�☻♦��

for the program listed in "steps to reproduce".

Expected Behavior

It should output either "deflate and len 25 bytes"

HTTP/1.1 404 Not Found
Content-Encoding: deflate
Content-Type: text/plain; charset=utf-8
Vary: Accept-Encoding
X-Content-Type-Options: nosniff
Date: Mon, 18 Nov 2024 12:52:02 GMT
Content-Length: 25

404 page not found

or "not compressed and len 19 bytes"

HTTP/1.1 404 Not Found
Content-Type: text/plain; charset=utf-8
X-Content-Type-Options: nosniff
Date: Mon, 18 Nov 2024 12:54:26 GMT
Content-Length: 19

404 page not found

Steps To Reproduce

• go version go1.23.3 windows/amd64
• github.com/gorilla/handlers v1.5.2
• github.com/gorilla/mux v1.8.1

package main

import (
	"fmt"
	"log"
	"net/http"

	"github.com/gorilla/handlers"
	"github.com/gorilla/mux"
)

func main() {
	log.Println("Listening on http://localhost:9000/index.html")

	mux := mux.NewRouter()
	mux.Use(handlers.CompressHandler)

	mux.HandleFunc("/index.html", func(w http.ResponseWriter, r *http.Request) {
		_, _ = fmt.Fprintln(w, `<a href="/error404.html">Click me for a 404 error.</a> or use <pre>curl --output - -i --compressed http://localhost:9000</pre>`)
	})
	mux.PathPrefix("/").Methods("GET").Handler(http.FileServer(http.Dir(".")))

	log.Fatal(http.ListenAndServe(":9000", mux))
}

The "expected behavior" is present if the line mux.PathPrefix("/").Methods("GET").Handler(http.FileServer(http.Dir("."))) is commented out. It degrades to the "current behavior" if the FileServer is activated.

It works in both cases if a normal OK status code is produced (/index.html).

Chrome Dev tools behave similarly but different in details b/c of gzip / deflate request headers, I guess.

Anything else?

Sorry for not investigating more detail for the sample program. My more complex code worked with Go 1.22.x and handlers v1.5.1 and I don't know if it's an issue of the newer Go std lib, of gorilla/mux or gorilla/handlers.

Thanks
Martin

[Lercher]

More analysis: I guess, it's b/c of this Go std lib code in C:\Program Files\Go\src\net\http\fs.go which explicitly resets the header field "Content-Encoding":

// serveError serves an error from ServeFile, ServeFileFS, and ServeContent.
// Because those can all be configured by the caller by setting headers like
// Etag, Last-Modified, and Cache-Control to send on a successful response,
// the error path needs to clear them, since they may not be meant for errors.
func serveError(w ResponseWriter, text string, code int) {
	h := w.Header()

	nonDefault := false
	for _, k := range []string{
		"Cache-Control",
		"Content-Encoding",                               // <-------------- this line is "offensive"
		"Etag",
		"Last-Modified",
	} {
		if !h.has(k) {
			continue
		}
		if httpservecontentkeepheaders.Value() == "1" {
			nonDefault = true
		} else {
			h.Del(k)
		}
	}
	if nonDefault {
		httpservecontentkeepheaders.IncNonDefault()
	}

	Error(w, text, code)
}

So, I'm really unsure what indeed should be the correct behavior for the compress handler. Maybe it's defensive to just turn off compression in case of non OK http Status codes. Or, to add the compression-algorithm-compatible "Content-Encoding" header later, i.e. right before emitting the compressed body.

Or as the client app, honor this:

// GODEBUG=httpservecontentkeepheaders=1 restores the pre-1.23 behavior of not deleting
// Cache-Control, Content-Encoding, Etag, or Last-Modified headers on ServeContent errors.
var httpservecontentkeepheaders = godebug.New("httpservecontentkeepheaders")