Skip to content
Permalink
Browse files

check if Content-Encoding header is already set before compressing

In case the handler has already dealt with compression and applied it,
ServeContent shouldn't try to do duplicate work. Detect that situation
and serve content as is if so.

This also makes it possible for caller to indicate a file isn't worth
compressing by explicitly providing a Content-Encoding header:

	w.Header().Set("Content-Type", "image/png")
	w.Header()["Content-Encoding"] = nil
	httpgzip.ServeContent([...])

This is the other half of commit b1c53ac.

Move ServeContent tests to gzip_test.go file and rename tests to be
more idiomatic. Underscores are used in some example names, but they
shouldn't be used in test names.
  • Loading branch information...
dmitshur committed Jul 20, 2019
1 parent 1c7afaa commit 320755c1c1b0484e6179c9a5b68aabcc0dae5ac2
Showing with 79 additions and 32 deletions.
  1. +1 −31 fs_test.go
  2. +8 −1 gzip.go
  3. +70 −0 gzip_test.go
@@ -6,7 +6,6 @@ import (
"net/http/httptest"
"strings"
"testing"
"time"

"github.com/shurcooL/httpgzip"
"golang.org/x/tools/godoc/vfs/httpfs"
@@ -22,40 +21,11 @@ func ExampleFileServer() {
)))
}

// ServeContent should correctly determine the content type as "text/plain",
// not as "application/x-gzip".
func TestServeContent_detectContentType(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
content := "This is some plain text that compresses easily. " +
strings.Repeat("NaN", 16) + " Batman!"

httpgzip.ServeContent(w, req, "", time.Time{}, strings.NewReader(content))
}))
defer ts.Close()

req, err := http.NewRequest("GET", ts.URL, nil)
if err != nil {
t.Fatal(err)
}
req.Header.Set("Accept-Encoding", "gzip")
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatal(err)
}
resp.Body.Close()

got := resp.Header.Get("Content-Type")
want := "text/plain; charset=utf-8"
if got != want {
t.Errorf("got:\n%v\nwant:\n%v\n", got, want)
}
}

// Test that there are no infinite redirects at root path even if the entire
// req.URL.Path is stripped, e.g., via an overly aggressive http.StripPrefix.
// See https://github.com/shurcooL/httpgzip/pull/3
// and https://github.com/golang/go/commit/3745716bc3940f471137bf06fbe8c042257a43d3.
func TestFileServer_implicitLeadingSlash(t *testing.T) {
func TestFileServerImplicitLeadingSlash(t *testing.T) {
fs := httpfs.New(mapfs.New(map[string]string{
"foo.txt": "Hello world",
}))
@@ -28,11 +28,18 @@ type NotWorthGzipCompressing interface {
NotWorthGzipCompressing()
}

// ServeContent is like http.ServeContent, except it applies gzip compression.
// ServeContent is like http.ServeContent, except it applies gzip compression
// if compression hasn't already been done (i.e., the "Content-Encoding" header is set).
// It's aware of GzipByter and NotWorthGzipCompressing interfaces, and uses them
// to improve performance when the provided content implements them. Otherwise,
// it applies gzip compression on the fly, if it's found to be beneficial.
func ServeContent(w http.ResponseWriter, req *http.Request, name string, modTime time.Time, content io.ReadSeeker) {
// If compression has already been dealt with, serve as is.
if _, ok := w.Header()["Content-Encoding"]; ok {
http.ServeContent(w, req, name, modTime, content)
return
}

// If request doesn't accept gzip encoding, serve without compression.
if !httpguts.HeaderValuesContainsToken(req.Header["Accept-Encoding"], "gzip") {
http.ServeContent(w, req, name, modTime, content)
@@ -0,0 +1,70 @@
package httpgzip_test

import (
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"

"github.com/shurcooL/httpgzip"
)

// Test that ServeContent correctly determines the content type as "text/plain",
// not as "application/x-gzip".
func TestServeContentDetectContentType(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
content := "This is some plain text that compresses easily. " +
strings.Repeat("NaN", 16) + " Batman!"

httpgzip.ServeContent(w, req, "", time.Time{}, strings.NewReader(content))
}))
defer ts.Close()

req, err := http.NewRequest("GET", ts.URL, nil)
if err != nil {
t.Fatal(err)
}
req.Header.Set("Accept-Encoding", "gzip")
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatal(err)
}
resp.Body.Close()

got := resp.Header.Get("Content-Type")
want := "text/plain; charset=utf-8"
if got != want {
t.Errorf("got:\n%v\nwant:\n%v\n", got, want)
}
}

// Test that if the handler already explicitly set "Content-Encoding" header,
// then ServeContent shouldn't try to do apply compression, just serve as is.
func TestServeContentExplicitContentEncoding(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
content := "This is some plain text that compresses easily. " +
strings.Repeat("NaN", 16) + " Batman!"

w.Header()["Content-Encoding"] = nil
httpgzip.ServeContent(w, req, "", time.Time{}, strings.NewReader(content))
}))
defer ts.Close()

req, err := http.NewRequest("GET", ts.URL, nil)
if err != nil {
t.Fatal(err)
}
req.Header.Set("Accept-Encoding", "gzip")
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatal(err)
}
resp.Body.Close()

got := resp.Header.Get("Content-Encoding")
want := ""
if got != want {
t.Errorf("got:\n%q\nwant:\n%q\n", got, want)
}
}

0 comments on commit 320755c

Please sign in to comment.
You can’t perform that action at this time.