Skip to content

Commit 316c5e0

Browse files
hxdmitshur
authored andcommitted
Use method as cache key prefix for non-GET requests. (gregjones#75)
The response for a HEAD request shouldn't be served in response to a GET request. In practice, HEAD responses contain http.noBody values, which return empty body if you try to read them. Prior to this change, that's what happened if you hit a server with a HEAD request and then GET the same URL. The GET response would be incorrectly served from cache with an empty body (that of the HEAD response). Rather than prefix all keys, we add prefixing to non-GET requests only. As well as saving the cost of a string concatenation, it ensures existing disk caches can upgrade without completely invalidating.
1 parent 787624d commit 316c5e0

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

‎httpcache.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,11 @@ type Cache interface {
4141

4242
// cacheKey returns the cache key for req.
4343
func cacheKey(req *http.Request) string {
44-
return req.URL.String()
44+
if req.Method == http.MethodGet {
45+
return req.URL.String()
46+
} else {
47+
return req.Method + " " + req.URL.String()
48+
}
4549
}
4650

4751
// CachedResponse returns the cached http.Response for req if present, and nil

‎httpcache_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,30 @@ func TestCacheableMethod(t *testing.T) {
218218
}
219219
}
220220

221+
func TestDontServeHeadResponseToGetRequest(t *testing.T) {
222+
resetTest()
223+
url := s.server.URL + "/"
224+
req, err := http.NewRequest(http.MethodHead, url, nil)
225+
if err != nil {
226+
t.Fatal(err)
227+
}
228+
_, err = s.client.Do(req)
229+
if err != nil {
230+
t.Fatal(err)
231+
}
232+
req, err = http.NewRequest(http.MethodGet, url, nil)
233+
if err != nil {
234+
t.Fatal(err)
235+
}
236+
resp, err := s.client.Do(req)
237+
if err != nil {
238+
t.Fatal(err)
239+
}
240+
if resp.Header.Get(XFromCache) != "" {
241+
t.Errorf("Cache should not match")
242+
}
243+
}
244+
221245
func TestDontStorePartialRangeInCache(t *testing.T) {
222246
resetTest()
223247
{

0 commit comments

Comments
 (0)