gziphandler icon indicating copy to clipboard operation
gziphandler copied to clipboard

Invalid Content-Type for small initial write.

Open tmthrgd opened this issue 7 years ago • 2 comments

The current method of inferring the mime type of the uncompressed data is broken when multiple calls are made to Write and the first block of data is small. The current method only considers the first call to Write and not subsequent calls. As the data is being buffered for the minSize test, it makes sense to detect the mime type across the buffer rather than the first fragment.

I noticed this in my fork which has diverged significantly, so my fix and test case won't apply cleanly, but they should provide someone a solid basis for fixing it in this repository, if someone thinks it worthwhile.

Test case was added in: tmthrgd/gziphandler@9855883c662e8516733524ea239882ee4ea9b1d7 Fix was added in: tmthrgd/gziphandler@4324668c5125c02964542a0b6d076b6bb28799a1

http.DetectContentType considers at most 512 bytes which is also the default minSize. So detecting the mime type over the minSize buffer provides much nicer behaviour here.

A test case that applies to this repository is below:

func TestInferContentType(t *testing.T) {
	wrapper, _ := NewGzipLevelAndMinSize(gzip.DefaultCompression, len("<!doctype html"))
	handler := wrapper(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		io.WriteString(w, "<!doc")
		io.WriteString(w, "type html>")
	}))

	req1, _ := http.NewRequest("GET", "/whatever", nil)
	req1.Header.Add("Accept-Encoding", "gzip")
	resp1 := httptest.NewRecorder()
	handler.ServeHTTP(resp1, req1)
	res1 := resp1.Result()

	const expect = "text/html; charset=utf-8"
	if ct := res1.Header.Get("Content-Type"); ct != expect {
		t.Error("Infering Content-Type failed for buffered response")
		t.Logf("Expected: %s", expect)
		t.Logf("Got:      %s", ct)
	}
}

tmthrgd avatar Apr 04 '17 10:04 tmthrgd

Great bug report, thanks! It seems reasonable to delay content type sniffing until we either write the gzip encoding header or close without/flush gzipping; I'm not sure why it's in Write at all.

adammck avatar Apr 04 '17 15:04 adammck

Fixed in https://github.com/klauspost/compress/commit/b7e9e8e0e63041258fa6cb9f3a9e8ab6a5428715

klauspost avatar Jun 03 '21 10:06 klauspost