Automatically infer content type of response

[renfredxh]

When using the net/http server, if the "Content-Type" header of the response is not set at the time that it the response is served the http package will automatically try to sniff the content type. When using gziphandler, if the response is gzip compressed it will set the Content-Type of the response to be application/x-gzip.

This can be surprising for developers that rely on the behavior of ServeContent to apply the correct Content-Type header. Having an incorrect content-type header can cause unexpected behavior, depending on the browser.

Ideally the wrapped handler should set the Content-Type before returning, but since the standard library automatically handles this I thought it would be a good idea to have the same behavior preserved by GzipResponseWriter.

[adammck]

I wasn't aware of this behavior (and agree that gziphandler shouldn't break it, so thank you for pointing it out), but if I'm understanding correctly, ServeContent is generally used to serve files, not arbitrary requests. It wouldn't be appropriate to introduce this behavior to all handlers. Do you have any ideas how we might resolve this? Or perhaps I'm misunderstanding?

[renfredxh]

Right, ServeContent is for files but that is only one instance of where this behavior occurs in the net/http package. A better, more general example of how this is used is by net/http's response.Write. If you look at the docstring for that method you'll that it also performs Content-Type sniffing:

... if the handler didn't set a Content-Type, we sniff that from the initial chunk of output.

Following the rabbit-hole all the way down (or up) you'll see this response struct is the one that's used by conn.readRequest, which is called by conn.serve which is called by Server.Serve, which is the method many people use to serve requests via handlers in Go. Keeping the behavior of gziphandler consistent with how the Content-Type is automatically set by http.Server.Serve is what would be desirable.

[adammck]

Aha! Thank you very much for the explanation. I wasn't aware that the http package did this. You're right that we should preserve this behavior to avoid surprising developers. Your patch looks good, but would you mind adding a test, to ensure that this stays fixed?

[renfredxh]

Sure thing! Added a test. Output before patch:

--- FAIL: TestGzipHandler (0.00s)
        Error Trace:    gzip_test.go:73
    Error:      Not equal: "text/plain; charset=utf-8" (expected)
                    != "application/x-gzip" (actual)

FAIL
exit status 1
FAIL    github.com/NYTimes/gziphandler  0.019s

After:

PASS
ok      github.com/NYTimes/gziphandler  0.017s

Tried to make it as simple as possible, open to suggestions if you think it could be written in a better way.

[adammck]

Fantastic, thank you! Merged.