Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Invalid Content-Type for small initial write. #45

Open
tmthrgd opened this issue Apr 4, 2017 · 2 comments
Open

Invalid Content-Type for small initial write. #45

tmthrgd opened this issue Apr 4, 2017 · 2 comments

Comments

@tmthrgd
Copy link
Contributor

tmthrgd commented Apr 4, 2017

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@9855883
Fix was added in: tmthrgd/gziphandler@4324668

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)
	}
}
@adammck
Copy link
Contributor

adammck commented Apr 4, 2017

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.

@nytimes nytimes deleted a comment Sep 18, 2018
@klauspost
Copy link

Fixed in klauspost/compress@b7e9e8e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@adammck @tmthrgd @klauspost