Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

Commit

Permalink
Use method as cache key prefix for non-GET requests. (#75)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hx authored and dmitshur committed Sep 20, 2017
1 parent 787624d commit 316c5e0
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
6 changes: 5 additions & 1 deletion httpcache.go
Expand Up @@ -41,7 +41,11 @@ type Cache interface {

// cacheKey returns the cache key for req.
func cacheKey(req *http.Request) string {
return req.URL.String()
if req.Method == http.MethodGet {
return req.URL.String()
} else {
return req.Method + " " + req.URL.String()
}
}

// CachedResponse returns the cached http.Response for req if present, and nil
Expand Down
24 changes: 24 additions & 0 deletions httpcache_test.go
Expand Up @@ -218,6 +218,30 @@ func TestCacheableMethod(t *testing.T) {
}
}

func TestDontServeHeadResponseToGetRequest(t *testing.T) {
resetTest()
url := s.server.URL + "/"
req, err := http.NewRequest(http.MethodHead, url, nil)
if err != nil {
t.Fatal(err)
}
_, err = s.client.Do(req)
if err != nil {
t.Fatal(err)
}
req, err = http.NewRequest(http.MethodGet, url, nil)
if err != nil {
t.Fatal(err)
}
resp, err := s.client.Do(req)
if err != nil {
t.Fatal(err)
}
if resp.Header.Get(XFromCache) != "" {
t.Errorf("Cache should not match")
}
}

func TestDontStorePartialRangeInCache(t *testing.T) {
resetTest()
{
Expand Down

0 comments on commit 316c5e0

Please sign in to comment.