From 9f7797bb0bf68c8b74bb161f14bb50947a09e531 Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Wed, 22 Aug 2018 03:26:01 -0300 Subject: [PATCH 1/3] Add test to failure to revalidate compressed JSON --- httpcache_test.go | 114 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 112 insertions(+), 2 deletions(-) diff --git a/httpcache_test.go b/httpcache_test.go index a504641..01d742f 100644 --- a/httpcache_test.go +++ b/httpcache_test.go @@ -2,6 +2,8 @@ package httpcache import ( "bytes" + "compress/gzip" + "encoding/json" "errors" "flag" "io" @@ -10,6 +12,7 @@ import ( "net/http/httptest" "os" "strconv" + "strings" "testing" "time" ) @@ -56,6 +59,30 @@ func setup() { w.Write([]byte(r.Method)) })) + var compressedJsonCounter = 0 + mux.HandleFunc("/compressedJson", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("accept-encoding"), "gzip") { + w.WriteHeader(http.StatusBadRequest) + return + } + + compressedJsonCounter++ + w.Header().Set("X-Counter", strconv.Itoa(compressedJsonCounter)) + + etag := "124567" + w.Header().Set("etag", etag) + if r.Header.Get("if-none-match") == etag { + w.WriteHeader(http.StatusNotModified) + return + } + + w.Header().Set("Content-Encoding", "gzip") + + gzw := gzip.NewWriter(w) + defer gzw.Close() + gzw.Write([]byte(`{"some": "json"}`)) + })) + mux.HandleFunc("/range", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { lm := "Fri, 14 Dec 2010 01:01:50 GMT" if r.Header.Get("if-modified-since") == lm { @@ -365,10 +392,93 @@ func TestDontStorePartialRangeInCache(t *testing.T) { } } +func TestRevalidateCompressedJSONResponses(t *testing.T) { + type some struct{ Some string } + { + req, err := http.NewRequest("GET", s.server.URL+"/compressedJson", nil) + if err != nil { + t.Fatal(err) + } + resp, err := s.client.Do(req) + if err != nil { + t.Fatal(err) + } + var got some + err = json.NewDecoder(resp.Body).Decode(&got) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + want := some{"json"} + if got != want { + t.Errorf("got %q, want %q", got, want) + } + if resp.StatusCode != http.StatusOK { + t.Errorf("response status code isn't 200 OK: %v", resp.StatusCode) + } + if resp.Header.Get(XFromCache) != "" { + t.Error("XFromCache header isn't blank") + } + if resp.Header.Get("x-counter") != "1" { + t.Error("X-Counter header is not 1") + } + if resp.Header.Get("etag") == "" { + t.Error("ETag is blank") + } + } + { + req, err := http.NewRequest("GET", s.server.URL+"/compressedJson", nil) + if err != nil { + t.Fatal(err) + } + resp, err := s.client.Do(req) + if err != nil { + t.Fatal(err) + } + var got some + err = json.NewDecoder(resp.Body).Decode(&got) + if err != nil { + t.Fatal(err) + } + // no defer here in order to update cache on Close call + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Errorf("response status code isn't 200 OK: %v", resp.StatusCode) + } + if resp.Header.Get(XFromCache) != "1" { + t.Errorf(`XFromCache header isn't "1": %v`, resp.Header.Get(XFromCache)) + } + if resp.Header.Get("x-counter") != "2" { + t.Error("X-Counter header is not 2") + } + } + { + req, err := http.NewRequest("GET", s.server.URL+"/compressedJson", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("cache-control", "only-if-cached") + resp, err := s.client.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Errorf("response status code isn't 200 OK: %v", resp.StatusCode) + } + if resp.Header.Get(XFromCache) != "1" { + t.Errorf(`XFromCache header isn't "1": %v`, resp.Header.Get(XFromCache)) + } + if resp.Header.Get("x-counter") != "2" { + t.Error("X-Counter was not updated on revalidation") + } + } +} + func TestCacheOnlyIfBodyRead(t *testing.T) { resetTest() { - req, err := http.NewRequest("GET", s.server.URL, nil) + req, err := http.NewRequest("GET", s.server.URL+"/method", nil) if err != nil { t.Fatal(err) } @@ -383,7 +493,7 @@ func TestCacheOnlyIfBodyRead(t *testing.T) { resp.Body.Close() } { - req, err := http.NewRequest("GET", s.server.URL, nil) + req, err := http.NewRequest("GET", s.server.URL+"/method", nil) if err != nil { t.Fatal(err) } From ee1021163d0bceb7f5c339a166222ee0d101244d Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Wed, 22 Aug 2018 03:26:18 -0300 Subject: [PATCH 2/3] Call OnEOF when full response read but hadn't EOF'ed yet --- httpcache.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/httpcache.go b/httpcache.go index f6a2ec4..ece06e2 100644 --- a/httpcache.go +++ b/httpcache.go @@ -521,7 +521,8 @@ type cachingReadCloser struct { // Underlying ReadCloser. R io.ReadCloser // OnEOF is called with a copy of the content of R when EOF is reached. - OnEOF func(io.Reader) + OnEOF func(io.Reader) + eofOnce sync.Once buf bytes.Buffer // buf stores a copy of the content of R. } @@ -534,12 +535,21 @@ func (r *cachingReadCloser) Read(p []byte) (n int, err error) { n, err = r.R.Read(p) r.buf.Write(p[:n]) if err == io.EOF { - r.OnEOF(bytes.NewReader(r.buf.Bytes())) + r.eofOnce.Do(func() { + r.OnEOF(bytes.NewReader(r.buf.Bytes())) + }) } return n, err } +var dummyBuf = make([]byte, 1) + func (r *cachingReadCloser) Close() error { + r.eofOnce.Do(func() { + if n, err := r.R.Read(dummyBuf); n == 0 && err == io.EOF { + r.OnEOF(bytes.NewReader(r.buf.Bytes())) + } + }) return r.R.Close() } From 8cedd3b6a191f649ee3ac08bebb90232c9154d2e Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Wed, 22 Aug 2018 15:29:32 -0300 Subject: [PATCH 3/3] Fix tests on go:1.6 On go:1.6 the error happens even on the original response from the server so the response is never added to the cache! --- httpcache_test.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/httpcache_test.go b/httpcache_test.go index 01d742f..069410e 100644 --- a/httpcache_test.go +++ b/httpcache_test.go @@ -394,6 +394,12 @@ func TestDontStorePartialRangeInCache(t *testing.T) { func TestRevalidateCompressedJSONResponses(t *testing.T) { type some struct{ Some string } + readJsonResponse := func(rc io.ReadCloser) (some, error) { + defer rc.Close() + var got some + err := json.NewDecoder(rc).Decode(&got) + return got, err + } { req, err := http.NewRequest("GET", s.server.URL+"/compressedJson", nil) if err != nil { @@ -403,12 +409,10 @@ func TestRevalidateCompressedJSONResponses(t *testing.T) { if err != nil { t.Fatal(err) } - var got some - err = json.NewDecoder(resp.Body).Decode(&got) + got, err := readJsonResponse(resp.Body) if err != nil { t.Fatal(err) } - defer resp.Body.Close() want := some{"json"} if got != want { t.Errorf("got %q, want %q", got, want) @@ -435,13 +439,10 @@ func TestRevalidateCompressedJSONResponses(t *testing.T) { if err != nil { t.Fatal(err) } - var got some - err = json.NewDecoder(resp.Body).Decode(&got) + _, err = readJsonResponse(resp.Body) if err != nil { t.Fatal(err) } - // no defer here in order to update cache on Close call - resp.Body.Close() if resp.StatusCode != http.StatusOK { t.Errorf("response status code isn't 200 OK: %v", resp.StatusCode) } @@ -462,7 +463,10 @@ func TestRevalidateCompressedJSONResponses(t *testing.T) { if err != nil { t.Fatal(err) } - defer resp.Body.Close() + _, err = readJsonResponse(resp.Body) + if err != nil { + t.Fatal(err) + } if resp.StatusCode != http.StatusOK { t.Errorf("response status code isn't 200 OK: %v", resp.StatusCode) }