Merge pull request gregjones#49 from gregjones/dont-store-uncacheable-method-responses

Fix invalid caching of uncacheable methods.

Author: gregjones

httpcache.go

@@ -281,10 +281,7 @@ func (t *Transport) RoundTrip(req *http.Request) (resp *http.Response, err error
 		}
 	}
 
-	reqCacheControl := parseCacheControl(req.Header)
-	respCacheControl := parseCacheControl(resp.Header)
-
-	if canStore(reqCacheControl, respCacheControl) {
+	if cacheableMethod && canStore(parseCacheControl(req.Header), parseCacheControl(resp.Header)) {
 		for _, varyKey := range headerAllCommaSepValues(resp.Header, "vary") {
 			varyKey = http.CanonicalHeaderKey(varyKey)
 			fakeHeader := "X-Varied-" + varyKey

httpcache_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"flag"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -48,6 +49,11 @@ func setup() {
 		w.Header().Set("Cache-Control", "max-age=3600")
 	}))
 
+	mux.HandleFunc("/method", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Cache-Control", "max-age=3600")
+		w.Write([]byte(r.Method))
+	}))
+
 	mux.HandleFunc("/nostore", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Cache-Control", "no-store")
 	}))
@@ -56,6 +62,7 @@ func setup() {
 		etag := "124567"
 		if r.Header.Get("if-none-match") == etag {
 			w.WriteHeader(http.StatusNotModified)
+			return
 		}
 		w.Header().Set("etag", etag)
 	}))
@@ -64,6 +71,7 @@ func setup() {
 		lm := "Fri, 14 Dec 2010 01:01:50 GMT"
 		if r.Header.Get("if-modified-since") == lm {
 			w.WriteHeader(http.StatusNotModified)
+			return
 		}
 		w.Header().Set("last-modified", lm)
 	}))
@@ -102,9 +110,9 @@ func setup() {
 		updateFieldsCounter++
 		if r.Header.Get("if-none-match") != "" {
 			w.WriteHeader(http.StatusNotModified)
-		} else {
-			w.Write([]byte("Some text content"))
+			return
 		}
+		w.Write([]byte("Some text content"))
 	}))
 }
 
@@ -117,6 +125,65 @@ func resetTest() {
 	clock = &realClock{}
 }
 
+// TestCacheableMethod ensures that uncacheable method does not get stored
+// in cache and get incorrectly used for a following cacheable method request.
+func TestCacheableMethod(t *testing.T) {
+	resetTest()
+	{
+		req, err := http.NewRequest("POST", s.server.URL+"/method", nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		resp, err := s.client.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		var buf bytes.Buffer
+		_, err = io.Copy(&buf, resp.Body)
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = resp.Body.Close()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if got, want := buf.String(), "POST"; 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)
+		}
+	}
+	{
+		req, err := http.NewRequest("GET", s.server.URL+"/method", nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		resp, err := s.client.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		var buf bytes.Buffer
+		_, err = io.Copy(&buf, resp.Body)
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = resp.Body.Close()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if got, want := buf.String(), "GET"; got != want {
+			t.Errorf("got wrong body %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.Errorf("XFromCache header isn't blank")
+		}
+	}
+}
+
 func TestGetOnlyIfCachedHit(t *testing.T) {
 	resetTest()
 	{