net/http: Deleting Content-Encoding in http.ServeFile on error can be a mistake

[martinrode]

We got just bitten by the problem that Content-Encoding is unconditionally removed in http.ServeFile if the file doesn't exist.

Here we do in our fileservering endpoint:

• Start GZIP encoding in a middleware (https://github.com/gorilla/handlers/blob/main/compress.go)-> this sets "Content-Encoding: gzip"
• Now calling http.ServeFile
• http.ServeFile doesn't find the requested file and outputs an 404 Not Found response
• The response gets GZIP encoded by the middleware (as it is wrapping of the http.ResponseWriter)
• Browser / Caller receives GZIPped "404 Not Found" which displays as Tofu as there is no Content-Encoding header

If you want to keep this in there (and I really don't see a reason why not just leave that header untouched), maybe make sure to not delete it if it is set to "gzip" or "deflate".

And I tried to re-read the whole discussion on the topic, but our case seems to be missed and there is no workaround without a bigger refactor!.

Originally posted by @martinrode in #66343 (comment)

[martinrode]

Sorry for the repost, but the problem is really a breaking change WITH NO remedy (see above)

[mitar]

I think you can use httpservecontentkeepheaders debug flag to get previous behavior.

I think your middleware should check before compression (on first Write) if Content-Encoding: gzip is still set.

[gabyhelp]

Related Issues

• net/http: serveError removes "Content-Encoding" which breaks our system (GODEBUG=httpservecontentkeepheaders=0) #71138 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[seankhliao]

see above.

[RPGillespie6]

So I guess this means we can't dynamically compress error responses floating out of ServeContent at all?

The Go 1.23 release notes say the solution is to set Transfer-Encoding instead of Content-Encoding in the middleware but that results in a crappy user experience. Chrome, at least, doesn't decompress the response at all with Transfer-Encoding set so you get this:

[RPGillespie6]

As a workaround, I was able to modify the middleware to add the Content-Encoding header back in if it detects it is missing. If your middleware implements the ResponseWriter interface, you can add some logic to the WriteHeader and Write methods, like this:

func (cw *compressResponseWriter) WriteHeader(c int) {
	h := cw.w.Header()

	// Go 1.23 introduced a breaking change makes it so Content-Encoding is deleted under certain conditions
	// For example if an error floats out of ServeContent (https://pkg.go.dev/net/http@master#ServeContent)
	// See also:
	//		https://github.com/golang/go/issues/66343
	// 		https://github.com/golang/go/issues/71149
	// 		https://tip.golang.org/doc/go1.23#language
	//
	// If Content-Encoding is missing, add it back in
	if h.Get("Content-Encoding") == "" {
		h.Set("Content-Encoding", cw.encType)
	}

	h.Del("Content-Length")
	cw.w.WriteHeader(c)
}

func (cw *compressResponseWriter) Write(b []byte) (int, error) {
	h := cw.w.Header()

	// If Content-Encoding is missing, return an uncompressed response
	// This can happen starting with Go 1.23 if an error floats out of ServeContent(https://pkg.go.dev/net/http@master#ServeContent)
	if h.Get("Content-Encoding") == "" {
		return cw.w.Write(b)
	}
        
        //...
}