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

Commit

Permalink
Don't store responses from uncacheable range requests.
Browse files Browse the repository at this point in the history
Previously, this library would correctly detect range requests and not
serve cached responses. However, it would _store_ responses to partial
range requests, and then incorrectly serve them for non-range requests,
which would result in partial response provided when a full response is
expected.

This change fixes that. It tracks all uncacheable requests with a
single boolean and does not serve, not store responses from cache in
those cases.

In theory, it is possible to cache (to various degree) or serve from
cache requests with range, but that is currently not implemented by
this library and the current behavior was resulting in incorrect
responses. This commit focuses on fixing invalid behavior first.
Optional followup PRs can focus on optimizations by adding support for
storing/serving range requests/responses. However, the implementation
may not be trivial, and there may be other trade-offs at play.
  • Loading branch information
dmitshur committed May 20, 2016
1 parent 01b2fc8 commit 4082b08
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions httpcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,9 @@ func (t *Transport) setModReq(orig, mod *http.Request) {
// will be returned.
func (t *Transport) RoundTrip(req *http.Request) (resp *http.Response, err error) {
cacheKey := cacheKey(req)
cacheableMethod := req.Method == "GET" || req.Method == "HEAD"
cacheable := (req.Method == "GET" || req.Method == "HEAD") && req.Header.Get("range") == ""
var cachedResp *http.Response
if cacheableMethod {
if cacheable {
cachedResp, err = CachedResponse(t.Cache, req)
} else {
// Need to invalidate an existing value
Expand All @@ -194,7 +194,7 @@ func (t *Transport) RoundTrip(req *http.Request) (resp *http.Response, err error
transport = http.DefaultTransport
}

if cachedResp != nil && err == nil && cacheableMethod && req.Header.Get("range") == "" {
if cacheable && cachedResp != nil && err == nil {
if t.MarkCachedResponses {
cachedResp.Header.Set(XFromCache, "1")
}
Expand Down Expand Up @@ -281,7 +281,7 @@ func (t *Transport) RoundTrip(req *http.Request) (resp *http.Response, err error
}
}

if cacheableMethod && canStore(parseCacheControl(req.Header), parseCacheControl(resp.Header)) {
if cacheable && canStore(parseCacheControl(req.Header), parseCacheControl(resp.Header)) {
for _, varyKey := range headerAllCommaSepValues(resp.Header, "vary") {
varyKey = http.CanonicalHeaderKey(varyKey)
fakeHeader := "X-Varied-" + varyKey
Expand Down

0 comments on commit 4082b08

Please sign in to comment.